[Segment Replication] Rolling upgrade support for default codecs #7698

Poojita-Raj · 2023-05-23T16:58:42Z

Description

Supports rolling upgrade for default codecs

while cluster is in mixed cluster state, primary downgrades the codec it uses to one that matches the cluster min node version
once full cluster is upgraded, primary resets the lucene codec it uses to write segments to latest one

Related Issues

Resolves #7349

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.

github-actions · 2023-05-23T17:08:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16066/
CommitID: 1aec373
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-05-23T17:23:35Z

Gradle Check (Jenkins) Run Completed with:

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

server/src/main/java/org/opensearch/index/engine/NRTReplicationEngineFactory.java

Bukhtawar · 2023-05-23T17:31:54Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationUpgradeListener.java

+        try {
+            if (indexShardList.isEmpty() == false) {
+                for (IndexShard is : indexShardList) {
+                    is.resetEngineToGlobalCheckpoint();
+                }
+            }
+        } catch (Exception e) {
+            logger.error("Received unexpected exception: [{}]", e.getMessage());
+        }


Will this cause disruptions during upgrades?

Throughput will be impacted but we will still queue incoming requests that come in while the switch of index writer is taking place and process them when it's back up.

Is there a test around this that can confirm the same? Can we run some benchmarks/test to see the impact on performance

Ran 2 benchmarks to confirm on nyc_taxis dataset - saw a 0% error rate and a 0.01% error rate on indexing respectively.

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationUpgradeListener.java

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

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationUpgradeListener.java

server/src/main/java/org/opensearch/index/codec/CodecService.java

server/src/main/java/org/opensearch/index/engine/EngineConfig.java

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationUpgradeListener.java

server/src/main/java/org/opensearch/index/engine/NRTReplicationEngineFactory.java

server/src/main/java/org/opensearch/index/engine/InternalEngine.java

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTargetService.java

github-actions · 2023-05-24T23:53:55Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16256/
CommitID: fac5d45
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-05-25T00:05:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16257/
CommitID: 9149046
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-05-25T15:59:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16303/
CommitID: 71f1803
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-05-25T23:27:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16334/
CommitID: a563655
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-05-30T07:43:20Z

Gradle Check (Jenkins) Run Completed with:

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

Bukhtawar · 2023-05-30T16:43:06Z

server/src/main/java/org/opensearch/cluster/ClusterChangedEvent.java

+    /**
+     * Returns <code>true</code> if a version upgrade has taken place in the cluster
+     */
+    public boolean clusterUpgraded() {


Rename to something better maybe hasMixedVersionNodes

We're using this method to check that cluster upgrade has been completed - it checks if it used to have mixed version nodes and current state does not. hasMixedVersionNodes might be misleading in this case.

clusterUpgraded is equivalent to NOT hasMixedVersionNodes.

server/src/main/java/org/opensearch/index/codec/CodecService.java

github-actions · 2023-05-30T20:08:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16564/
CommitID: 93aecc0
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-05-31T00:46:54Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16565/
CommitID: cddd4f0
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: Poojita Raj <[email protected]>

github-actions · 2023-05-31T18:13:11Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16590/
CommitID: 8745d50
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-06-06T18:03:20Z

server/src/main/java/org/opensearch/index/codec/CodecService.java

+        versionStringMap.put(Version.fromString("3.0.0"), "Lucene95");
+        versionStringMap.put(Version.fromString("2.8.0"), "Lucene95");
+        versionStringMap.put(Version.fromString("2.7.1"), "Lucene95");
+        versionStringMap.put(Version.fromString("2.7.0"), "Lucene95");


nit: Rather than having specific call, we can static initialize this map. This is due to the fact we are calling this inside class ctor, I don't see advantage of lazy loading.

public static final Map<Version, String> opensearchVersionToLuceneCodec; static { Map<Version, String> versionStringMap = new HashMap<>(); versionStringMap.put(Version.fromString("3.0.0"), "Lucene95"); ... opensearchVersionToLuceneCodec = Collections.unmodifiableMap(new HashMap<>(versionStringMap)); }

Can we build this map reading in Version.java, as this info is present there. This will prevent future maintenance of version <-> lucene codec map. I know this is not straightforward as Lucene version bumps doesn't necessarily mean codec bumps. We can take this in follow up PR.

dreamer-89 · 2023-06-06T18:05:33Z

server/src/main/java/org/opensearch/indices/replication/checkpoint/ReplicationCheckpoint.java

@@ -71,6 +82,11 @@ public ReplicationCheckpoint(StreamInput in) throws IOException {
            length = 0L;
            codec = null;
        }
+        if (in.getVersion().onOrAfter(Version.V_2_8_0)) {


For main branch (this PR). This needs to be changed to 3.0.0 or else this will break bwc tests (if any is exercising this code) because this field is not yet present in 2.x 2.9.0 branch. Reading from or sending to, this field to 2.9.0 node will fail.

On 2.x backport, change this back to 2.9.0.

Additional Step/PR. Change main to use 2.9.0 after PR in step 2 is merged.

dreamer-89 · 2023-06-06T18:25:10Z

server/src/main/java/org/opensearch/index/codec/CodecService.java

@@ -58,8 +61,11 @@ public class CodecService {
    public static final String BEST_COMPRESSION_CODEC = "best_compression";
    /** the raw unfiltered lucene default. useful for testing */
    public static final String LUCENE_DEFAULT_CODEC = "lucene_default";
+    static Map<Version, String> versionStringMap = new HashMap<>();


nit: This variable declaration can go inside loadMap() as it is only used to init opensearchVersionToLuceneCodec. It doesn't need to be static

dreamer-89 · 2023-06-06T18:39:33Z

server/src/main/java/org/opensearch/index/codec/CodecService.java

@@ -58,8 +61,11 @@ public class CodecService {
    public static final String BEST_COMPRESSION_CODEC = "best_compression";
    /** the raw unfiltered lucene default. useful for testing */
    public static final String LUCENE_DEFAULT_CODEC = "lucene_default";
+    static Map<Version, String> versionStringMap = new HashMap<>();
+    public static Map<Version, String> opensearchVersionToLuceneCodec;


nit: opensearchVersionToLuceneCodec -> versionToCodecMap. There are integrations which overrides Lucene codecs.

This variable can be scoped protected that still allows integrations overriding CodecService to provide their own mapping

dreamer-89 · 2023-06-06T19:25:26Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationSourceService.java

@@ -170,6 +174,33 @@ public void clusterChanged(ClusterChangedEvent event) {
                }
            }
        }
+        if (event.clusterUpgraded()) {
+            List<IndexShard> indexShardList = new ArrayList<>();


nit: final ?

dreamer-89 · 2023-06-06T19:51:03Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationSourceService.java

+                    for (IndexShard indexShard : indexService) {
+                        try {
+                            if (indexShard.routingEntry().primary()
+                                && (indexShard.getEngine().config().getClusterMinVersion() != nodes.getMaxNodeVersion())) {


For large clusters (100s of nodes), it is not uncommon to have few nodes running on older OS version, which means running primary shard in bwc for extended period, in worst case forever. I am not sure about the end result of the state. As an improvement, can this switch be performed when nodes containing all shard copies are upgraded.

Performing this engine switch gradually also make more sense versus do it all at once. The user may see indexing requests getting piled up, when upgrade completes.

Need tests.

dreamer-89 · 2023-06-06T19:57:51Z

server/src/main/java/org/opensearch/indices/replication/checkpoint/ReplicationCheckpoint.java

@@ -131,6 +154,9 @@ public void writeTo(StreamOutput out) throws IOException {
            out.writeLong(length);
            out.writeString(codec);
        }
+        if (out.getVersion().onOrAfter(Version.V_2_8_0)) {


Same as above.

dreamer-89 · 2023-06-06T20:11:22Z

Thanks @Poojita-Raj for working on this. Few top level comments.

Lucene major version upgrades

I think Lucene does not allow wiring in previous major version codecs with IndexWriter. For e.g. I see using Lucene87 during index creation results in failures during indexing operations (test code link) when running on 3.0.0 OS version using Lucene95. This can be a problem during Lucene major version upgrades i.e. 8.x -> 9.x. Tests is the best way to verify but at this point don't see a way.

Older codecs provided during index creation

Today, we allow users to provide older codec names as is during index creation. e.g

    "index": {
      "number_of_shards": 1,
      "number_of_replicas": 1,
      "replication.type": "SEGMENT",
      "codec": "Lucene90"
    }
  }

For existing indices, it is posible that node on specific OS version can have lucene codec which does not conform to mappings we have defined inside CodecService.java. I don't see this as a problem because we are always loading codec (latest) which should still be able to read/write with older codec. We can add test to verify this.
New index creation in mixed version cluster. With replica assignment not allowed on older OS version nodes, there is no need to run in bwc mode.

dreamer-89 · 2023-06-07T01:43:25Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationSourceService.java

+            try {
+                if (indexShardList.isEmpty() == false) {
+                    for (IndexShard indexShard : indexShardList) {
+                        indexShard.resetEngine();


Engine reset is not required when there is no codec change. This change will unnecessarily impact end users post upgrade (delay operations) when it is not really needed.

dreamer-89 · 2023-06-07T19:18:48Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTargetService.java

+            Version localNodeVersion = Version.CURRENT;
+            // if replica's OS version is not on or after primary version, then can ignore checkpoint
+            if (localNodeVersion.onOrAfter(receivedCheckpoint.getMinVersion()) == false) {
+                logger.trace(
+                    () -> new ParameterizedMessage(
+                        "Ignoring checkpoint, shard not started {} {}\n Shard does not support the received lucene codec version {}",
+                        receivedCheckpoint,
+                        replicaShard.state(),
+                        receivedCheckpoint.getCodec()
+                    )
+                );
+                return;
+            }


This check should go inside shouldProcessCheckpoint containing other validations around processing checkpoint.

dreamer-89 · 2023-06-07T19:22:16Z

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTargetService.java

+                    () -> new ParameterizedMessage(
+                        "Ignoring checkpoint, shard not started {} {}\n Shard does not support the received lucene codec version {}",
+                        receivedCheckpoint,
+                        replicaShard.state(),
+                        receivedCheckpoint.getCodec()


Suggested change

() -> new ParameterizedMessage(

"Ignoring checkpoint, shard not started {} {}\n Shard does not support the received lucene codec version {}",

receivedCheckpoint,

replicaShard.state(),

receivedCheckpoint.getCodec()

() -> new ParameterizedMessage(

"Ignoring checkpoint {} as shard does not support the received lucene codec version {}",

receivedCheckpoint,

receivedCheckpoint.getCodec()

dreamer-89 · 2023-06-07T22:33:42Z

Lucene major version upgrades

I think Lucene does not allow wiring in previous major version codecs with IndexWriter. For e.g. I see using Lucene87 during index creation results in failures during indexing operations (test code link) when running on 3.0.0 OS version using Lucene95. This can be a problem during Lucene major version upgrades i.e. 8.x -> 9.x. Tests is the best way to verify but at this point don't see a way.

Verified that using previous major latest lucene codecs is not allowed and any indexing operation fails with UnsupportedOperationException. Verified this on different version 9x version of lucenes, using latest 8x i.e. Lucene87 codec as mentioned below.

Lucene95 - latest main
Lucene92 5358502
Lucene90 006c832

Step 1. Create index with older lucene index using current lucene version 9x (any of above 3)

{
    "settings": {
        "index": {
            "number_of_shards": 1,
            "number_of_replicas": 1,
            "codec": "Lucene87"
        }
    }
}

Step 2. Index operation

{
    "error": {
        "root_cause": [
            {
                "type": "unsupported_operation_exception",
                "reason": "Old codecs may only be used for reading"
            }
        ],
        "type": "unsupported_operation_exception",
        "reason": "Old codecs may only be used for reading"
    },
    "status": 500
}

It appears lucene only allows codecs which are part of core lucene library and older/bwc codecs are only meant for reading the older segments.

Poojita-Raj · 2023-06-15T21:36:33Z

Closing until a decision is made on what approach to take with rolling upgrades with segment replication enabled.

Poojita-Raj requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng and VachaShah as code owners May 23, 2023 16:58

Bukhtawar reviewed May 23, 2023

View reviewed changes

server/src/main/java/org/opensearch/index/engine/NRTReplicationEngineFactory.java Outdated Show resolved Hide resolved

Bukhtawar reviewed May 23, 2023

View reviewed changes

server/src/main/java/org/opensearch/indices/replication/SegmentReplicationUpgradeListener.java Outdated Show resolved Hide resolved

mch2 reviewed May 24, 2023

View reviewed changes

Poojita-Raj requested a review from dbwiddis as a code owner May 24, 2023 23:46

Poojita-Raj force-pushed the bwcDowngrade branch from fac5d45 to 9149046 Compare May 24, 2023 23:58

Poojita-Raj force-pushed the bwcDowngrade branch from 9149046 to 71f1803 Compare May 25, 2023 15:52

Bukhtawar reviewed May 30, 2023

View reviewed changes

server/src/main/java/org/opensearch/index/codec/CodecService.java Outdated Show resolved Hide resolved

Poojita-Raj force-pushed the bwcDowngrade branch from c1d2678 to 93aecc0 Compare May 30, 2023 19:43

Poojita-Raj force-pushed the bwcDowngrade branch from 93aecc0 to cddd4f0 Compare May 30, 2023 23:49

Poojita-Raj added 4 commits May 31, 2023 10:08

rolling upgrade

137177c

Signed-off-by: Poojita Raj <[email protected]>

spotless check

21f7a75

Signed-off-by: Poojita Raj <[email protected]>

refactoring

1727637

Signed-off-by: Poojita Raj <[email protected]>

add unit tests

8745d50

Signed-off-by: Poojita Raj <[email protected]>

Poojita-Raj force-pushed the bwcDowngrade branch from cddd4f0 to 8745d50 Compare May 31, 2023 17:08

dreamer-89 reviewed Jun 6, 2023

View reviewed changes

dreamer-89 mentioned this pull request Jun 6, 2023

[Segment Replication] Add lucene codec tests #7903

Merged

6 tasks

dreamer-89 requested changes Jun 7, 2023

View reviewed changes

dreamer-89 reviewed Jun 7, 2023

View reviewed changes

Poojita-Raj mentioned this pull request Jun 7, 2023

[Segment Replication] Support for custom codecs #7668

Closed

dreamer-89 mentioned this pull request Jun 8, 2023

[Segment Replication] Support for mixed cluster versions (Rolling Upgrade) #3881

Closed

Poojita-Raj closed this Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Replication] Rolling upgrade support for default codecs #7698

[Segment Replication] Rolling upgrade support for default codecs #7698

Poojita-Raj commented May 23, 2023

github-actions bot commented May 23, 2023

github-actions bot commented May 23, 2023

Bukhtawar May 23, 2023

Poojita-Raj May 23, 2023

Bukhtawar May 31, 2023

Poojita-Raj Jun 7, 2023

github-actions bot commented May 24, 2023

github-actions bot commented May 25, 2023

github-actions bot commented May 25, 2023

github-actions bot commented May 25, 2023

github-actions bot commented May 30, 2023

Bukhtawar May 30, 2023

Poojita-Raj May 30, 2023

Bukhtawar May 31, 2023

github-actions bot commented May 30, 2023

github-actions bot commented May 31, 2023

github-actions bot commented May 31, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023

dreamer-89 Jun 6, 2023 •

edited

Loading

dreamer-89 Jun 6, 2023

dreamer-89 commented Jun 6, 2023 •

edited

Loading

dreamer-89 Jun 7, 2023 •

edited

Loading

dreamer-89 Jun 7, 2023

dreamer-89 Jun 7, 2023

dreamer-89 commented Jun 7, 2023 •

edited

Loading

Lucene major version upgrades

Poojita-Raj commented Jun 15, 2023

[Segment Replication] Rolling upgrade support for default codecs #7698

[Segment Replication] Rolling upgrade support for default codecs #7698

Conversation

Poojita-Raj commented May 23, 2023

Description

Related Issues

Check List

github-actions bot commented May 23, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 23, 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

github-actions bot commented May 24, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 25, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 25, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 25, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 30, 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 May 30, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 31, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented May 31, 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

Choose a reason for hiding this comment

dreamer-89 Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreamer-89 commented Jun 6, 2023 • edited Loading

Lucene major version upgrades

Older codecs provided during index creation

dreamer-89 Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreamer-89 commented Jun 7, 2023 • edited Loading

Lucene major version upgrades

Poojita-Raj commented Jun 15, 2023

dreamer-89 Jun 6, 2023 •

edited

Loading

dreamer-89 commented Jun 6, 2023 •

edited

Loading

dreamer-89 Jun 7, 2023 •

edited

Loading

dreamer-89 commented Jun 7, 2023 •

edited

Loading