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 #159

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Jan 21, 2025

Small fix to reattempt failing slotmap updates.

Additionally:

  • Handle empty and NULL addresses in CLUSTER SLOTS responses.
    As described in cluster slots docs:
    Clients may treat the empty string in the same way as NULL, that is the same endpoint it used to send the current command to

  • clusterclient_async received a new flag --async-initial-update to enable usage of valkeyClusterAsyncConnect2, which avoids blocking initial slotmap update.

  • Use short timeout when scheduling processing of next command in clusterclient_async and allow the event engine to read from sockets before next command, which improves predictability in tests.

Final change similar to Nordix/hiredis-cluster#252.

bjosv added 3 commits January 21, 2025 22:48
…dates

* clusterclient_async received a new flag --async-initial-update to enable usage of
  valkeyClusterAsyncConnect2, which avoids blocking initial slotmap update.
* Use short timeout when scheduling processing of next command in clusterclient_async
  which allow the event engine to read from sockets to improve predictability in tests.

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
As described in cluster slots docs:
"Clients may treat the empty string in the same way as NULL,
 that is the same endpoint it used to send the current command to"

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
Additionally two new tests are added to verify that the slotmap
updates are retried until it succeeds.

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
@bjosv bjosv requested a review from zuiderkwast January 21, 2025 22:45
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
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

@bjosv bjosv merged commit fb8af4c into valkey-io:main Jan 23, 2025
43 checks passed
@bjosv bjosv deleted the support-empty-ip-in-cluster-slots branch January 23, 2025 13:04
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.

2 participants