-
Notifications
You must be signed in to change notification settings - Fork 1.3k
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Timeout point send in forward_request
verb comes from seastar::lowres_clock
#12458
Comments
@havaker please fix - it seams that you know exactly what the problem is 🙂 |
Note: this bug exists in 5.1 version (since the merge of #9209). |
Thanks for the notice. this can be backported when the fix is ready. We are using the milestones to reflect when this issue will have a fix (or at least when we intend to get to it) - therefore 5.2 is the correct milestone. |
Fixed by bbbe12a |
`forward_request` verb carried information about timeouts using `lowres_clock::time_point` (that came from local steady clock `seastar::lowres_clock`). The time point was produced on one node and later compared against other node `lowres_clock`. That behavior was wrong (`lowres_clock::time_point`s produced with different `lowres_clock`s cannot be compared) and could lead to delayed or premature timeout. To fix this issue, `lowres_clock::time_point` was replaced with `lowres_system_clock::time_point` in `forward_request` verb. Representation to which both time point types serialize is the same (64-bit integer denoting the count of elapsed nanoseconds), so it was possible to do an in-place switch of those types using logic suggested by @avikivity: - using steady_clock is just broken, so we aren't taking anything from users by breaking it further - once all nodes are upgraded, it magically starts to work Closes #12529 (cherry picked from commit bbbe12a) Fixes #12458
Backported to 5.1. 5.0 did not have the broken functionality. |
In the parallelized aggregation layer
query::forward_request
(that is sent to remote nodes) carries information about timeouts usinglowres_clock::time_point
(that came from localseastar::lowres_clock
). Thelowres_clock::time_point
is later used on remote nodes to designate a timeout date. This is wrong and may lead to delayed or premature timeout.The text was updated successfully, but these errors were encountered: