-
Notifications
You must be signed in to change notification settings - Fork 337
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: use ctx and timer instead sleep #1256
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. Left one comment.
8d8b6f2
to
d32ac75
Compare
Let me fix the consumer seek test first. |
Seek test will be fixed by #1265. |
e307794
to
f7caa08
Compare
f7caa08
to
9904d77
Compare
@RobertIndie @crossoverJie Could you have a chance to review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation
We are using
time.Sleep
andfor
to execute the retry, which cannot be interrupted, this PR will use context and timer to improve this behavior.The next improvement idea supports passing the context to interrupt the retry, when closing the producer or consumer, we need to do that.
Modifications
Retry
method to execute the try, and useRetry
instead oftime.Sleep
andfor
.