Allow `_shrink` to N shards if source shards is a multiple of N #18699

s1monw · 2016-06-02T12:56:56Z

Today we allow to shrink to 1 shard but that might not be possible due to
too many documents or a single shard doesn't meet the requirements for the index.
The logic can be expanded to N shards if the source index shards is a multiple of N.
This guarantees that there are not hotspots created due to different number of shards
being shrunk into one.

There is still some work to do on the documentation end etc. but I wanted to get it out there to get initial feedback if we should do it at all.

jpountz · 2016-06-02T15:39:36Z

core/src/main/java/org/elasticsearch/action/admin/indices/shrink/TransportShrinkAction.java

+                if (docsStats != null) {
+                    count += docsStats.getCount();
+                }
+                if (count >= IndexWriter.MAX_DOCS) {


should it be count > IndexWriter.MAX_DOCS?

yeah true :)

jpountz · 2016-06-02T16:42:25Z

This is not the part of the codebase I'm most familiar with, but it looks good.

s1monw · 2016-06-03T09:00:07Z

this is ready for review, @clintongormley I messed with docs you might wanna make sure the art is not damaged

clintongormley · 2016-06-03T11:39:45Z

docs/reference/indices/shrink-index.asciidoc

-every shard in the index must be present on the same node.
+with fewer primary shards. The number of primary shards in the target index
+must be a factor of the shards in the source index. For example an index with
+`8` primary shards can be shrunk into `4`, `2` or `1` primary shard or an index


or 1 primary shardS

clintongormley · 2016-06-03T11:42:30Z

@s1monw couple of small comments, otherwise LGTM

ywelsch · 2016-06-03T12:51:34Z

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

@@ -299,15 +297,20 @@ public ClusterState execute(ClusterState currentState) throws Exception {

                            indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());
                            final Index shrinkFromIndex = request.shrinkFrom();
+                            int routingFactor = 1;


ywelsch · 2016-06-03T13:35:37Z

Can you also update the estimate shard size for shrinked indices (#18659)? I was also wondering if we can relax the condition to have all shards active on one node... i.e. reduce it to the ones that are needed for local recovery (selectShrinkShards for the shard that is being recovered).

s1monw · 2016-06-03T14:19:52Z

Can you also update the estimate shard size for shrinked indices (#18659)? I was also wondering if we can relax the condition to have all shards active on one node... i.e. reduce it to the ones that are needed for local recovery (selectShrinkShards for the shard that is being recovered).

all of those should be done in a follow up. I am currently not very in favor to reduce that limitation just now

ywelsch · 2016-06-03T14:25:02Z

ok, Iet's do a follow-up for estimated shard size then. LGTM

s1monw · 2016-06-03T21:30:47Z

ok, Iet's do a follow-up for estimated shard size then. LGTM

@ywelsch I misunderstood, I agree we should fix the estimated shard size here - the node restriction should be a followup. I will push a new commit later or next week

s1monw · 2016-06-06T13:52:32Z

@ywelsch I pushed a new commit

ywelsch · 2016-06-06T14:43:21Z

...java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java

@@ -298,17 +298,22 @@ public void testSizeShrinkIndex() {
        ImmutableOpenMap.Builder<String, Long> shardSizes = ImmutableOpenMap.builder();
        shardSizes.put("[test][0][p]", 10L);
        shardSizes.put("[test][1][p]", 100L);
-        shardSizes.put("[test][2][p]", 1000L);
+        shardSizes.put("[test][2][p]", 500L);
+        shardSizes.put("[test][3][p]", 500l);


I think checkstyle will fail with the lowercase L.

picky bastard that checkstyle

ywelsch · 2016-06-06T14:45:19Z

LGTM. Thanks @s1monw

Today we allow to shrink to 1 shard but that might not be possible due to too many documnent or a single shard doesn't meet the requirements for the index. The logic can be expanded to N shards if the source index shards is a mutiple of N. This guarantees that there are not hotspots created due to different number of shards being shrunk into one.

The `_routing` metadata field docs currently include formulas for how Elasticsearch routes documents to shards. However, these formulas were not updated for #18699. This updates the routing formulas and adds xrefs for related settings. Closes #76072

The `_routing` metadata field docs currently include formulas for how Elasticsearch routes documents to shards. However, these formulas were not updated for elastic#18699. This updates the routing formulas and adds xrefs for related settings. Closes elastic#76072

The `_routing` metadata field docs currently include formulas for how Elasticsearch routes documents to shards. However, these formulas were not updated for #18699. This updates the routing formulas and adds xrefs for related settings. Closes #76072 Co-authored-by: James Rodewig <[email protected]>

s1monw added >enhancement review WIP v5.0.0-alpha4 labels Jun 2, 2016

jpountz reviewed Jun 2, 2016
View reviewed changes

s1monw force-pushed the shrink_to_N branch from 7668406 to 14132b2 Compare June 3, 2016 08:58

s1monw removed the WIP label Jun 3, 2016

clintongormley reviewed Jun 3, 2016
View reviewed changes

ywelsch reviewed Jun 3, 2016
View reviewed changes

ywelsch reviewed Jun 6, 2016
View reviewed changes

s1monw added 5 commits June 7, 2016 10:02

add javadocs and simplify code

e9d36b4

fix docs

69b346c

qualify by class

63b2e00

add additional tests for IndexMetaData

4e92654

s1monw added 4 commits June 7, 2016 10:02

apply feedback

6cbad64

fix expected shard size calculation for shard shrinking

bb91175

make checkstyle happy

c4061fb

add extra ensureGreen()

c7f7cdb

s1monw force-pushed the shrink_to_N branch from 454fa7d to c7f7cdb Compare June 7, 2016 08:04

s1monw merged commit b2c4c32 into elastic:master Jun 7, 2016

lcawl added :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. and removed :Allocation labels Feb 13, 2018

jrodewig mentioned this pull request Aug 8, 2021

[DOCS] Update routing formulas #76203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `_shrink` to N shards if source shards is a multiple of N #18699

Allow `_shrink` to N shards if source shards is a multiple of N #18699

s1monw commented Jun 2, 2016

jpountz Jun 2, 2016

s1monw Jun 2, 2016

jpountz commented Jun 2, 2016

s1monw commented Jun 3, 2016

clintongormley Jun 3, 2016

clintongormley commented Jun 3, 2016

ywelsch Jun 3, 2016

ywelsch commented Jun 3, 2016

s1monw commented Jun 3, 2016

ywelsch commented Jun 3, 2016

s1monw commented Jun 3, 2016

s1monw commented Jun 6, 2016

ywelsch Jun 6, 2016

s1monw Jun 6, 2016

ywelsch commented Jun 6, 2016

Allow _shrink to N shards if source shards is a multiple of N #18699

Allow _shrink to N shards if source shards is a multiple of N #18699

Conversation

s1monw commented Jun 2, 2016

jpountz Jun 2, 2016

Choose a reason for hiding this comment

s1monw Jun 2, 2016

Choose a reason for hiding this comment

jpountz commented Jun 2, 2016

s1monw commented Jun 3, 2016

clintongormley Jun 3, 2016

Choose a reason for hiding this comment

clintongormley commented Jun 3, 2016

ywelsch Jun 3, 2016

Choose a reason for hiding this comment

ywelsch commented Jun 3, 2016

s1monw commented Jun 3, 2016

ywelsch commented Jun 3, 2016

s1monw commented Jun 3, 2016

s1monw commented Jun 6, 2016

ywelsch Jun 6, 2016

Choose a reason for hiding this comment

s1monw Jun 6, 2016

Choose a reason for hiding this comment

ywelsch commented Jun 6, 2016

Allow `_shrink` to N shards if source shards is a multiple of N #18699

Allow `_shrink` to N shards if source shards is a multiple of N #18699