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
[Optimization]: During reroute async fetch data in GatewayAllocator, send request in generic threadpool instead of masterService#updateTask #57498
Comments
Pinging @elastic/es-distributed (:Distributed/Network) |
Talked about this with David and I think we can improve the way we use the networking layer instead of working around this issue by forking off to a different thread-pool to then have that hit the I'll look into creating a PR that optimises this and other spots where we might be sending a large number of requests to channels on the same selector in a loop. |
Thanks @original-brownbear for looking into it. Can you elaborate a bit on how are you planning to optimize the use of networking layer? Also, I was wondering that these async fetch calls are made per unassigned shard per node. These could be batched per node as well to reduce the no. of calls over the transport layer from master to data nodes? |
Sure, it's fairly straight forward. Currently, whenever we write a message to the transport layer we use
True, I think we could make protocol improvements/changes there. Since we identified a number of spots where improving the behaviour on the network layer would be beneficial and it's somewhat involved to introduce batching into these messages on the protocol level I think I think the network layer improvements are what I would focus on for now. |
I looked into the details of what a fix for this would look like in ES and found that Netty recently made some improvements to the way it avoids unnecessary wake-ups in netty/netty#9799 (fixed in a Netty version that isn't used in 7.1). => It's hard to tell from theory alone by how much the situation has already improved qualitatively but I would expect to see a non-trivial improvement relative to I still think this is worth improving on our end if we can, but there is a good chance you'll have a better experience with more recent (and upcoming) ES versions. |
I profiled this exact spot a lot lately when investigating slowness on the IO thread and I don't think this is much of an issue any more with the changes mentioned above. I don't think there's reasonable cause to improve batching of sending transport messages in the near future => closing this, it should be have much better in newer ES versions. |
Elasticsearch version (
bin/elasticsearch --version
): 7.1Description of the problem including expected versus actual behavior:
This cluster had roughly ~ 50K shards and multiple nodes were disconnected from master and during hot_threads dump observed that master#updateTask thread was spending lot of time sending these TransportNodesListGatewayStartedShards requests. These are async calls and response handling is done in separate threadpool but transport sendRequest seems to be still executing in master#updateTask. The sending logic could also move to separate threadpool.
Though with this #42855 this wont happen in the context of JoinExecutor but when reroute task is scheduled, it will happen.
Provide logs (if relevant):
The text was updated successfully, but these errors were encountered: