-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Correct context for ClusterConnManager listener #83035
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
Correct context for ClusterConnManager listener #83035
Conversation
Today `ClusterConnectionManager#connectToNode` completes its listeners in the thread context in which the connection completes, which may not be the correct context if there are multiple concurrent connection attempts. With this commit we make sure to complete each listener in the context in which it was passed to the corresponding call to `connectToNode`.
Pinging @elastic/es-distributed (Team:Distributed) |
Hi @DaveCTurner, I've created a changelog YAML for you. |
@elasticmachine please run elasticsearch-ci/rest-compatibility |
#83170 will fix the failure @elasticmachine please run elasticsearch-ci/part-1 |
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
Are we sure about backporting to 7.17 though? I can't think of a bug this would cause?
💔 Backport failed
You can use sqren/backport to manually backport by running |
Yeah it might not be a real bug, at least not right now, but also not a controversial bugfix so worth the backport IMO. |
Today `ClusterConnectionManager#connectToNode` completes its listeners in the thread context in which the connection completes, which may not be the correct context if there are multiple concurrent connection attempts. With this commit we make sure to complete each listener in the context in which it was passed to the corresponding call to `connectToNode`. Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co>
Today `ClusterConnectionManager#connectToNode` completes its listeners in the thread context in which the connection completes, which may not be the correct context if there are multiple concurrent connection attempts. With this commit we make sure to complete each listener in the context in which it was passed to the corresponding call to `connectToNode`. Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co>
* Correct context for ClusterConnManager listener (#83035) Today `ClusterConnectionManager#connectToNode` completes its listeners in the thread context in which the connection completes, which may not be the correct context if there are multiple concurrent connection attempts. With this commit we make sure to complete each listener in the context in which it was passed to the corresponding call to `connectToNode`. Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co> * Missing import * Missed one Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co>
* Correct context for ClusterConnManager listener (#83035) Today `ClusterConnectionManager#connectToNode` completes its listeners in the thread context in which the connection completes, which may not be the correct context if there are multiple concurrent connection attempts. With this commit we make sure to complete each listener in the context in which it was passed to the corresponding call to `connectToNode`. Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co> * Missing import * Fix up tests Co-authored-by: ievgen.degtiarenko <ievgen.degtiarenko@elastic.co>
Today
ClusterConnectionManager#connectToNode
completes its listenersin the thread context in which the connection completes, which may not
be the correct context if there are multiple concurrent connection
attempts. With this commit we make sure to complete each listener in the
context in which it was passed to the corresponding call to
connectToNode
.