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

Improve retry semantics #2330

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve retry semantics #2330

wants to merge 1 commit into from

Conversation

ikolomi
Copy link
Collaborator

@ikolomi ikolomi commented Sep 19, 2024

  1. Disable command retries for CME connections due to:
    a. Absence of command retry configuration in client creation params making retry mechanics unexpected, confusing and
    inconsistent with command timeout constrains.
    b. Retry semantics are inconsistent between the CME and CMD client since CMD lacks the retry logic

  2. Integrate fix for retry logic in redis-rs, cancelling the request in case the future was canceled. This will allow for a correct implementation of retry logic when its configuration is exposed to the user.

Issue: #2251

	a. Absence of command retry configuration in client creation params making retry mechanics unexpected, confusing and inconsistent with command timeout constrains.
	b. Retry semantics are inconsistent between the CME and CMD client since CMD lacks the retry logic
2. Integrate fix for retry logic in redis-rs, cancelling the request in case the future was canceled. This will allow for a correct implementation of retry logic when its configuration is exposed to the user.
@ikolomi ikolomi added Rust core Core changes Used to label a PR as PR with significant changes that should trigger a full matrix tests. labels Sep 19, 2024
@ikolomi ikolomi self-assigned this Sep 19, 2024
@ikolomi ikolomi requested a review from a team as a code owner September 19, 2024 12:49
@Yury-Fridlyand
Copy link
Collaborator

You're missing DCO.
Please list PRs and/or changes you're integrating from the submodule.

Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Please add changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core changes Used to label a PR as PR with significant changes that should trigger a full matrix tests. Rust core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants