Rename the bulk thread pool to write thread pool #29593

jasontedor · 2018-04-18T19:04:41Z

This commit renames the bulk thread pool to the write thread pool. This is to better reflect the fact that the underlying thread pool is used to execute any document write request (single-document index/delete/update requests, and bulk requests).

With this change, we add support for fallback settings thread_pool.bulk.* which will be supported until 7.0.0.

We also add a system property so that the display name of the thread pool remains as "bulk" if needed to avoid breaking users.

This commit renames the bulk thread pool to the write thread pool. This is to better reflect the fact that the underlying thread pool is used to execute any document write request (single-document index/delete/update requests, and bulk requests). With this change, we add support for fallback settings thread_pool.bulk.* which will be supported until 7.0.0. We also add a system property so that the display name of the thread pool remains as "bulk" if needed to avoid breaking users.

elasticmachine · 2018-04-18T19:04:43Z

Pinging @elastic/es-core-infra

rjernst

LGTM

rjernst · 2018-04-18T19:11:52Z

docs/reference/migration/migrate_6_3.asciidoc

+`thread_pool.write.size` and `thread_pool.write.queue_size`, respectively. The
+fallback settings will be removed in 7.0.0. Additionally, this means that some
+APIs (e.g., the node stats API) will now display the name of this thread pool as
+`write`. If this is change impacts you (e.g., for monitoring that you have in


typo: is change -> change

rjernst · 2018-04-18T19:15:17Z

server/src/main/java/org/elasticsearch/threadpool/FixedExecutorBuilder.java

+                            fallbackSizeSetting,
+                            s -> Setting.parseInt(s, 1, applyHardSizeLimit(settings, name), sizeKey),
+                            properties);
+            this.fallbackSizeSetting = fallbackSizeSetting;


nit: this could be set above instead of creating a local?

rjernst · 2018-04-18T19:15:33Z

server/src/main/java/org/elasticsearch/threadpool/FixedExecutorBuilder.java

+                            fallbackQueueSizeSetting,
+                            s -> Setting.parseInt(s, Integer.MIN_VALUE, queueSizeKey),
+                            properties);
+            this.fallbackQueueSizeSetting = fallbackQueueSizeSetting;


jpountz · 2018-04-18T20:00:54Z

docs/reference/modules/threadpool.asciidoc

-`bulk`::
-    For bulk operations. Thread pool type is `fixed`
+`write`::
+    For write operations. Thread pool type is `fixed`


let's be more explicit, eg. "index and bulk"?

jpountz · 2018-04-18T20:04:54Z

server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java

@@ -69,7 +69,7 @@
        public static final String GET = "get";
        public static final String ANALYZE = "analyze";
        public static final String INDEX = "index";


silly question (only looking at the diff I don't have full context), do we need to keep this constant?

@jpountz Yes; this is in 6.x where the index thread pool remains.

pickypg · 2018-04-19T00:31:22Z

server/src/test/java/org/elasticsearch/action/bulk/BulkProcessorRetryIT.java

@@ -55,7 +55,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
                //.put("thread_pool.listener.queue_size", 1)
                .put("thread_pool.get.queue_size", 1)
                // default is 50


Nitpick, but can we update this to 200 too (or remove the comment?).

I pushed bec9592.

This commit renames the bulk thread pool to the write thread pool. This is to better reflect the fact that the underlying thread pool is used to execute any document write request (single-document index/delete/update requests, and bulk requests). With this change, we add support for fallback settings thread_pool.bulk.* which will be supported until 7.0.0. We also add a system property so that the display name of the thread pool remains as "bulk" if needed to avoid breaking users.

* master: (21 commits) Remove bulk fallback for write thread pool (elastic#29609) Fix an incorrect reference to 'zero_terms_docs' in match_phrase queries. Update the version compatibility for zero_terms_query in match_phrase. Account translog location to ram usage in version map Remove extra spaces from changelog Add support to match_phrase query for zero_terms_query. (elastic#29598) Fix incorrect references to 'zero_terms_docs' in query parsing error messages. (elastic#29599) Build: Move java home checks to pre-execution phase (elastic#29548) Avoid side-effect in VersionMap when assertion enabled (elastic#29585) [Tests] Remove accidental logger usage Add tests for ranking evaluation with aliases (elastic#29452) Deprecate use of `htmlStrip` as name for HtmlStripCharFilter (elastic#27429) Update plan for the removal of mapping types. (elastic#29586) [Docs] Add rankEval method for Jva HL client Make ranking evaluation details accessible for client Rename the bulk thread pool to write thread pool (elastic#29593) [Test] Minor changes to rank_eval tests (elastic#29577) Fix missing node id prefix in startup logs (elastic#29534) Added painless execute api. (elastic#29164) test: also assert deprecation warning after clusters have been closed. ...

* es/6.x: (28 commits) TEST: Unmute testPrimaryRelocationWhileIndexing Never leave stale delete tombstones in version map (#29619) Do not serialize common stats flags using ordinal (#29600) Remove stale comment from JVM stats (#29625) TEST: Mute testPrimaryRelocationWhileIndexing Remove 7.0.0 from 6.x changelog (#29621) Add support to match_phrase query for zero_terms_query. (#29598) Account translog location to ram usage in version map Avoid side-effect in VersionMap when assertion enabled (#29585) Build: Move java home checks to pre-execution phase (#29548) Add tests for ranking evaluation with aliases (#29452) [Test] Fix assertion in SearchDocumentationIT Deprecate use of `htmlStrip` as name for HtmlStripCharFilter (#27429) test: Assert deprecated http.enebled setting warning Update plan for the removal of mapping types. (#29586) [Docs] Add rankEval method for Jva HL client Make ranking evaluation details accessible for client Rename the bulk thread pool to write thread pool (#29593) [Test] Minor changes to rank_eval tests (#29577) test: Assert deprecated http.enebled setting warning ...

simmel · 2018-06-14T10:42:47Z

This change also broke the nodes stats API since there is no bulk threadpool anymore. So all metrics and alerting tools which used this information is now broken in a minor version upgrade of Elasticsearch.

Yes, it's mentioned but still, in a minor?

simmel · 2018-06-14T10:43:52Z

Gah, note to self: don't write comments when you're annoyed. There is a workaround mentioned in the linked site.

Bulk threadpool was renamed to index. See elastic/elasticsearch#29593

jasontedor added >breaking review :Core/Infra/Core Core issues without another label v7.0.0 v6.3.0 labels Apr 18, 2018

jasontedor requested a review from nik9000 April 18, 2018 19:04

rjernst approved these changes Apr 18, 2018

View reviewed changes

jpountz approved these changes Apr 18, 2018

View reviewed changes

jasontedor added 2 commits April 18, 2018 18:39

Fix doc typo

b05e624

Explicit

f420edb

pickypg reviewed Apr 19, 2018

View reviewed changes

Fix comment

bec9592

jpountz approved these changes Apr 19, 2018

View reviewed changes

jasontedor merged commit bab45e7 into elastic:6.x Apr 19, 2018

jasontedor deleted the bulk-to-write branch October 16, 2018 14:31

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

lukas-vlcek mentioned this pull request Oct 2, 2020

Bug 1883719: Bulk statistics are recorded under index threadpool openshift/elasticsearch-operator#511

Merged

lukas-vlcek added a commit to lukas-vlcek/elasticsearch-operator that referenced this pull request Oct 13, 2020

Bug 1883719: Bulk statistics are recorded under write threadpool

6b88270

Bulk threadpool was renamed to index. See elastic/elasticsearch#29593

lukas-vlcek mentioned this pull request Oct 13, 2020

Bug 1883719: Bulk statistics are recorded under write threadpool openshift/elasticsearch-operator#533

Merged

openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/elasticsearch-operator that referenced this pull request Oct 14, 2020

Bug 1883719: Bulk statistics are recorded under write threadpool

4dde968

Bulk threadpool was renamed to index. See elastic/elasticsearch#29593

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename the bulk thread pool to write thread pool #29593

Rename the bulk thread pool to write thread pool #29593

jasontedor commented Apr 18, 2018

elasticmachine commented Apr 18, 2018

rjernst left a comment

rjernst Apr 18, 2018

rjernst Apr 18, 2018

rjernst Apr 18, 2018

jpountz Apr 18, 2018

jpountz Apr 18, 2018

jasontedor Apr 18, 2018

pickypg Apr 19, 2018

jasontedor Apr 19, 2018 •

edited

Loading

simmel commented Jun 14, 2018

simmel commented Jun 14, 2018

Rename the bulk thread pool to write thread pool #29593

Rename the bulk thread pool to write thread pool #29593

Conversation

jasontedor commented Apr 18, 2018

elasticmachine commented Apr 18, 2018

rjernst left a comment

Choose a reason for hiding this comment

rjernst Apr 18, 2018

Choose a reason for hiding this comment

rjernst Apr 18, 2018

Choose a reason for hiding this comment

rjernst Apr 18, 2018

Choose a reason for hiding this comment

jpountz Apr 18, 2018

Choose a reason for hiding this comment

jpountz Apr 18, 2018

Choose a reason for hiding this comment

jasontedor Apr 18, 2018

Choose a reason for hiding this comment

pickypg Apr 19, 2018

Choose a reason for hiding this comment

jasontedor Apr 19, 2018 • edited Loading

Choose a reason for hiding this comment

simmel commented Jun 14, 2018

simmel commented Jun 14, 2018

jasontedor Apr 19, 2018 •

edited

Loading