Skip to content

Batch cluster state tasks for index opening, closing, and blocks#83432

Closed
joegallo wants to merge 8 commits intoelastic:masterfrom
joegallo:batch-index-blocks
Closed

Batch cluster state tasks for index opening, closing, and blocks#83432
joegallo wants to merge 8 commits intoelastic:masterfrom
joegallo:batch-index-blocks

Conversation

@joegallo
Copy link
Copy Markdown
Contributor

@joegallo joegallo commented Feb 2, 2022

Closes #81627

Pretty straightforward -- instead of using unbatched() five times, we use a simple batching executor that just loops through the tasks and executes them all.

A few thoughts:

  1. open-indices uses an AckedClusterStateUpdateTask rather than just a ClusterStateUpdateTask -- is it still okay to batch those together with the rest of the tasks?
  2. For that matter, are there any drawbacks to batching open/close and the index blocks all together? I can't think of any, but I'm curious if anybody else can come up with a reason we shouldn't.

@joegallo joegallo added :Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. v8.2.0 labels Feb 2, 2022
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Feb 2, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Joe! I'll take a detailed look tomorrow. One small point only after a quick read. I think this might be fine as is but I'll have to think through potential side effects for a bit.

return allocationService.reroute(updatedState, "indices opened [" + indicesAsString + "]");
}
},
ClusterStateTaskExecutor.unbatched()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we want to batch this in with the other operations? It seems like this is not something that ILM will do in a hot loop right?
I could see some strange side effects of batching open and close index updates into a single batch in some edge cases.

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @joegallo, I've created a changelog YAML for you.

@joegallo
Copy link
Copy Markdown
Contributor Author

joegallo commented Feb 3, 2022

With the callback-accepting simple batching executor I added in ee83da0, we could defer the allocationService.reroute in the open and close tasks until the end of their batch.

@joegallo
Copy link
Copy Markdown
Contributor Author

joegallo commented Feb 3, 2022

if (codecChange) {
mergeSteps.add(closeIndexStep);
mergeSteps.add(updateBestCompressionSettings);
mergeSteps.add(openIndexStep);
mergeSteps.add(waitForIndexGreenStep);
}

w.r.t. batching of opens and closes, I'm hesitant to only batch the blocks, because it seems like the blocks are part of the close, and then the close is always followed by an open (at least in the case of ILM). OTOH if the closes and opens are cheaper/faster by an order of magnitude or something like that, then maybe it doesn't matter.

@joegallo
Copy link
Copy Markdown
Contributor Author

Between #83760, #84259, and #84374, this one is no longer necessary and can be closed.

@joegallo joegallo closed this Feb 24, 2022
@joegallo joegallo deleted the batch-index-blocks branch February 24, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. >enhancement Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Batch add-block-index-to-close, add-index-block and finalize-index-block tasks

4 participants