Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clarified Redis connection errors and added Redis dependency to docker-compose files #278

Merged
merged 6 commits into from
Jun 11, 2024

Conversation

frolvanya
Copy link
Collaborator

@frolvanya frolvanya commented Jun 10, 2024

  • Improve the error to make it less ambiguous, we need a clear message identifying it is related to connection to Redis

    Before:
    image

    After:
    image

  • Update the docker-compose.yml to ensure it is always in a working condition to docker-compose up from main

    To complete this, I've added redis dependency to postgres and rightsizing docker-compose files

  • Decide how to proceed with having Redis as a mandatory dependency and whether the rpc-server can work without it even with limited capabilities

    In any case we can merge these minor fixes now and later (@kobayurii @khorolets) will decide wether Redis is required or not

closes #274
closes #279

Copy link
Member

@khorolets khorolets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I will follow up in a separate issue about decision to transform the Redis dependency to optional

Copy link
Collaborator

@kobayurii kobayurii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! Thank you! I think we should use radis for development. It will only be used if we have a running instance of near_state_indexer which is based on nearcore. I would like you to check at the initialization stage whether the connection to redis is established and if there is an error then do not spawn the tasks update_final_block_regularly_from_redis, update_optimistic_block_regularly, OPTIMISTIC_UPDATING.set_not_working()
and generated a Warning error. This way you won't have to carry dependencies in all places.

I would also like you to update the readme.md specify redis as an optional dependency and describe its use only when paired with near_state_indexer.

@frolvanya frolvanya requested a review from kobayurii June 10, 2024 17:23
@frolvanya
Copy link
Collaborator Author

frolvanya commented Jun 10, 2024

I would also like you to update the readme.md specify redis as an optional dependency and describe its use only when paired with near_state_indexer.

Should it be done in root README.md file or inside the rpc-server/README.md?

I've added some info to rpc-server/README.md, but I'm not sure if this is enough:
https://github.com/near/read-rpc/blob/2a04ba3914128c8a0af01060123b249fda1383fb/rpc-server/README.md#redis-optional

@khorolets
Copy link
Member

Should it be done in root README.md file or inside the rpc-server/README.md?

It's enough for now, we'll perform docs cleaning later.

Copy link
Collaborator

@kobayurii kobayurii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you!

@khorolets khorolets merged commit dc74efe into near:develop Jun 11, 2024
3 checks passed
khorolets pushed a commit that referenced this pull request Jun 21, 2024
…docker-compose files (#278)

* feat: provided better error message in case "Connection refused" caused by Redis

* feat: added redis dependency to docker compose files

* feat: made redis an optional dependency

* chore: downgraded block fetching error logs to the warn level

* chore: set `OPTIMISTIC_UPDATING` to not working

* docs: added info about Redis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants