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

Rishikesh1159 · 2023-04-05T14:48:51Z

Description

This PR adds new SEGMENT_REPLICATION_EXPERIMENTAL feature flag. This Feature Flag should strictly be used only for testing purposes.

This PR puts cluster setting introduced behind this new SEGMENT_REPLICATION_EXPERIMENTAL feature flag.

Issues Resolved

Resolves #6952

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]>

…ready for users to use. Signed-off-by: Rishikesh1159 <[email protected]>

…h1159/OpenSearch into new-segrep-feature-flag

github-actions · 2023-04-05T15:17:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/13527/
CommitID: 2ce92f1
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-04-05T15:23:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/13528/
CommitID: ec48020
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-04-05T15:38:43Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/13532/
CommitID: cc892e4
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?

Poojita-Raj · 2023-04-05T15:17:27Z

server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java

@@ -35,6 +35,7 @@ protected FeatureFlagSettings(
        new HashSet<>(
            Arrays.asList(
                FeatureFlags.REPLICATION_TYPE_SETTING,
+                FeatureFlags.SEGMENT_REPLICATION_CLUSTER_REPLICATION_TYPE_SETTING,


This could be renamed REPLICATION_TYPE_SEGMENT_REPLICATION_CLUSTER_SETTING for readability. That way we preserve the SEGMENT_REPLICATION_CLUSTER_SETTING in the name to show users it's related.

Sure will update this in next commit.

Poojita-Raj · 2023-04-05T15:18:22Z

server/src/main/java/org/opensearch/common/util/FeatureFlags.java

@@ -27,6 +27,13 @@ public class FeatureFlags {
     */
    public static final String REPLICATION_TYPE = "opensearch.experimental.feature.replication_type.enabled";

+    /**
+     * Gates the visibility of the segment replication cluster setting that allows changing of replication type.


Suggested change

* Gates the visibility of the segment replication cluster setting that allows changing of replication type.

* Gates the visibility of the segment replication cluster setting that allows changing of default replication type in a cluster.

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

github-actions · 2023-04-05T23:14:23Z

Gradle Check (Jenkins) Run Completed with:

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

codecov-commenter · 2023-04-05T23:15:55Z

Codecov Report

Merging #7006 (20c2546) into main (59e881b) will increase coverage by 0.04%.
The diff coverage is 50.00%.

📣 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    #7006      +/-   ##
============================================
+ Coverage     70.77%   70.82%   +0.04%     
- Complexity    59270    59299      +29     
============================================
  Files          4822     4822              
  Lines        283940   283940              
  Branches      40947    40947              
============================================
+ Hits         200963   201090     +127     
+ Misses        66454    66370      -84     
+ Partials      16523    16480      -43

Impacted Files	Coverage Δ
...org/opensearch/cluster/metadata/IndexMetadata.java	`84.65% <ø> (-0.02%)`	⬇️
...rg/opensearch/common/settings/ClusterSettings.java	`92.50% <ø> (ø)`
...pensearch/common/settings/FeatureFlagSettings.java	`50.00% <ø> (ø)`
.../main/java/org/opensearch/index/IndexSettings.java	`85.30% <0.00%> (ø)`
.../java/org/opensearch/common/util/FeatureFlags.java	`90.90% <100.00%> (+0.90%)`	⬆️

... and 472 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-04-06T16:21:19Z

server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

@@ -646,7 +646,7 @@ public void apply(Settings value, Settings current, Settings previous) {
    public static final Map<String, List<Setting>> FEATURE_FLAGGED_CLUSTER_SETTINGS = Map.of(
        FeatureFlags.SEARCHABLE_SNAPSHOT,
        List.of(Node.NODE_SEARCH_CACHE_SIZE_SETTING),
-        FeatureFlags.REPLICATION_TYPE,
+        FeatureFlags.SEGMENT_REPLICATION_CLUSTER_SETTING,


I thought the goal was to reuse the FeatureFlags.REPLICATION_TYPE_SETTING since we were going to get rid of the gating mechanism in other places, and keep this one behind the existing flag.

Yes initially even I was thinking on same line, but the reason I added new feature flag is because FeatureFlags.REPLICATION_TYPE_SETTING feature flag is old, I mean many users are already aware of this feature flag, they can mistakenly enable this expecting some other behaviour and some users might get confused on why after segrep GA release they have a feature flag. In order to avoid this confusion I thought creating a new feature flag which is not popular among users and only known to few for testing purposes.

Other than avoiding confusion I don't have any strong opinion on adding new feature flag. @kotwanikunal what are your thoughts?

Seems like a good plan! Let's go ahead with the new flag to avoid any confusion.

Considering new features and enhancements that we are going to introduce in the future, I think it better to use the same feature flag, So, they are flagged as beta features and users can use same flag to test them, instead of having different flags for each enhancement. @Rishikesh1159 @kotwanikunal what do you think?

That was my original intent. An alternative solution that might work going forward is introducing a new feature flag which gates experimental features only - FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL. This will avoid the confusion that @Rishikesh1159 was talking about as well as serves our purpose going forward with new/experimental features until we release them.

Sure thanks @kotwanikunal and @anasalkouz for your thoughts. I think kunal's alternative solution here makes sense. It will make things cleaner. I will go ahead and create new FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL and we can use this for future segrep experimental features.

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

github-actions · 2023-04-06T23:11:18Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/13675/
CommitID: 20c2546

…feature flag. (#7006) * Add new SEGMENT_REPLICATION_CLUSTER_SETTING feature flag. Signed-off-by: Rishikesh1159 <[email protected]> * Remove entry from changelog as the cluster setting introduced is noy ready for users to use. 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 on PR and chnage feature flag to SEGMENT_REPLICATION_EXPERIMENTAL. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> (cherry picked from commit fe8b4d4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…feature flag. (#7006) (#7042) * Add new SEGMENT_REPLICATION_CLUSTER_SETTING feature flag. * Remove entry from changelog as the cluster setting introduced is noy ready for users to use. * Fix failing unit tests. * Address comments on PR. * Address comments on PR and chnage feature flag to SEGMENT_REPLICATION_EXPERIMENTAL. --------- (cherry picked from commit fe8b4d4) 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 and others added 2 commits April 5, 2023 07:45

Add new SEGMENT_REPLICATION_CLUSTER_SETTING feature flag.

27ee1ab

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

Merge branch 'opensearch-project:main' into new-segrep-feature-flag

2ce92f1

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 April 5, 2023 14:48

Rishikesh1159 added 2 commits April 5, 2023 14:54

Remove entry from changelog as the cluster setting introduced is noy …

50a2d63

…ready for users to use. Signed-off-by: Rishikesh1159 <[email protected]>

Merge branch 'new-segrep-feature-flag' of https://github.com/Rishikes…

ec48020

…h1159/OpenSearch into new-segrep-feature-flag

Fix failing unit tests.

cc892e4

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

Poojita-Raj reviewed Apr 5, 2023

View reviewed changes

Address comments on PR.

f816fa2

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

Poojita-Raj approved these changes Apr 6, 2023

View reviewed changes

kotwanikunal reviewed Apr 6, 2023

View reviewed changes

kotwanikunal approved these changes Apr 6, 2023

View reviewed changes

Address comments on PR and chnage feature flag to SEGMENT_REPLICATION…

20c2546

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

kotwanikunal approved these changes Apr 6, 2023

View reviewed changes

Rishikesh1159 changed the title ~~[Segment Replication] Introduce new SEGMENT_REPLICATION_CLUSTER_SETTING feature flag.~~ [Segment Replication] Introduce new SEGMENT_REPLICATION_EXPERIMENTAL feature flag. Apr 6, 2023

Rishikesh1159 added the backport 2.x Backport to 2.x branch label Apr 6, 2023

Rishikesh1159 merged commit fe8b4d4 into opensearch-project:main Apr 6, 2023

opensearch-trigger-bot bot mentioned this pull request Apr 6, 2023

[Backport 2.x] [Segment Replication] Introduce new SEGMENT_REPLICATION_EXPERIMENTAL feature flag. #7042

Merged

Rishikesh1159 mentioned this pull request Apr 6, 2023

[Segment Replication] Provide option to turn on Segment Replication as default replication strategy. #6656

Closed

Rishikesh1159 mentioned this pull request Apr 18, 2023

Update 2.7.0 segment-replication manifest file with new feature flag opensearch-project/opensearch-build#3416

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Rishikesh1159 commented Apr 5, 2023 •

edited

Loading

github-actions bot commented Apr 5, 2023

github-actions bot commented Apr 5, 2023

github-actions bot commented Apr 5, 2023

Poojita-Raj Apr 5, 2023

Rishikesh1159 Apr 5, 2023

Poojita-Raj Apr 5, 2023

github-actions bot commented Apr 5, 2023

codecov-commenter commented Apr 5, 2023 •

edited

Loading

kotwanikunal Apr 6, 2023

Rishikesh1159 Apr 6, 2023

kotwanikunal Apr 6, 2023

anasalkouz Apr 6, 2023 •

edited

Loading

kotwanikunal Apr 6, 2023

Rishikesh1159 Apr 6, 2023

github-actions bot commented Apr 6, 2023

	* Gates the visibility of the segment replication cluster setting that allows changing of replication type.
	* Gates the visibility of the segment replication cluster setting that allows changing of default replication type in a cluster.

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

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

Conversation

Rishikesh1159 commented Apr 5, 2023 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Apr 5, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Apr 5, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Apr 5, 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

github-actions bot commented Apr 5, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Apr 5, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anasalkouz Apr 6, 2023 • edited Loading

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 Apr 6, 2023

Gradle Check (Jenkins) Run Completed with:

Rishikesh1159 commented Apr 5, 2023 •

edited

Loading

codecov-commenter commented Apr 5, 2023 •

edited

Loading

anasalkouz Apr 6, 2023 •

edited

Loading