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

Remove the index thread pool #29556

Merged
merged 2 commits into from
Apr 18, 2018

Conversation

jasontedor
Copy link
Member

Now that single-document indexing requests are executed on the bulk thread pool the index thread pool is no longer needed. This commit removes this thread pool from Elasticsearch.

Now that single-document indexing requests are executed on the bulk
thread pool the index thread pool is no longer needed. This commit
removes this thread pool from Elasticsearch.
@jasontedor jasontedor added >breaking review :Core/Infra/Core Core issues without another label v7.0.0 labels Apr 17, 2018
@jasontedor jasontedor requested a review from bleskes April 17, 2018 10:55
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jpountz
Copy link
Contributor

jpountz commented Apr 17, 2018

Should we rename bulk to index at the same time? I think this would be a better name for this threadpool?

@jasontedor
Copy link
Member Author

jasontedor commented Apr 17, 2018

@jpountz I was thinking something similar last night except renaming it to the write thread pool. What do you think?

@jpountz
Copy link
Contributor

jpountz commented Apr 17, 2018

👍 I had done something similar at https://github.com/elastic/elasticsearch/pull/12939/files#diff-28405cdd8e4973b90f711084352459be. It also merged the get and search threadpools into a read threadpool.

@jpountz
Copy link
Contributor

jpountz commented Apr 17, 2018

Correction: I wanted to merge get and search but faced some problems, see comment in the PR.

@nik9000
Copy link
Member

nik9000 commented Apr 17, 2018

I was thinking something similar last night except renaming it to the write thread pool. What do you think?

Maybe it makes sense to do it in a follow up?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

@jasontedor
Copy link
Member Author

Maybe it makes sense to do it in a follow up?

I already have a change queued locally ready to go in a follow-up once this PR is in. 😇

@jasontedor jasontedor merged commit 2b47d67 into elastic:master Apr 18, 2018
@jasontedor jasontedor deleted the index-thread-pool-be-gone branch April 18, 2018 13:18
dnhatn added a commit that referenced this pull request Apr 18, 2018
* master:
  Remove the index thread pool (#29556)
  Remove extra copy in ScriptDocValues.Strings
  Fix full cluster restart test recovery (#29545)
  Fix binary doc values fetching in _search (#29567)
  Mutes failing MovAvgIT tests
  Fix the assertion message for an incorrect current version. (#29572)
  Fix the version ID for v5.6.10. (#29570)
  Painless Spec Documentation Clean Up (#29441)
  Add versions 5.6.10 and 6.2.5
  [TEST] test against scaled value instead of fixed epsilon in MovAvgIT
  Remove `flatSettings` support from request classes (#29560)
  MapperService to wrap a single DocumentMapper. (#29511)
  Fix dependency checks on libs when generating Eclipse configuration. (#29550)
  Add null_value support to geo_point type (#29451)
  Add documentation about the include_type_name option. (#29555)
  Enforce translog access via engine (#29542)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking :Core/Infra/Core Core issues without another label v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants