Skip to content

Commit f8ed51e

Browse files
monusingh-1monusingh-1
committed
Skip syncing number_of_replicas when follower has auto_expand_replicas active
Fixes #1661. When the follower index has `index.auto_expand_replicas` active (any value other than the literal "false"), the follower's local OpenSearch is responsible for deriving `index.number_of_replicas` from its own data-node count. CCR's 60s metadata sync was blindly copying the leader's `number_of_replicas` onto the follower, which in topologies where the leader has fewer data nodes than the follower caused a destructive cycle: 1. CCR pushes leader's lower `number_of_replicas` -> STARTED replica shards are destroyed on the follower. 2. OpenSearch's `adaptAutoExpandReplicas()` immediately corrects the count back up -> new UNASSIGNED shards enter peer recovery -> cluster goes YELLOW. 3. 60s later CCR syncs again -> destroys the recovering shards before recovery completes -> cycle repeats forever. Fix: in `IndexReplicationTask.pollForMetadata()`, after computing the desired settings from leader+overrides and before diffing against the follower, if the follower's `auto_expand_replicas` is active, strip `number_of_replicas` from BOTH the desired and follower settings so the diff loops neither add nor remove it. The leader's `auto_expand_replicas` itself continues to be synced, so once the user disables auto-expand on the leader the next sync cycle resumes syncing `number_of_replicas`. Any explicit `number_of_replicas` override set via the replication metadata is also suppressed while auto-expand is active on the follower; this is intentional because a fixed replica count and auto-expand are contradictory and the follower's active auto-expand takes precedence. Added unit tests for the two new companion-object helpers `isAutoExpandReplicasActive` and `filterOutNumberOfReplicas` covering: numeric range, "0-all", "false"/"False"/absent, key removal, no-op when absent, and empty settings.
1 parent e39a023 commit f8ed51e

3 files changed

Lines changed: 258 additions & 2 deletions

File tree

‎.sisyphus/handoff/fix-1661.diff‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
diff --git a/src/main/kotlin/org/opensearch/replication/task/index/IndexReplicationTask.kt b/src/main/kotlin/org/opensearch/replication/task/index/IndexReplicationTask.kt
2+
index 6125619..d8d8351 100644
3+
--- a/src/main/kotlin/org/opensearch/replication/task/index/IndexReplicationTask.kt
4+
+++ b/src/main/kotlin/org/opensearch/replication/task/index/IndexReplicationTask.kt
5+
@@ -166,6 +166,34 @@ open class IndexReplicationTask(id: Long, type: String, action: String, descript
6+
const val AUTOPAUSED_REASON_PREFIX = "AutoPaused: "
7+
const val TASK_CANCELLATION_REASON = AUTOPAUSED_REASON_PREFIX + "Index replication task was cancelled by user"
8+
9+
+ /**
10+
+ * Returns true if the given settings has `index.auto_expand_replicas` set to anything other
11+
+ * than "false" (the OpenSearch sentinel that disables auto-expand).
12+
+ *
13+
+ * When this returns true, CCR must NOT sync `index.number_of_replicas` from the leader to the
14+
+ * follower: the follower's local OpenSearch is responsible for computing `number_of_replicas`
15+
+ * from its own data-node count. See issue #1661.
16+
+ */
17+
+ @JvmStatic
18+
+ internal fun isAutoExpandReplicasActive(settings: Settings): Boolean {
19+
+ val value = settings.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key) ?: return false
20+
+ // OpenSearch stores the disabled sentinel as the literal string "false".
21+
+ return !value.equals("false", ignoreCase = true)
22+
+ }
23+
+
24+
+ /**
25+
+ * Returns a copy of the given settings with `index.number_of_replicas` removed. Used to
26+
+ * prevent CCR from syncing `number_of_replicas` to a follower whose auto-expand is active
27+
+ * (see issue #1661).
28+
+ */
29+
+ @JvmStatic
30+
+ internal fun filterOutNumberOfReplicas(settings: Settings): Settings {
31+
+ if (settings.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS) == null) {
32+
+ return settings
33+
+ }
34+
+ return settings.filter { k: String -> k != IndexMetadata.SETTING_NUMBER_OF_REPLICAS }
35+
+ }
36+
+
37+
}
38+
39+
//only for testing
40+
@@ -523,7 +551,22 @@ open class IndexReplicationTask(id: Long, type: String, action: String, descript
41+
}
42+
}
43+
}
44+
- val desiredSettings = desiredSettingsBuilder.build()
45+
+ var desiredSettings = desiredSettingsBuilder.build()
46+
+
47+
+ // Issue #1661: When the follower has `index.auto_expand_replicas` active (any value other
48+
+ // than "false"), the follower cluster independently manages `index.number_of_replicas` based
49+
+ // on its local node count. Syncing `number_of_replicas` from the leader in that case destroys
50+
+ // STARTED replica shards and triggers perpetual YELLOW state as `adaptAutoExpandReplicas()`
51+
+ // re-creates them only for the next sync to destroy them again.
52+
+ //
53+
+ // Strip `number_of_replicas` from BOTH desired and follower settings so the diff below
54+
+ // neither adds nor removes it. The leader's `auto_expand_replicas` value is still synced
55+
+ // through `desiredSettings`, so if the user disables auto-expand on the leader, the next
56+
+ // sync (after auto-expand is disabled on the follower) will resume syncing `number_of_replicas`.
57+
+ if (isAutoExpandReplicasActive(followerSettings)) {
58+
+ desiredSettings = filterOutNumberOfReplicas(desiredSettings)
59+
+ followerSettings = filterOutNumberOfReplicas(followerSettings)
60+
+ }
61+
62+
val changedSettingsBuilder = Settings.builder()
63+
for(key in desiredSettings.keySet()) {
64+
diff --git a/src/test/kotlin/org/opensearch/replication/task/index/IndexReplicationTaskTests.kt b/src/test/kotlin/org/opensearch/replication/task/index/IndexReplicationTaskTests.kt
65+
index f0e4f56..0b93adb 100644
66+
--- a/src/test/kotlin/org/opensearch/replication/task/index/IndexReplicationTaskTests.kt
67+
+++ b/src/test/kotlin/org/opensearch/replication/task/index/IndexReplicationTaskTests.kt
68+
@@ -386,4 +386,75 @@ class IndexReplicationTaskTests : OpenSearchTestCase() {
69+
val isMissingResult = replicationTask.isIndexClosed("non-existent-index")
70+
assertThat(isMissingResult).isTrue() // Should default to true for safety
71+
}
72+
+
73+
+ // --- Issue #1661: number_of_replicas must not be synced when follower has auto_expand_replicas active ---
74+
+
75+
+ fun testIsAutoExpandReplicasActive_trueForNumericRange() {
76+
+ val settings = Settings.builder()
77+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-1")
78+
+ .build()
79+
+ assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isTrue()
80+
+ }
81+
+
82+
+ fun testIsAutoExpandReplicasActive_trueForAllRange() {
83+
+ val settings = Settings.builder()
84+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
85+
+ .build()
86+
+ assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isTrue()
87+
+ }
88+
+
89+
+ fun testIsAutoExpandReplicasActive_falseWhenDisabled() {
90+
+ val settings = Settings.builder()
91+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "false")
92+
+ .build()
93+
+ assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
94+
+ }
95+
+
96+
+ fun testIsAutoExpandReplicasActive_falseWhenDisabledCaseInsensitive() {
97+
+ val settings = Settings.builder()
98+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "False")
99+
+ .build()
100+
+ assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
101+
+ }
102+
+
103+
+ fun testIsAutoExpandReplicasActive_falseWhenAbsent() {
104+
+ val settings = Settings.builder()
105+
+ .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "1")
106+
+ .build()
107+
+ assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
108+
+ }
109+
+
110+
+ fun testFilterOutNumberOfReplicas_removesKey() {
111+
+ val settings = Settings.builder()
112+
+ .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "1")
113+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
114+
+ .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "3")
115+
+ .build()
116+
+
117+
+ val filtered = IndexReplicationTask.filterOutNumberOfReplicas(settings)
118+
+
119+
+ assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)).isNull()
120+
+ // Other keys are preserved.
121+
+ assertThat(filtered.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key)).isEqualTo("0-all")
122+
+ assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)).isEqualTo("3")
123+
+ }
124+
+
125+
+ fun testFilterOutNumberOfReplicas_noopWhenKeyAbsent() {
126+
+ val settings = Settings.builder()
127+
+ .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
128+
+ .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "3")
129+
+ .build()
130+
+
131+
+ val filtered = IndexReplicationTask.filterOutNumberOfReplicas(settings)
132+
+
133+
+ // Same keys, same values.
134+
+ assertThat(filtered.keySet()).isEqualTo(settings.keySet())
135+
+ assertThat(filtered.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key)).isEqualTo("0-all")
136+
+ assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)).isEqualTo("3")
137+
+ }
138+
+
139+
+ fun testFilterOutNumberOfReplicas_emptySettings() {
140+
+ val filtered = IndexReplicationTask.filterOutNumberOfReplicas(Settings.EMPTY)
141+
+ assertThat(filtered.keySet()).isEmpty()
142+
+ }
143+
}
144+
\ No newline at end of file

‎src/main/kotlin/org/opensearch/replication/task/index/IndexReplicationTask.kt‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,32 @@ open class IndexReplicationTask(id: Long, type: String, action: String, descript
166166
const val AUTOPAUSED_REASON_PREFIX = "AutoPaused: "
167167
const val TASK_CANCELLATION_REASON = AUTOPAUSED_REASON_PREFIX + "Index replication task was cancelled by user"
168168

169+
/**
170+
* Returns true if the given settings has `index.auto_expand_replicas` set to anything other
171+
* than "false" (the OpenSearch sentinel that disables auto-expand).
172+
*
173+
* When this returns true, CCR must NOT sync `index.number_of_replicas` from the leader to the
174+
* follower: the follower's local OpenSearch is responsible for computing `number_of_replicas`
175+
* from its own data-node count. See issue #1661.
176+
*/
177+
internal fun isAutoExpandReplicasActive(settings: Settings): Boolean {
178+
val value = settings.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key) ?: return false
179+
// OpenSearch stores the disabled sentinel as the literal string "false".
180+
return !value.equals("false", ignoreCase = true)
181+
}
182+
183+
/**
184+
* Returns a copy of the given settings with `index.number_of_replicas` removed. Used to
185+
* prevent CCR from syncing `number_of_replicas` to a follower whose auto-expand is active
186+
* (see issue #1661).
187+
*/
188+
internal fun filterOutNumberOfReplicas(settings: Settings): Settings {
189+
if (settings.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS) == null) {
190+
return settings
191+
}
192+
return settings.filter { k: String -> k != IndexMetadata.SETTING_NUMBER_OF_REPLICAS }
193+
}
194+
169195
}
170196

171197
//only for testing
@@ -523,7 +549,22 @@ open class IndexReplicationTask(id: Long, type: String, action: String, descript
523549
}
524550
}
525551
}
526-
val desiredSettings = desiredSettingsBuilder.build()
552+
var desiredSettings = desiredSettingsBuilder.build()
553+
554+
// Issue #1661: When the follower has `index.auto_expand_replicas` active (any value other
555+
// than "false"), the follower cluster independently manages `index.number_of_replicas` based
556+
// on its local node count. Syncing `number_of_replicas` from the leader in that case destroys
557+
// STARTED replica shards and triggers perpetual YELLOW state as `adaptAutoExpandReplicas()`
558+
// re-creates them only for the next sync to destroy them again.
559+
//
560+
// Note: any explicit `number_of_replicas` the user set in the replication metadata overrides
561+
// is also suppressed here. This is intentional \u2014 `auto_expand_replicas` and a fixed
562+
// `number_of_replicas` are contradictory settings, and the follower's active auto-expand
563+
// takes precedence until it is disabled.
564+
if (isAutoExpandReplicasActive(followerSettings)) {
565+
desiredSettings = filterOutNumberOfReplicas(desiredSettings)
566+
followerSettings = filterOutNumberOfReplicas(followerSettings)
567+
}
527568

528569
val changedSettingsBuilder = Settings.builder()
529570
for(key in desiredSettings.keySet()) {

‎src/test/kotlin/org/opensearch/replication/task/index/IndexReplicationTaskTests.kt‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,4 +386,75 @@ class IndexReplicationTaskTests : OpenSearchTestCase() {
386386
val isMissingResult = replicationTask.isIndexClosed("non-existent-index")
387387
assertThat(isMissingResult).isTrue() // Should default to true for safety
388388
}
389-
}
389+
390+
// --- Issue #1661: number_of_replicas must not be synced when follower has auto_expand_replicas active ---
391+
392+
fun testIsAutoExpandReplicasActive_trueForNumericRange() {
393+
val settings = Settings.builder()
394+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-1")
395+
.build()
396+
assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isTrue()
397+
}
398+
399+
fun testIsAutoExpandReplicasActive_trueForAllRange() {
400+
val settings = Settings.builder()
401+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
402+
.build()
403+
assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isTrue()
404+
}
405+
406+
fun testIsAutoExpandReplicasActive_falseWhenDisabled() {
407+
val settings = Settings.builder()
408+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "false")
409+
.build()
410+
assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
411+
}
412+
413+
fun testIsAutoExpandReplicasActive_falseWhenDisabledCaseInsensitive() {
414+
val settings = Settings.builder()
415+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "False")
416+
.build()
417+
assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
418+
}
419+
420+
fun testIsAutoExpandReplicasActive_falseWhenAbsent() {
421+
val settings = Settings.builder()
422+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "1")
423+
.build()
424+
assertThat(IndexReplicationTask.isAutoExpandReplicasActive(settings)).isFalse()
425+
}
426+
427+
fun testFilterOutNumberOfReplicas_removesKey() {
428+
val settings = Settings.builder()
429+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "1")
430+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
431+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "3")
432+
.build()
433+
434+
val filtered = IndexReplicationTask.filterOutNumberOfReplicas(settings)
435+
436+
assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)).isNull()
437+
// Other keys are preserved.
438+
assertThat(filtered.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key)).isEqualTo("0-all")
439+
assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)).isEqualTo("3")
440+
}
441+
442+
fun testFilterOutNumberOfReplicas_noopWhenKeyAbsent() {
443+
val settings = Settings.builder()
444+
.put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key, "0-all")
445+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "3")
446+
.build()
447+
448+
val filtered = IndexReplicationTask.filterOutNumberOfReplicas(settings)
449+
450+
// Same keys, same values.
451+
assertThat(filtered.keySet()).isEqualTo(settings.keySet())
452+
assertThat(filtered.get(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.key)).isEqualTo("0-all")
453+
assertThat(filtered.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)).isEqualTo("3")
454+
}
455+
456+
fun testFilterOutNumberOfReplicas_emptySettings() {
457+
val filtered = IndexReplicationTask.filterOutNumberOfReplicas(Settings.EMPTY)
458+
assertThat(filtered.keySet()).isEmpty()
459+
}
460+
}

0 commit comments

Comments
 (0)