Batch ApplyAliasActions cluster state updates (#89924) #90010
Conversation
elasticsearchmachine
added
the
needs:triage
gmarouli
added
:Data Management/Indices APIs
elasticsearchmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
gmarouli
added
>bug
>refactoring
and removed
Team:Data Management
elasticsearchmachine
added
the
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
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
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 <[email protected]>
|
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
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.
7 participants
The
MetadataIndexAliasesServicewas submitting the cluster state update tasks unbatched with priorityURGENT. This PR is an attempt to fix this.Closes: #89924.