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

Retry when an async slotmap update fails #252

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Jan 14, 2025

Additional related corrections found while testing:

  • Handle empty addresses in CLUSTER SLOTS and CLUSTER NODES responses.
    Treat an empty ip string as it means the same endpoint the current command was sent to, as described in docs.
    Use the host address that is kept in the hiredis context.
    This solves the problem that an empty address was accepted, despite connecting using and empty address will fail in the next slotmap update.
  • Handle NIL addresses in CLUSTER SLOTS responses.

Test improvements:

  • Add connect-during-cluster-startup tests.
    Two new tests were added to verify that the slotmap will be be updated until it succeed.

  • clusterclient_async received a new flag --async-initial-update to be able to connect to
    an cluster using redisClusterAsyncConnect2, which will not block while performing the initial slotmap update.

  • Use short timeout when scheduling processing of next command in clusterclient_async.

bjosv added 14 commits January 14, 2025 00:28
Treat an empty ip string as it means the same endpoint the current command
was sent to, as described in docs.

Signed-off-by: Björn Svensson <[email protected]>
Treat an empty ip string as it means the same endpoint the current command
was sent to, as described in docs.

Signed-off-by: Björn Svensson <[email protected]>
Treat an empty ip string as it means the same endpoint the current command
was sent to, as described in docs. Use the host address that is kept in the
hiredis context.

Signed-off-by: Björn Svensson <[email protected]>
Treat an empty ip string as it means the same endpoint the current command
was sent to, as described in docs. Use the host address from the hiredis context.

Remove duplicate validation of host and port elements.

Signed-off-by: Björn Svensson <[email protected]>
The added flag --async-initial-update allows testcases to connect to
the cluster using `redisClusterAsyncConnect2`, which will not
block while performing the initial slotmap update.

Signed-off-by: Björn Svensson <[email protected]>
Since timers are handled before filedescription events in libevent
we will not give time to socket reads when sending multiple commands.
Using a short time allows hiredis to read from sockets and avoids
some unpredictability in tests.

Signed-off-by: Björn Svensson <[email protected]>
"A NULL value for the endpoint indicates the node has an unknown endpoint and
 the client should connect to the same endpoint it used to send the CLUSTER SLOTS
 command but with the port returned from the command."

Signed-off-by: Björn Svensson <[email protected]>
Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

LGTM

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.

A question about redisClusterAsyncConnect2() and slot map updates
2 participants