Default to one shard #30539

jasontedor · 2018-05-11T15:48:31Z

This commit changes the default out-of-the-box configuration for the number of shards from five to one. We think this will help address a common problem of oversharding. For users with time-based indices that need a different default, this can be managed with index templates. For users with non-time-based indices that find they need to re-shard with the split API in place they no longer need to resort only to reindexing.

Since this has the impact of changing the default number of shards used in REST tests, we want to ensure that we still have coverage for issues that could arise from multiple shards. As such, we randomize (rarely) the default number of shards in REST tests to two. This is managed via a global index template. However, some tests check the templates that are in the cluster state during the test. Since this template is randomly there, we need a way for tests to skip adding the template used to set the number of shards to two. For this we add the default_shards feature skip. To avoid having to write our docs in a complicated way because sometimes they might be behind one shard, and sometimes they might be behind two shards we apply the default_shards feature skip to all docs tests. That is, these tests will always run with the default number of shards (one).

This commit changes the default out-of-the-box configuration for the number of shards from five to one. We think this will help address a common problem of oversharding. For users with time-based indices that need a different default, this can be managed with index templates. For users with non-time-based indices that find they need to re-shard with the shrink API in place they no longer need to resort only to reindexing. Since this has the impact of changing the default number of shards used in REST tests, we want to ensure that we still have coverage for issues that could arise from multiple shards. As such, we randomize (rarely) the default number of shards in REST tests to two. This is managed via a global index template. However, some tests check the templates that are in the cluster state during the test. Since this template is randomly there, we need a way for tests to skip adding the template used to set the number of shards to two. For this we add the default_shards feature skip. To avoid having to write our docs in a complicated way because sometimes they might be behind one shard, and sometimes they might be behind two shards we apply the default_shards feature skip to all docs tests. That is, these tests will always run with the default number of shards (one).

elasticmachine · 2018-05-11T15:48:33Z

Pinging @elastic/es-core-infra

jasontedor · 2018-05-11T15:52:36Z

FYI @elastic/es-clients for the feature skip default_shards. Note that you should not need to be concerned with this at all as the randomization to add a global template to set index.number_of_shards to 2 is done via our test runner so you will always pick up the new product default of 1. Thus, you do not need to skip tests that have this feature skip.

rjernst

LGTM, this was much simpler (fewer changes needed) than I would have anticipated!

This commit fixes a few issues in the REST client tests that arose from moving to one shard. Moving to one shard causes a reordering of calculations which impacts floating-point arithmetic. It can also impact scoring and thus the ranking of docs.

This commit fixes a SQL test with security that was expecting five shards.

s1monw

left one comment LGTM otherwise

s1monw · 2018-05-11T19:47:29Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -367,7 +367,7 @@ public ClusterState execute(ClusterState currentState) throws Exception {
                // now, put the request settings, so they override templates
                indexSettingsBuilder.put(request.settings());
                if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) {
-                    indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, 5));
+                    indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, 1));


should this be 5 if the index.version.created is < 7.0 I mean we can still have a mixed cluster here no?

@s1monw I pushed 2e89d98.

s1monw · 2018-05-12T12:00:38Z

Ship it

Let us just hard code the test to always use one shard, it is easier that way.

With this commit we set the number of shards explicitly to the default of five shards in order to keep results consistent. Relates elastic/elasticsearch#30539

* master: Deprecate not copy settings and explicitly disallow (elastic#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (elastic#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (elastic#30535) SQL: Use request flavored methods in tests (elastic#30345)

* master: Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (elastic#30530)

clintongormley · 2018-05-14T09:32:18Z

Let's make sure we blog about this to explain the reasoning.

* master: Default to one shard (#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (#30531) Moved tokenizers to analysis common module (#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) Deprecate not copy settings and explicitly disallow (#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) Delete temporary blobs before creating index file (#30528) Watcher: Remove TriggerEngine.getJobCount() (#30395) [ML] Fix wire BWC for JobUpdate (#30512) Use simpler write-once semantics for FS repository (#30435) Derive max composite buffers from max content len Use simpler write-once semantics for HDFS repository (#30439) SQL: Improve correctness of SYS COLUMNS & TYPES (#30418) Mute two tests in FlushIT with @AwaitsFix. Fix incorrect template name in test case Build: Remove legacy bwc files from xpack (#30485) Mute UnicastZenPingTests#testSimplePings with @AwaitsFix. Security: cleanup code in file stores (#30348) Security: fix TokenMetaData equals and hashcode (#30347) Mute two tests from SmokeTestWatcherWithSecurityClientYamlTestSuiteIT. Mute SharedClusterSnapshotRestoreIT#testSnapshotSucceedsAfterSnapshotFailure with @AwaitsFix. SQL: Improve compatibility with MS query (#30516) SQL: Fix parsing of dates with milliseconds (#30419)

* es/ccr: (37 commits) Default to one shard (#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (#30531) Moved tokenizers to analysis common module (#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) Deprecate not copy settings and explicitly disallow (#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) ...

* es/ccr: (37 commits) Default to one shard (elastic#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (elastic#30151) Docs: Update HighLevelRestClient migration docs (elastic#30544) Clients: Switch to new performRequest (elastic#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (elastic#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (elastic#30531) Moved tokenizers to analysis common module (elastic#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (elastic#30530) Deprecate not copy settings and explicitly disallow (elastic#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (elastic#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (elastic#30535) SQL: Use request flavored methods in tests (elastic#30345) Suppress hdfsFixture if there are spaces in the path (elastic#30302) ...

Update the default number of primary shards to match doc update work done in 4852f34 for PR elastic#30539.

Update the default number of primary shards to match doc update work done in #30539.

With this commit we set the number of shards explicitly to the default of five shards in order to keep results consistent. Relates elastic/elasticsearch#30539 Relates #44

robcowart · 2018-05-29T08:25:05Z

Overall this is a welcome change. However I was curious about why the decision for only a single shard instead of two?

We are largely unaffected here as we default to two shards in all of our index templates, and then adjust based on the customer's environment and load. We default to two because even on a single node, with a single mount point, all of our benchmarking tests show a 5-10% boost on indexing performance with two shards over only one.

Admittedly all of our use-cases are time-series oriented and heavily ingest-biased, so we are always looking to maximize ingest/index performance. Certainly most query-centric use-cases will add replicas, not shards.

Really I am just curious. I would have picked two as default, but either is much better than five.

Align with the new Elasticsearch defaults[1] for number of shards for the indices used in the metrics store. [1] elastic/elasticsearch#30539

jasontedor · 2018-08-16T22:41:43Z

@robcowart I am so sorry for the slow reply here. Thanks so much for such a thoughtful question. Briefly, we think that the benefits of keeping shard counts as low as possible will benefit more users than shipping with a default configuration that will be beneficial to high-throughout use-cases. We aim to scalable in this regard, but we have a lot of users for which a single shard will suffice to absorb their write traffic. For users that need to scale, we have many knobs they can tune which include increasing their default shard count as well as splitting indices that were created with a single shard. We encourage such users to do performance testing to ensure that the tradeoff or doubling or more their shard count is worth the performance gain.

This commit adds a migration note regarding the default number of shards changing from five to one. Relates #30539

Relates: elastic/elasticsearch#30539

jasontedor added >breaking review release highlight :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 labels May 11, 2018

rjernst approved these changes May 11, 2018

View reviewed changes

jasontedor added 2 commits May 11, 2018 14:06

Fix REST client tests

3c16228

This commit fixes a few issues in the REST client tests that arose from moving to one shard. Moving to one shard causes a reordering of calculations which impacts floating-point arithmetic. It can also impact scoring and thus the ranking of docs.

Fix SQL test expecting five shards

da5ddb3

This commit fixes a SQL test with security that was expecting five shards.

s1monw reviewed May 11, 2018

View reviewed changes

jasontedor added 2 commits May 11, 2018 16:28

Fix rank eval test

446e85b

Make default version dep.

2e89d98

jasontedor force-pushed the one-shard-to-rule-them-all branch from eacba7f to 2e89d98 Compare May 11, 2018 20:29

jasontedor added 3 commits May 11, 2018 16:34

Fix precommit

e9ec5f9

Fix reindex tests

872f73f

Fix multi-cluster test

00c32ae

jasontedor added 5 commits May 12, 2018 10:10

Simplify put template call

85429b5

Cleanup

801f7f0

Remove import

68c138c

More needed for two shard randomization

09108a1

Fix multi-cluster search test with two shards

e5b0f49

Let us just hard code the test to always use one shard, it is easier that way.

jasontedor force-pushed the one-shard-to-rule-them-all branch from c91f0b9 to e5b0f49 Compare May 13, 2018 01:10

danielmitterdorfer mentioned this pull request May 13, 2018

Set number of shards explicitly for all tracks elastic/rally-tracks#44

Merged

jasontedor added 2 commits May 13, 2018 13:30

Merge branch 'master' into one-shard-to-rule-them-all

21f87f2

* master: Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (elastic#30530)

jasontedor merged commit 4a4e3d7 into elastic:master May 14, 2018

jasontedor deleted the one-shard-to-rule-them-all branch May 14, 2018 16:22

jasontedor mentioned this pull request May 14, 2018

Add deprecation warning for default shards #30587

Merged

jasontedor mentioned this pull request May 15, 2018

Skip shard deprecation messages in REST tests #30630

Merged

atc0005 added a commit to atc0005/elasticsearch that referenced this pull request May 20, 2018

create-index: Update default primary shards count

3b3be6b

Update the default number of primary shards to match doc update work done in 4852f34 for PR elastic#30539.

This was referenced May 20, 2018

create-index: Default number of shards does not match v7 changes #30746

Closed

create-index: Update default primary shards count #30747

Merged

jasontedor pushed a commit that referenced this pull request May 20, 2018

Fix default shards count in create index docs (#30747)

7cc38ab

Update the default number of primary shards to match doc update work done in #30539.

jasontedor mentioned this pull request May 22, 2018

Simplify number of shards setting #30783

Merged

dliappis mentioned this pull request Jun 13, 2018

Specify 1 for number of shards for metrics store indices elastic/rally#520

Closed

jpountz mentioned this pull request Sep 19, 2018

Remove support for types? #15613

Closed

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

Add migration note on the number of shards

e8fe624

This commit adds a migration note regarding the default number of shards changing from five to one. Relates #30539

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

russcam added a commit to elastic/elasticsearch-net that referenced this pull request Jun 20, 2019

Update documentation for default number of shards

30417c3

Relates: elastic/elasticsearch#30539

russcam mentioned this pull request Jun 20, 2019

Update documentation for default number of shards elastic/elasticsearch-net#3840

Merged

Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Jun 20, 2019

Update documentation for default number of shards (#3840)

5d84d79

Relates: elastic/elasticsearch#30539

nvtkaszpir mentioned this pull request Feb 21, 2020

[elasticsearch] Why is default number of primary shards 1? elastic/helm-charts#443

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Default to one shard #30539

Default to one shard #30539

jasontedor commented May 11, 2018 •

edited

Loading

elasticmachine commented May 11, 2018

jasontedor commented May 11, 2018

rjernst left a comment

s1monw left a comment

s1monw May 11, 2018

jasontedor May 11, 2018

s1monw commented May 12, 2018

clintongormley commented May 14, 2018

robcowart commented May 29, 2018

jasontedor commented Aug 16, 2018

Default to one shard #30539

Default to one shard #30539

Conversation

jasontedor commented May 11, 2018 • edited Loading

elasticmachine commented May 11, 2018

jasontedor commented May 11, 2018

rjernst left a comment

Choose a reason for hiding this comment

s1monw left a comment

Choose a reason for hiding this comment

s1monw May 11, 2018

Choose a reason for hiding this comment

jasontedor May 11, 2018

Choose a reason for hiding this comment

s1monw commented May 12, 2018

clintongormley commented May 14, 2018

robcowart commented May 29, 2018

jasontedor commented Aug 16, 2018

jasontedor commented May 11, 2018 •

edited

Loading