Skip to content

Commit f8fd6af

Browse files
talevyjasontedor
talevy
authored and
jasontedor
committed
remove requirement for shards/replicas in allocation check steps (#30855)
As we are preparing to support policy updates/changes, we noticed that restricting allocation wait steps with pinned replicas/shard counts makes this difficult to continue from. For example, as user may update or switch a policy to increase replicas. If this is done, then the check will never pass and user intervention will be required. If we simply remove this restriction, we still check that the index is allocated correctly, but without depending on the newly configured replicas setting in the policy.
1 parent 819b73c commit f8fd6af

File tree

9 files changed

+56
-211
lines changed

9 files changed

+56
-211
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ReplicasAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
7979
StepKey enoughKey = new StepKey(phase, NAME, ReplicasAllocatedStep.NAME);
8080
Settings replicaSettings = Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas).build();
8181
return Arrays.asList(new UpdateSettingsStep(updateReplicasKey, enoughKey, client, replicaSettings),
82-
new ReplicasAllocatedStep(enoughKey, nextStepKey, numberOfReplicas));
82+
new ReplicasAllocatedStep(enoughKey, nextStepKey));
8383
}
8484

8585
public int getNumberOfReplicas() {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ReplicasAllocatedStep.java‎

Lines changed: 14 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,9 @@
2121

2222
public class ReplicasAllocatedStep extends ClusterStateWaitStep {
2323
public static final String NAME = "enough-shards-allocated";
24-
private int numberReplicas;
2524

26-
public ReplicasAllocatedStep(StepKey key, StepKey nextStepKey, int numberReplicas) {
25+
public ReplicasAllocatedStep(StepKey key, StepKey nextStepKey) {
2726
super(key, nextStepKey);
28-
this.numberReplicas = numberReplicas;
29-
}
30-
31-
int getNumberReplicas() {
32-
return numberReplicas;
3327
}
3428

3529
@Override
@@ -41,73 +35,54 @@ public Result isConditionMet(Index index, ClusterState clusterState) {
4135
}
4236
// We only want to make progress if the cluster state reflects the number of replicas change and all shards are active
4337
boolean allShardsActive = ActiveShardCount.ALL.enoughShardsActive(clusterState, index.getName());
44-
boolean isConditionMet = idxMeta.getNumberOfReplicas() == numberReplicas && allShardsActive;
45-
if (isConditionMet) {
38+
if (allShardsActive) {
4639
return new Result(true, null);
4740
} else {
48-
return new Result(false, new Info(numberReplicas, idxMeta.getNumberOfReplicas(), allShardsActive));
41+
return new Result(false, new Info(idxMeta.getNumberOfReplicas(), allShardsActive));
4942
}
5043
}
51-
44+
5245
@Override
5346
public int hashCode() {
54-
return Objects.hash(super.hashCode(), numberReplicas);
47+
return super.hashCode();
5548
}
56-
49+
5750
@Override
5851
public boolean equals(Object obj) {
59-
if (obj == null) {
60-
return false;
61-
}
62-
if (getClass() != obj.getClass()) {
63-
return false;
64-
}
65-
ReplicasAllocatedStep other = (ReplicasAllocatedStep) obj;
66-
return super.equals(obj) &&
67-
Objects.equals(numberReplicas, other.numberReplicas);
52+
return obj != null && getClass() == obj.getClass() && super.equals(obj);
6853
}
69-
54+
7055
public static final class Info implements ToXContentObject {
7156

72-
private final long expectedReplicas;
7357
private final long actualReplicas;
7458
private final boolean allShardsActive;
7559
private final String message;
7660

77-
static final ParseField EXPECTED_REPLICAS = new ParseField("expected_replicas");
7861
static final ParseField ACTUAL_REPLICAS = new ParseField("actual_replicas");
7962
static final ParseField ALL_SHARDS_ACTIVE = new ParseField("all_shards_active");
8063
static final ParseField MESSAGE = new ParseField("message");
8164
static final ConstructingObjectParser<Info, Void> PARSER = new ConstructingObjectParser<>("replicas_allocated_step_info",
82-
a -> new Info((long) a[0], (long) a[1], (boolean) a[2]));
65+
a -> new Info((long) a[0], (boolean) a[1]));
8366
static {
84-
PARSER.declareLong(ConstructingObjectParser.constructorArg(), EXPECTED_REPLICAS);
8567
PARSER.declareLong(ConstructingObjectParser.constructorArg(), ACTUAL_REPLICAS);
8668
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ALL_SHARDS_ACTIVE);
8769
PARSER.declareString((i, s) -> {}, MESSAGE);
8870
}
8971

90-
public Info(long expectedReplicas, long actualReplicas, boolean allShardsActive) {
91-
this.expectedReplicas = expectedReplicas;
72+
public Info(long actualReplicas, boolean allShardsActive) {
9273
this.actualReplicas = actualReplicas;
9374
this.allShardsActive = allShardsActive;
94-
if (actualReplicas != expectedReplicas) {
95-
message = "Waiting for " + IndexMetaData.SETTING_NUMBER_OF_REPLICAS + " to be updated to " + expectedReplicas;
96-
} else if (allShardsActive == false) {
75+
if (allShardsActive == false) {
9776
message = "Waiting for all shard copies to be active";
9877
} else {
9978
message = "";
10079
}
10180
}
10281

103-
public long getExpectedReplicas() {
104-
return expectedReplicas;
105-
}
106-
10782
public long getActualReplicas() {
10883
return actualReplicas;
10984
}
110-
85+
11186
public boolean allShardsActive() {
11287
return allShardsActive;
11388
}
@@ -116,7 +91,6 @@ public boolean allShardsActive() {
11691
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
11792
builder.startObject();
11893
builder.field(MESSAGE.getPreferredName(), message);
119-
builder.field(EXPECTED_REPLICAS.getPreferredName(), expectedReplicas);
12094
builder.field(ACTUAL_REPLICAS.getPreferredName(), actualReplicas);
12195
builder.field(ALL_SHARDS_ACTIVE.getPreferredName(), allShardsActive);
12296
builder.endObject();
@@ -125,7 +99,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
12599

126100
@Override
127101
public int hashCode() {
128-
return Objects.hash(expectedReplicas, actualReplicas, allShardsActive);
102+
return Objects.hash(actualReplicas, allShardsActive);
129103
}
130104

131105
@Override
@@ -137,8 +111,7 @@ public boolean equals(Object obj) {
137111
return false;
138112
}
139113
Info other = (Info) obj;
140-
return Objects.equals(expectedReplicas, other.expectedReplicas) &&
141-
Objects.equals(actualReplicas, other.actualReplicas) &&
114+
return Objects.equals(actualReplicas, other.actualReplicas) &&
142115
Objects.equals(allShardsActive, other.allShardsActive);
143116
}
144117

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkAction.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
8686
SetSingleNodeAllocateStep setSingleNodeStep = new SetSingleNodeAllocateStep(setSingleNodeKey, allocationRoutedKey, client);
8787
AllocationRoutedStep allocationStep = new AllocationRoutedStep(allocationRoutedKey, shrinkKey, false);
8888
ShrinkStep shrink = new ShrinkStep(shrinkKey, enoughShardsKey, client, numberOfShards, SHRUNKEN_INDEX_PREFIX);
89-
ShrunkShardsAllocatedStep allocated = new ShrunkShardsAllocatedStep(enoughShardsKey, aliasKey, numberOfShards,
90-
SHRUNKEN_INDEX_PREFIX);
89+
ShrunkShardsAllocatedStep allocated = new ShrunkShardsAllocatedStep(enoughShardsKey, aliasKey, SHRUNKEN_INDEX_PREFIX);
9190
ShrinkSetAliasStep aliasSwapAndDelete = new ShrinkSetAliasStep(aliasKey, isShrunkIndexKey, client, SHRUNKEN_INDEX_PREFIX);
9291
ShrunkenIndexCheckStep waitOnShrinkTakeover = new ShrunkenIndexCheckStep(isShrunkIndexKey, nextStepKey, SHRUNKEN_INDEX_PREFIX);
9392
return Arrays.asList(setSingleNodeStep, allocationStep, shrink, allocated, aliasSwapAndDelete, waitOnShrinkTakeover);

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrunkShardsAllocatedStep.java‎

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,13 @@
1919

2020
public class ShrunkShardsAllocatedStep extends ClusterStateWaitStep {
2121
public static final String NAME = "shrunk-shards-allocated";
22-
private final int numberOfShards;
2322
private String shrunkIndexPrefix;
2423

25-
public ShrunkShardsAllocatedStep(StepKey key, StepKey nextStepKey, int numberOfShards, String shrunkIndexPrefix) {
24+
public ShrunkShardsAllocatedStep(StepKey key, StepKey nextStepKey, String shrunkIndexPrefix) {
2625
super(key, nextStepKey);
27-
this.numberOfShards = numberOfShards;
2826
this.shrunkIndexPrefix = shrunkIndexPrefix;
2927
}
3028

31-
public int getNumberOfShards() {
32-
return numberOfShards;
33-
}
34-
3529
String getShrunkIndexPrefix() {
3630
return shrunkIndexPrefix;
3731
}
@@ -42,23 +36,22 @@ public Result isConditionMet(Index index, ClusterState clusterState) {
4236
// active
4337
boolean indexExists = clusterState.metaData().index(shrunkIndexPrefix + index.getName()) != null;
4438
if (indexExists == false) {
45-
return new Result(false, new Info(false, -1, -1, false));
39+
return new Result(false, new Info(false, -1, false));
4640
}
4741
boolean allShardsActive = ActiveShardCount.ALL.enoughShardsActive(clusterState, shrunkIndexPrefix + index.getName());
4842
int numShrunkIndexShards = clusterState.metaData().index(shrunkIndexPrefix + index.getName()).getNumberOfShards();
49-
boolean isConditionMet = numShrunkIndexShards == numberOfShards && allShardsActive;
50-
if (isConditionMet) {
43+
if (allShardsActive) {
5144
return new Result(true, null);
5245
} else {
53-
return new Result(false, new Info(true, numberOfShards, numShrunkIndexShards, allShardsActive));
46+
return new Result(false, new Info(true, numShrunkIndexShards, allShardsActive));
5447
}
5548
}
56-
49+
5750
@Override
5851
public int hashCode() {
59-
return Objects.hash(super.hashCode(), numberOfShards, shrunkIndexPrefix);
52+
return Objects.hash(super.hashCode(), shrunkIndexPrefix);
6053
}
61-
54+
6255
@Override
6356
public boolean equals(Object obj) {
6457
if (obj == null) {
@@ -68,54 +61,42 @@ public boolean equals(Object obj) {
6861
return false;
6962
}
7063
ShrunkShardsAllocatedStep other = (ShrunkShardsAllocatedStep) obj;
71-
return super.equals(obj) &&
72-
Objects.equals(numberOfShards, other.numberOfShards) &&
73-
Objects.equals(shrunkIndexPrefix, other.shrunkIndexPrefix);
64+
return super.equals(obj) && Objects.equals(shrunkIndexPrefix, other.shrunkIndexPrefix);
7465
}
7566

7667
public static final class Info implements ToXContentObject {
7768

78-
private final int expectedShards;
7969
private final int actualShards;
8070
private final boolean shrunkIndexExists;
8171
private final boolean allShardsActive;
8272
private final String message;
8373

84-
static final ParseField EXPECTED_SHARDS = new ParseField("expected_shards");
8574
static final ParseField ACTUAL_SHARDS = new ParseField("actual_shards");
8675
static final ParseField SHRUNK_INDEX_EXISTS = new ParseField("shrunk_index_exists");
8776
static final ParseField ALL_SHARDS_ACTIVE = new ParseField("all_shards_active");
8877
static final ParseField MESSAGE = new ParseField("message");
8978
static final ConstructingObjectParser<Info, Void> PARSER = new ConstructingObjectParser<>("shrunk_shards_allocated_step_info",
90-
a -> new Info((boolean) a[0], (int) a[1], (int) a[2], (boolean) a[3]));
79+
a -> new Info((boolean) a[0], (int) a[1], (boolean) a[2]));
9180
static {
9281
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), SHRUNK_INDEX_EXISTS);
93-
PARSER.declareInt(ConstructingObjectParser.constructorArg(), EXPECTED_SHARDS);
9482
PARSER.declareInt(ConstructingObjectParser.constructorArg(), ACTUAL_SHARDS);
9583
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ALL_SHARDS_ACTIVE);
9684
PARSER.declareString((i, s) -> {}, MESSAGE);
9785
}
9886

99-
public Info(boolean shrunkIndexExists, int expectedShards, int actualShards, boolean allShardsActive) {
100-
this.expectedShards = expectedShards;
87+
public Info(boolean shrunkIndexExists, int actualShards, boolean allShardsActive) {
10188
this.actualShards = actualShards;
10289
this.shrunkIndexExists = shrunkIndexExists;
10390
this.allShardsActive = allShardsActive;
10491
if (shrunkIndexExists == false) {
10592
message = "Waiting for shrunk index to be created";
106-
} else if (actualShards != expectedShards) {
107-
message = "Waiting for shrunk index shards to be " + expectedShards;
10893
} else if (allShardsActive == false) {
10994
message = "Waiting for all shard copies to be active";
11095
} else {
11196
message = "";
11297
}
11398
}
11499

115-
public int getExpectedShards() {
116-
return expectedShards;
117-
}
118-
119100
public int getActualShards() {
120101
return actualShards;
121102
}
@@ -133,7 +114,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
133114
builder.startObject();
134115
builder.field(MESSAGE.getPreferredName(), message);
135116
builder.field(SHRUNK_INDEX_EXISTS.getPreferredName(), shrunkIndexExists);
136-
builder.field(EXPECTED_SHARDS.getPreferredName(), expectedShards);
137117
builder.field(ACTUAL_SHARDS.getPreferredName(), actualShards);
138118
builder.field(ALL_SHARDS_ACTIVE.getPreferredName(), allShardsActive);
139119
builder.endObject();
@@ -142,7 +122,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
142122

143123
@Override
144124
public int hashCode() {
145-
return Objects.hash(shrunkIndexExists, expectedShards, actualShards, allShardsActive);
125+
return Objects.hash(shrunkIndexExists, actualShards, allShardsActive);
146126
}
147127

148128
@Override
@@ -155,7 +135,6 @@ public boolean equals(Object obj) {
155135
}
156136
Info other = (Info) obj;
157137
return Objects.equals(shrunkIndexExists, other.shrunkIndexExists) &&
158-
Objects.equals(expectedShards, other.expectedShards) &&
159138
Objects.equals(actualShards, other.actualShards) &&
160139
Objects.equals(allShardsActive, other.allShardsActive);
161140
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ReplicasAllocatedStepInfoTests.java‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class ReplicasAllocatedStepInfoTests extends AbstractXContentTestCase<Rep
1717

1818
@Override
1919
protected Info createTestInstance() {
20-
return new Info(randomNonNegativeLong(), randomNonNegativeLong(), randomBoolean());
20+
return new Info(randomNonNegativeLong(), randomBoolean());
2121
}
2222

2323
@Override
@@ -37,27 +37,18 @@ public final void testEqualsAndHashcode() {
3737
}
3838

3939
protected final Info copyInstance(Info instance) throws IOException {
40-
return new Info(instance.getExpectedReplicas(), instance.getActualReplicas(), instance.allShardsActive());
40+
return new Info(instance.getActualReplicas(), instance.allShardsActive());
4141
}
4242

4343
protected Info mutateInstance(Info instance) throws IOException {
44-
long expectedReplicas = instance.getExpectedReplicas();
4544
long actualReplicas = instance.getActualReplicas();
4645
boolean allShardsActive = instance.allShardsActive();
47-
switch (between(0, 2)) {
48-
case 0:
49-
expectedReplicas += between(1, 20);
50-
break;
51-
case 1:
46+
if (randomBoolean()) {
5247
actualReplicas += between(1, 20);
53-
break;
54-
case 2:
48+
} else {
5549
allShardsActive = allShardsActive == false;
56-
break;
57-
default:
58-
throw new AssertionError("Illegal randomisation branch");
5950
}
60-
return new Info(expectedReplicas, actualReplicas, allShardsActive);
51+
return new Info(actualReplicas, allShardsActive);
6152
}
6253

6354
}

0 commit comments

Comments
 (0)