-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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][client] Make seek wait for reconnect #18202
Conversation
Signed-off-by: Zixuan Liu <[email protected]>
fddbb68
to
68ba899
Compare
Codecov Report
@@ Coverage Diff @@
## master #18202 +/- ##
============================================
+ Coverage 34.91% 40.35% +5.44%
- Complexity 5707 8685 +2978
============================================
Files 607 687 +80
Lines 53396 67396 +14000
Branches 5712 7219 +1507
============================================
+ Hits 18644 27200 +8556
- Misses 32119 37161 +5042
- Partials 2633 3035 +402
Flags with carried forward coverage won't be shown. Click here to find out more.
|
is there any relationship to #16757? |
Right, but this PR fixes some issues on the client side. |
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.
I think we should reconsider seek. This PR only solves some problems, but it does not solve all seek and reconnect concurrency problems.
Do you have any suggestions? |
I have no idea about that. [PIP-194] https://lists.apache.org/thread/97o9t4ltkds5pfq41l9xbbd31t41qm8w discuss how to solve this problem. |
I think this is a different issue, I just fix the concurrency problems of seeking on the client side. |
The pr had no activity for 30 days, mark with Stale label. |
No reviewer, so close this PR. |
Motivation
When doing seek operation, the client needs to seed a seek command to the broker. When one seeks is successful, the broker disconnects the client, and then the client reconnects and resubscribes to the broker based on the seek position.
Right now, the consumer responds to the caller after sending a seek command succeeds, this should be incorrect behavior.
The right behavior should be to wait for reconnect and resubscribe, then respond to the caller.
Modifications
Based on the global
CompletableFuture
andseekMessageId
check the seek operation to wait for reconnect and resubscribe.When reconnecting is done, make the
CompletableFuture
to complete, when resubscribing is done, cleanup theseekMessageId
.Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: nodece#9