Skip to content
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

IndexingMemoryController should not track shard index states #15251

Merged
merged 2 commits into from Dec 4, 2015

Conversation

Projects
None yet
2 participants
@jasontedor
Copy link
Member

commented Dec 4, 2015

This commit modifies IndexingMemoryController to be stateless. Rather
than statefully tracking the indexing status of shards,
IndexingMemoryController can grab all available shards, check their idle
state, and then resize the buffers based on the number of and which
shards are not idle.

The driver for this change is a performance regression that can arise in
some scenarios after #13918. One scenario under which this performance
regression can arise is if an index is deleted and then created
again. Because IndexingMemoryController was previously statefully
tracking the state of shards via a map of ShardIds, the new shards with
the same ShardIds as previously existing shards would not be detected
and therefore their version maps would never be resized from the
defaults. This led to an explosion in the number of merges causing a
degradation in performance.

Closes #15225

IndexingMemoryController should not track shard index states
This commit modifies IndexingMemoryController to be stateless. Rather
than statefully tracking the indexing status of shards,
IndexingMemoryController can grab all available shards, check their idle
state, and then resize the buffers based on the number of and which
shards are not idle.

The driver for this change is a performance regression that can arise in
some scenarios after #13918. One scenario under which this performance
regression can arise is if an index is deleted and then created
again. Because IndexingMemoryController was previously statefully
tracking the state of shards via a map of ShardIds, the new shards with
the same ShardIds as previously existing shards would not be detected
and therefore their version maps would never be resized from the
defaults. This led to an explosion in the number of merges causing a
degradation in performance.

Closes #15225
for (Map.Entry<ShardId,Boolean> ent : shardWasActive.entrySet()) {
if (ent.getValue()) {
activeShardCount++;
private void calcAndSetShardBuffers() {

This comment has been minimized.

Copy link
@mikemccand

mikemccand Dec 4, 2015

Contributor

Can we just absorb this method directly into run()?

This comment has been minimized.

Copy link
@jasontedor

jasontedor Dec 4, 2015

Author Member

Done in 5341404.

@mikemccand

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2015

LGTM, this is a great simplification, thanks @jasontedor!

@jasontedor jasontedor removed the review label Dec 4, 2015

jasontedor added a commit that referenced this pull request Dec 4, 2015

Merge pull request #15251 from jasontedor/stateless-indexing-memory-c…
…ontroller

IndexingMemoryController should not track shard index states

@jasontedor jasontedor merged commit 5a391f1 into elastic:master Dec 4, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@jasontedor jasontedor deleted the jasontedor:stateless-indexing-memory-controller branch Dec 4, 2015

jasontedor added a commit that referenced this pull request Dec 7, 2015

Addtional simplifications in IndexingMemoryController
This commit removes some unneeded null checks from
IndexingMemoryController that were left over from the work in #15251,
and simplifies the try-catch block in
IndexingMemoryController#updateShardBuffers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.