[Segment Replication] Add setting to enable segment replication as default replication type. #6791

Rishikesh1159 · 2023-03-22T15:54:19Z

Description

This PR adds setting for users to enable segment replication as default replication strategy.

-> This setting is a static setting and can only be enable through configuration and not by REST API's.
-> When this setting is enabled it makes all new indices in cluster use segment replication as default replication type. But users can still create an index using document replication by passing replication type as DOCUMENT during index creation. As Index level settings always override cluster level settings.
-> Not only during index creation but also if users set "index.replication.type" in configuration file, as it is an index level setting it will override the new setting of default segment replication.
-> This setting is not applied for system indices. By default all system indices in opensearch will still use document replication even if this setting is enabled.
-> This setting is still behind a Segment Replication Feature flag. So, users have to still enable feature flag for this setting to work. Once feature flag is removed users can set this setting directly.

Issues Resolved

Resolves #6656

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Rishikesh1159 <[email protected]>

github-actions · 2023-03-22T16:19:10Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12777/
CommitID: 5065732
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Rishikesh1159 <[email protected]>

Poojita-Raj · 2023-03-22T16:19:29Z

...internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationDefaultIT.java

+import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
+
+@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
+public class SegmentReplicationDefaultIT extends OpenSearchIntegTestCase {


SegmentReplicationDefaultIT doesn't tell us that this file is related to the setting. I would suggest renaming it to SegmentReplicationDefaultClusterSettingIT instead.

Poojita-Raj · 2023-03-22T16:25:47Z

...internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationDefaultIT.java

+            // Verify that Segment Replication happened on the replica shard.
+            assertFalse(segmentReplicationStatsResponse.getReplicationStats().get(indexName).get(0).getReplicaStats().isEmpty());
+        }
+    }


We could add some more tests here to test the behavior described:

A test to ensure that when cluster setting for segrep is true, creating an index with docrep enabled overrides this cluster setting.

Similarly a test to ensure overriding takes place when index.replication.type is set to docrep when the segrep cluster setting is true.

A last test (optional) to check that when the feature flag is not enabled, even setting the cluster setting doesn't result in segment replication indices.

Poojita-Raj · 2023-03-22T16:29:56Z

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

@@ -282,6 +282,16 @@ public Iterator<Setting<?>> settings() {
        Property.IndexScope
    );

+    /**
+     * Used to specify SEGMENT replication type as default for all indices. By default, this is false.


Nit:

Suggested change

* Used to specify SEGMENT replication type as default for all indices. By default, this is false.

* Used to specify SEGMENT replication type as the default replication strategy for all indices in a cluster. By default, this is false.

Poojita-Raj · 2023-03-22T16:33:36Z

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

+    /**
+     * Used to specify SEGMENT replication type as default for all indices. By default, this is false.
+     */
+    public static final Setting<Boolean> DEFAULT_REPLICATION_TYPE_SETTING_SEGMENT = Setting.boolSetting(


Could be renamed to DEFAULT_SEGMENT_REPLICATION_TYPE_CLUSTER_SETTING or CLUSTER_DEFAULT_SEGMENT_REPLICATION_TYPE_SETTING to make it obvious this is a cluster level setting.

kotwanikunal · 2023-03-22T16:43:31Z

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

+     * Used to specify SEGMENT replication type as default for all indices. By default, this is false.
+     */
+    public static final Setting<Boolean> DEFAULT_REPLICATION_TYPE_SETTING_SEGMENT = Setting.boolSetting(
+        "opensearch.index.default.replication.type.segment",


I would rename it to something along the lines of indices.replication.strategy and set the default value to doc rep (preferably using an enum). The user can then set it to seg_rep as needed.
I would also move it under IndicesService similar to -

OpenSearch/server/src/main/java/org/opensearch/indices/IndicesService.java

Lines 214 to 219 in f40fa82

public static final String INDICES_SHARDS_CLOSED_TIMEOUT = "indices.shards_closed_timeout";

public static final Setting<TimeValue> INDICES_CACHE_CLEAN_INTERVAL_SETTING = Setting.positiveTimeSetting(

"indices.cache.cleanup_interval",

TimeValue.timeValueMinutes(1),

Property.NodeScope

);

github-actions · 2023-03-22T16:58:09Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12778/
CommitID: 7b3e333
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

dreamer-89 · 2023-03-22T17:54:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌

URL: https://build.ci.opensearch.org/job/gradle-check/12778/

CommitID: 7b3e333
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Looks like a legit failure.

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.indices.replication.SegmentReplicationDefaultIT.testReplicationWithDefaultSegmentReplicationSetting" -Dtests.seed=A834E113C4F1CF78 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=de-DE -Dtests.timezone=Australia/Queensland -Druntime.java=19

org.opensearch.indices.replication.SegmentReplicationDefaultIT > testReplicationWithDefaultSegmentReplicationSetting FAILED
    java.lang.AssertionError: expected null, but was:<[SegmentReplicationPerGroupStats{replicaStats=[SegmentReplicationShardStats{allocationId=s2L3gNsbQUei809hRc-pOg, checkpointsBehindCount=0, bytesBehindCount=0, currentReplicationTimeMillis=0, lastCompletedReplicationTimeMillis=45, currentReplicationState=org.opensearch.indices.replication.SegmentReplicationState@2e66ad62}], rejectedRequestCount=0}]>
        at __randomizedtesting.SeedInfo.seed([A834E113C4F1CF78:92D0663D70F53C93]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotNull(Assert.java:756)
        at org.junit.Assert.assertNull(Assert.java:738)
        at org.junit.Assert.assertNull(Assert.java:748)
        at org.opensearch.indices.replication.SegmentReplicationDefaultIT.testReplicationWithDefaultSegmentReplicationSetting(SegmentReplicationDefaultIT.java:106)

Signed-off-by: Rishikesh1159 <[email protected]>

github-actions · 2023-03-23T00:16:03Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12803/
CommitID: e0243ff
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Rishikesh1159 <[email protected]>

github-actions · 2023-03-23T00:20:14Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12805/
CommitID: 796171d
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

kotwanikunal

Added a few more comments.
Also add a CHANGELOG entry since you are introducing a new configuration.

kotwanikunal · 2023-03-23T00:16:58Z

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

                final Object replicationType = settings.get(INDEX_REPLICATION_TYPE_SETTING);
-                if (replicationType != ReplicationType.SEGMENT && value == true) {
+                if (replicationType != ReplicationType.SEGMENT


nit: use !ReplicationType.SEGMENT.equals(replicationType) for safer usage

kotwanikunal · 2023-03-23T00:19:54Z

server/src/main/java/org/opensearch/indices/IndicesService.java

+     */
+    public static final String CLUSTER_SETTING_REPLICATION_TYPE = "cluster.indices.replication.strategy";
+
+    public static final Setting<ReplicationType> CLUSTER_DEFAULT_REPLICATION_TYPE_SETTING = new Setting<>(


CLUSTER_REPLICATION_TYPE_SETTING seems more appropriate here. Default is implied by the default value that will be provided. Also would need documentation around this to demonstrate the hierarchy of CLUSTER_REPLICATION_TYPE_SETTING and INDEX_REPLICATION_TYPE_SETTING

Sure. I will update that. Yes, I will open an issue in documentation repo to document this changes once this PR is merged.

Signed-off-by: Rishikesh1159 <[email protected]>

github-actions · 2023-03-23T00:50:52Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.repositories.azure.AzureBlobContainerRetriesTests.testReadBlobWithRetries

URL: https://build.ci.opensearch.org/job/gradle-check/12806/
CommitID: b754413
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-03-23T01:12:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12810/
CommitID: 8511809
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-03-23T01:48:19Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12811/
CommitID: 8511809

github-actions · 2023-03-23T01:49:11Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12812/
CommitID: 81da78b

codecov-commenter · 2023-03-23T02:03:27Z

Codecov Report

Merging #6791 (81da78b) into main (4d78bbf) will increase coverage by 0.15%.
The diff coverage is 53.33%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6791      +/-   ##
============================================
+ Coverage     70.65%   70.80%   +0.15%     
- Complexity    59184    59240      +56     
============================================
  Files          4810     4810              
  Lines        283487   283498      +11     
  Branches      40877    40882       +5     
============================================
+ Hits         200299   200740     +441     
+ Misses        66761    66274     -487     
- Partials      16427    16484      +57

Impacted Files	Coverage Δ
.../main/java/org/opensearch/index/IndexSettings.java	`85.30% <20.00%> (-0.91%)`	⬇️
...org/opensearch/cluster/metadata/IndexMetadata.java	`84.66% <50.00%> (+0.30%)`	⬆️
...rg/opensearch/common/settings/ClusterSettings.java	`92.50% <100.00%> (+0.19%)`	⬆️
...in/java/org/opensearch/indices/IndicesService.java	`63.76% <100.00%> (-0.04%)`	⬇️

... and 527 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

kotwanikunal · 2023-03-23T05:52:32Z

server/src/main/java/org/opensearch/index/IndexSettings.java

+        if (FeatureFlags.isEnabled(FeatureFlags.REPLICATION_TYPE)
+            && indexMetadata.isSystem() == false
+            && settings.get(IndexMetadata.SETTING_REPLICATION_TYPE) == null) {
+            replicationType = ReplicationType.parseString(settings.get(IndicesService.CLUSTER_SETTING_REPLICATION_TYPE));


nit: This can be replaced by

Suggested change

replicationType = ReplicationType.parseString(settings.get(IndicesService.CLUSTER_SETTING_REPLICATION_TYPE));

replicationType = IndicesService.CLUSTER_REPLICATION_TYPE_SETTING.get(settings);

Sure, I will update this

kotwanikunal · 2023-03-23T05:53:12Z

server/src/main/java/org/opensearch/index/IndexSettings.java

+            && settings.get(IndexMetadata.SETTING_REPLICATION_TYPE) == null) {
+            replicationType = ReplicationType.parseString(settings.get(IndicesService.CLUSTER_SETTING_REPLICATION_TYPE));
+        } else {
+            replicationType = ReplicationType.parseString(settings.get(IndexMetadata.SETTING_REPLICATION_TYPE));


nit: This can be replaced by

Suggested change

replicationType = ReplicationType.parseString(settings.get(IndexMetadata.SETTING_REPLICATION_TYPE));

replicationType = IndexMetadata.INDEX_REPLICATION_TYPE_SETTING.get(settings).get(settings);

Signed-off-by: Rishikesh1159 <[email protected]>

…1159/OpenSearch into default-segrep-setting

github-actions · 2023-03-23T17:46:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12835/
CommitID: c953eb4

…fault replication type. (#6791) * Add setting to enable segment replication as default replication type. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing unit tests. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing unit test. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments. Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> (cherry picked from commit 879ade7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…fault replication type. (#6791) (#6812) * Add setting to enable segment replication as default replication type. * Fix failing unit tests. * Address comments on PR. * Address comments. * Fix failing unit test. * Address comments. * Refactoring PR. --------- (cherry picked from commit 879ade7) Signed-off-by: Rishikesh1159 <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Rishikesh1159 · 2023-03-31T16:22:58Z

Currently, when user set cluster setting "cluster.indices.replication.strategy" in opensearch.yml, all the newly created indices will use segment replication as default replication strategy. But when user tries to fetch index settings GET /<index>/_settings the response does not show replication type as SEGMENT. So this is a bug and it might be confusing for users. I am currently looking for any other workarounds to fix this issue, I will update here as soon as I find something. Any other thoughts are much appreciated please.

…fault replication type. (opensearch-project#6791) * Add setting to enable segment replication as default replication type. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing unit tests. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing unit test. Signed-off-by: Rishikesh1159 <[email protected]> * Address comments. Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> Signed-off-by: Valentin Mitrofanov <[email protected]>

Add setting to enable segment replication as default replication type.

5065732

Signed-off-by: Rishikesh1159 <[email protected]>

Rishikesh1159 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and xuezhou25 as code owners March 22, 2023 15:54

github-actions bot added the distinguished-contributor label Mar 22, 2023

Rishikesh1159 added the skip-changelog label Mar 22, 2023

Fix failing unit tests.

7b3e333

Signed-off-by: Rishikesh1159 <[email protected]>

Poojita-Raj reviewed Mar 22, 2023

View reviewed changes

kotwanikunal reviewed Mar 22, 2023

View reviewed changes

Address comments on PR.

e0243ff

Signed-off-by: Rishikesh1159 <[email protected]>

Address comments.

796171d

Signed-off-by: Rishikesh1159 <[email protected]>

Fix failing unit test.

b754413

Signed-off-by: Rishikesh1159 <[email protected]>

kotwanikunal reviewed Mar 23, 2023

View reviewed changes

Address comments.

8511809

Signed-off-by: Rishikesh1159 <[email protected]>

Rishikesh1159 removed the skip-changelog label Mar 23, 2023

Merge branch 'opensearch-project:main' into default-segrep-setting

81da78b

Poojita-Raj approved these changes Mar 23, 2023

View reviewed changes

kotwanikunal approved these changes Mar 23, 2023

View reviewed changes

Rishikesh1159 added 2 commits March 23, 2023 17:11

Refactoring PR.

15cc5f6

Signed-off-by: Rishikesh1159 <[email protected]>

Merge branch 'default-segrep-setting' of https://github.com/Rishikesh…

c953eb4

…1159/OpenSearch into default-segrep-setting

Rishikesh1159 merged commit 879ade7 into opensearch-project:main Mar 23, 2023

Rishikesh1159 added the backport 2.x Backport to 2.x branch label Mar 23, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 23, 2023

[Backport 2.x] [Segment Replication] Add setting to enable segment replication as default replication type. #6812

Merged

Rishikesh1159 mentioned this pull request Apr 3, 2023

[Segment Replication] Add new feature flag for testing new segment replication settings #6952

Closed

Rishikesh1159 mentioned this pull request Apr 5, 2023

[Segment Replication] Introduce new SEGMENT_REPLICATION_EXPERIMENTAL feature flag. #7006

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Replication] Add setting to enable segment replication as default replication type. #6791

[Segment Replication] Add setting to enable segment replication as default replication type. #6791

Rishikesh1159 commented Mar 22, 2023 •

edited

Loading

github-actions bot commented Mar 22, 2023

Poojita-Raj Mar 22, 2023

Poojita-Raj Mar 22, 2023

Poojita-Raj Mar 22, 2023

Poojita-Raj Mar 22, 2023

kotwanikunal Mar 22, 2023

github-actions bot commented Mar 22, 2023

dreamer-89 commented Mar 22, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

kotwanikunal left a comment

kotwanikunal Mar 23, 2023

kotwanikunal Mar 23, 2023

Rishikesh1159 Mar 23, 2023

github-actions bot commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

codecov-commenter commented Mar 23, 2023

kotwanikunal Mar 23, 2023

Rishikesh1159 Mar 23, 2023

kotwanikunal Mar 23, 2023

github-actions bot commented Mar 23, 2023

Rishikesh1159 commented Mar 31, 2023

	* Used to specify SEGMENT replication type as default for all indices. By default, this is false.
	* Used to specify SEGMENT replication type as the default replication strategy for all indices in a cluster. By default, this is false.

	public static final String INDICES_SHARDS_CLOSED_TIMEOUT = "indices.shards_closed_timeout";
	public static final Setting<TimeValue> INDICES_CACHE_CLEAN_INTERVAL_SETTING = Setting.positiveTimeSetting(
	"indices.cache.cleanup_interval",
	TimeValue.timeValueMinutes(1),
	Property.NodeScope
	);

	replicationType = ReplicationType.parseString(settings.get(IndicesService.CLUSTER_SETTING_REPLICATION_TYPE));
	replicationType = IndicesService.CLUSTER_REPLICATION_TYPE_SETTING.get(settings);

	replicationType = ReplicationType.parseString(settings.get(IndexMetadata.SETTING_REPLICATION_TYPE));
	replicationType = IndexMetadata.INDEX_REPLICATION_TYPE_SETTING.get(settings).get(settings);

[Segment Replication] Add setting to enable segment replication as default replication type. #6791

[Segment Replication] Add setting to enable segment replication as default replication type. #6791

Conversation

Rishikesh1159 commented Mar 22, 2023 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Mar 22, 2023

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 22, 2023

Gradle Check (Jenkins) Run Completed with:

dreamer-89 commented Mar 22, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

kotwanikunal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Mar 23, 2023

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 23, 2023

Gradle Check (Jenkins) Run Completed with:

Rishikesh1159 commented Mar 31, 2023

Rishikesh1159 commented Mar 22, 2023 •

edited

Loading