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

Limit the max size of bulk and index thread pools to bounded number of processors #15585

Merged
merged 2 commits into from Jan 8, 2016

Conversation

Projects
None yet
4 participants
@mikemccand
Copy link
Contributor

commented Dec 22, 2015

I only limit bulk and index thread pools.

Closes #15582

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2015

I'm also curious if there is a way to give feedback to users without breaking existing installations. Maybe @s1monw has a clue?

@spinscale spinscale added v2.2.1 and removed v2.2.0 labels Dec 23, 2015

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

@mikemccand to fix this the right way I agree we should be loud. I think we have to upgrade the settings in the clusterstate first and that can on work well if we do it only in a major version? On the other hand what if you have machines with 8 CPUs and others with 80 CPU? I mean this is really a per-node setting and I think we should maybe not allow to set this globally? I think you patch is good as a first step but we should really think about how to expose these settings?

@mikemccand

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

I talked to @s1monw ... we decided to add a logger.warn here if you tried to set to a too-large value, but otherwise be silent for now, and to open a separate issue that this setting should not be global and dynamically updatable.

@mikemccand

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

OK I pushed another commit w/ the feedback; I think it's ready.

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2016

LGTM

@mikemccand

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2016

Thanks @s1monw, I'll port to 2.2.0 as well.

@mikemccand mikemccand added v2.2.0 and removed v2.2.1 labels Jan 8, 2016

mikemccand pushed a commit that referenced this pull request Jan 8, 2016

Michael McCandless
Merge pull request #15585 from mikemccand/max_indexing_thread_pool_size
Limit the max size of bulk and index thread pools to bounded number of processors

@mikemccand mikemccand merged commit d3d41fc into elastic:master Jan 8, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

mikemccand added a commit to mikemccand/elasticsearch that referenced this pull request May 26, 2016

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.