Batch ApplyAliasActions cluster state updates (#89924)#90010
Batch ApplyAliasActions cluster state updates (#89924)#90010gmarouli merged 16 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
@dakrone & @DaveCTurner , I had some time on Friday and I wanted to give this a try to improve my knowledge on different parts of our domain. If you have a moment I would appreciate your input, if you are too busy let me know :). |
DaveCTurner
left a comment
There was a problem hiding this comment.
Thanks for picking this up @gmarouli; I left a couple of comments.
Also the executor deserves at least a little bit of testing, possibly using something from ClusterStateTaskExecutorUtils.
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine update branch |
|
@DaveCTurner thank you for the feedback and the tips, I will work on testing the executor too. |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
DaveCTurner
left a comment
There was a problem hiding this comment.
Sorry this dropped off my list @gmarouli. Looks good, I left a few nits and suggestions.
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
Outdated
Show resolved
Hide resolved
Co-authored-by: David Turner <david.turner@elastic.co>
|
The test I added wasn't working properly from the beginning. Working on it. An updated version will come soon. |
|
@DaveCTurner thank you for taking another look, the code improved a lot because of your suggestions. Furthermore, I fixed the test which wasn't doing what I thought was doing and I improved my understanding. I am very grateful. Since this relates to a recent issue we discussed with @original-brownbear , I am adding him in this PR as a reviewer. |
original-brownbear
left a comment
There was a problem hiding this comment.
Looks good, mostly just one suggestion on shortening this a little more and avoiding some duplication.
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
Show resolved
Hide resolved
|
LGTM, just for posterity. The latest customer issue that motivated work on this did not only show problems due to not batching these tasks, but also problems from just extremely slowness in the tasks themselves. -> #92609 |
|
I won't get a chance to give this a final look soon so don't wait for me 🙂 |
This reverts commit 9a7d2b5.
The
MetadataIndexAliasesServicewas submitting the cluster state update tasks unbatched with priorityURGENT. This PR is an attempt to fix this.Closes: #89924.