Support for ES6 using indexes created in ES5 #1253

peternied · 2025-01-28T15:58:57Z

Description

Support loading both Lucene 7 & 9 inside of RFS using shadow jar

Issues Resolved

Resolves [BUG] RFS Fails to read ES 5 created indices on ES 6 Snapshot #1247
Resolves MIGRATIONS-2370 Support for ES5 created indices on ES6 cluster

Check List

New functionality includes testing
~~Public documentation issue/PR created, if applicable.~~

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: Peter Nied <[email protected]>

This reverts commit 365ba5d. Signed-off-by: Peter Nied <[email protected]>

Signed-off-by: Peter Nied <[email protected]>

RFS/build.gradle

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader7.java

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

build.gradle

Signed-off-by: Peter Nied <[email protected]>

chelma

Thanks for the work on this! Have a few questions.

RFS/src/main/java/org/opensearch/migrations/bulkload/common/ByteArrayIndexInput.java

RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClientFactory.java

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader.java

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

build.gradle

chelma · 2025-01-28T22:53:49Z

@peternied Oh, also - do we have a test to confirm this enables us to read Lucene 6 segments? I didn't see one, and don't want to repeat the last time we had a PR claiming to solve this issue. 😛

peternied · 2025-01-28T22:57:45Z

@chelma The EndToEnd tests cover this scenario by using RFS with a ES5 cluster, I confirmed by disabling the Lucene7 reader and got the same error when it was reported.

Signed-off-by: Peter Nied <[email protected]>

RFS/src/main/java/org/opensearch/migrations/bulkload/common/ByteArrayIndexInput.java

RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClientFactory.java

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader.java

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

Signed-off-by: Peter Nied <[email protected]>

codecov · 2025-01-29T18:08:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.03%. Comparing base (d527b87) to head (5c22580).
Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1253      +/-   ##
============================================
- Coverage     80.16%   80.03%   -0.13%     
- Complexity     3110     3127      +17     
============================================
  Files           430      432       +2     
  Lines         15903    16005     +102     
  Branches       1082     1089       +7     
============================================
+ Hits          12749    12810      +61     
- Misses         2493     2528      +35     
- Partials        661      667       +6

Flag	Coverage Δ
unittests	`80.03% <ø> (-0.13%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chelma

There's a couple new SonarQube violations in the new/modified code; resolve those and I'll approve!

build.gradle

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

RFS/src/test/java/org/opensearch/migrations/bulkload/PerformanceVerificationTest.java

.../main/java/org/opensearch/migrations/bulkload/version_es_7_10/ShardMetadataData_ES_7_10.java

Signed-off-by: Peter Nied <[email protected]>

chelma

Ship it! Thanks Peter.

Checking allocations on the test, previously it was using between 400-480mb of memory while running, add additional jars for lucene7/9 and we are very close to running out of memory constantly. I've dialed down the document to 10mb - still chunky, but with all the testing and duplication representations the test now takes up ~180mb of memory so we should be clear. Signed-off-by: Peter Nied <[email protected]>

Signed-off-by: Peter Nied <[email protected]>

gregschohn

Thanks for doing this. I recognize that this is merged, but I think that a followup could address some of these minor issues.

gregschohn · 2025-01-30T02:22:26Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

+     * happens is that the document is marked as "deleted" in the Lucene Index, but it is still present in the Lucene segment
+     * on disk.  The next time a merge occurs, that segment will be deleted, and the deleted documents in it are thereby
+     * removed from the Lucene Index.  A similar thing happens when a document is updated; the old document is marked as
+     * "deleted" in the Lucene segment and the new version of the document is added in a new Lucene segment.  Until a merge


Can you please clarify the paradox between a segment being immutable and that the segment has a document marked as deleted? I haven't seen great explanations of the exact mechanics on how the tombstone bitmaps are updated.

gregschohn · 2025-01-30T02:32:02Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

+     * document might still exist in the Lucene segments on disk, all of which have the same _id (from the ES/OS perspective).
+     *
+     * Additionally, Elasticsearch 7 introduced a feature called "soft deletes" which allows you to mark a document as
+     * "deleted" in the Lucene Index without actually removing it from the Lucene Index.  From what I can gather, soft deletes


Instead of "From what I gather" - you can reference the link below, as you do, and just say that it's used for certain shard synchronization or management issues, then cite the link. Farther below, it's worth calling out that soft-deleted docs WILL be eventually FULLY-deleted lucene docs & that's why we don't want to load them onto the target.

gregschohn · 2025-01-30T02:33:30Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

+     * 1. We make sure we use the latest Lucene commit point on the Lucene Index.  A commit is a Lucene abstraction that
+     *     comprises a consistent, point-in-time view of the Segments in the Lucene Index.  By default, a DirectoryReader
+     *     will use the latest commit point.
+     * 2. We use a concept called "liveDocs" to determine if a document is "live" or not.  The liveDocs are a bitset that


Is this a bitset or compressed bitset?

gregschohn · 2025-01-30T02:36:01Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

+     * cluster.
+     *
+     * In order to retrieve only those documents that would be considered "live" in ES/OS, we use a few tricks:
+     * 1. We make sure we use the latest Lucene commit point on the Lucene Index.  A commit is a Lucene abstraction that


If there were two snapshots to a repo - a full one that successfully finished and a partial one that didn't, would this be true? Could we get the latest commit point for some shards in an index from the second snapshot and snapshots further back in time from the first snapshot for other shards?

gregschohn · 2025-01-30T02:37:10Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

+     * Just like deleted documents and old versions of updated documents, we don't want to reindex them agaisnt the target
+     * cluster.
+     *
+     * In order to retrieve only those documents that would be considered "live" in ES/OS, we use a few tricks:


s/tricks/strategies/ - or something else like algorithms. Tricks imply that there's something non-obvious or maybe something that's only true in certain cases.

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java

gregschohn · 2025-01-30T02:53:41Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader7.java

-            snapshotReader.getSoftDeletesFieldData()
-        );
-    }
+public class LuceneDocumentsReader7 implements LuceneDocumentsReader {


For later, it we're able to isolate lucene7 vs lucene9 code and we only need one version in the runtime at once, it should be a chip-shot to load the things from the ServiceLoader interface (and not require shadowing at all).

It would also be nice to not have to require as much copied code (e.g. just building/linking for different versions, or injecting different version specific overrides)

gregschohn · 2025-01-30T02:57:58Z

...rc/main/java/org/opensearch/migrations/bulkload/version_es_6_8/ShardMetadataData_ES_6_8.java

@@ -15,7 +15,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.Getter;
-import org.apache.lucene.util.BytesRef;
+import shadow.lucene9.org.apache.lucene.util.BytesRef;


This seems like it could be a liability. If there was a difference in the serialized ES5 bytes and how Lucene 9 read it, we could have a problem, right? I'm not too worried, but it does seem like it could be a bigger issue as we go back in ES/Lucene versions.

RFS/src/test/java/org/opensearch/migrations/bulkload/PerformanceVerificationTest.java

gregschohn · 2025-01-30T02:59:37Z

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader.java

+import lombok.extern.slf4j.Slf4j;
+import reactor.core.publisher.Flux;
+
+public interface LuceneDocumentsReader {


ILuceneDocumentsReader to show that it's an interface

peternied added 11 commits January 27, 2025 19:17

Checkpoint changes

94abccd

Signed-off-by: Peter Nied <[email protected]>

Remove code changes

b8d49d2

Signed-off-by: Peter Nied <[email protected]>

Lucene 7 & 9 constructors

007b2a0

Signed-off-by: Peter Nied <[email protected]>

Cleanup files pass one

ac166b0

Signed-off-by: Peter Nied <[email protected]>

Minor compatability changes

e61065d

Signed-off-by: Peter Nied <[email protected]>

Add in abstraction model

1f59a3e

Signed-off-by: Peter Nied <[email protected]>

Remove overkill abstraction model

3083413

Signed-off-by: Peter Nied <[email protected]>

Fixesassociated with gradle update

365ba5d

Signed-off-by: Peter Nied <[email protected]>

Shift back to older gradle version

7e4a32c

Signed-off-by: Peter Nied <[email protected]>

Move perf tests into the currect part of the project

9bd267e

Signed-off-by: Peter Nied <[email protected]>

Fix spotless failures

6ad7512

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 28, 2025 15:59 — with GitHub Actions Inactive

Merge remote-tracking branch 'origin/main' into shadow-jars

7ad7dab

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 28, 2025 17:58 — with GitHub Actions Inactive

Cleanup build.gradle a little

c1c68ef

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 28, 2025 18:38 — with GitHub Actions Inactive

peternied added 4 commits January 28, 2025 18:45

Prevent random failures due to OvewrflowException

9068752

Signed-off-by: Peter Nied <[email protected]>

Revert "Fixesassociated with gradle update"

6a4a6c8

This reverts commit 365ba5d. Signed-off-by: Peter Nied <[email protected]>

Update test cases

c0731f0

Signed-off-by: Peter Nied <[email protected]>

Remove extra test cases

a177a8e

Signed-off-by: Peter Nied <[email protected]>

peternied had a problem deploying to migrations-cicd January 28, 2025 20:24 — with GitHub Actions Failure

peternied commented Jan 28, 2025

View reviewed changes

After PR inspection

e511c1b

Signed-off-by: Peter Nied <[email protected]>

peternied marked this pull request as ready for review January 28, 2025 22:07

peternied requested review from AndreKurait, chelma, gregschohn, lewijacn, mikaylathompson and sumobrian as code owners January 28, 2025 22:07

peternied requested a review from jugal-chauhan as a code owner January 28, 2025 22:07

peternied had a problem deploying to migrations-cicd January 28, 2025 22:07 — with GitHub Actions Failure

chelma reviewed Jan 28, 2025

View reviewed changes

Update RFS project to be a library

9e042d0

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 29, 2025 16:39 — with GitHub Actions Inactive

chelma reviewed Jan 29, 2025

View reviewed changes

Small tweaks for PR feedback

59ddb88

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 29, 2025 17:35 — with GitHub Actions Inactive

chelma reviewed Jan 29, 2025

View reviewed changes

build.gradle Show resolved Hide resolved

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java Show resolved Hide resolved

jugal-chauhan reviewed Jan 29, 2025

View reviewed changes

RFS/src/main/java/org/opensearch/migrations/bulkload/lucene/LuceneDocumentsReader9.java Show resolved Hide resolved

jugal-chauhan reviewed Jan 29, 2025

View reviewed changes

RFS/src/test/java/org/opensearch/migrations/bulkload/PerformanceVerificationTest.java Show resolved Hide resolved

jugal-chauhan reviewed Jan 29, 2025

View reviewed changes

.../main/java/org/opensearch/migrations/bulkload/version_es_7_10/ShardMetadataData_ES_7_10.java Show resolved Hide resolved

Fix sonarqube issues

322bc36

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 29, 2025 20:23 — with GitHub Actions Inactive

chelma approved these changes Jan 29, 2025

View reviewed changes

peternied had a problem deploying to migrations-cicd January 29, 2025 22:36 — with GitHub Actions Failure

peternied force-pushed the shadow-jars branch from 079edc3 to 21fb574 Compare January 29, 2025 23:01

peternied had a problem deploying to migrations-cicd January 29, 2025 23:01 — with GitHub Actions Failure

Fix broken test case

5c22580

Signed-off-by: Peter Nied <[email protected]>

peternied temporarily deployed to migrations-cicd January 30, 2025 00:25 — with GitHub Actions Inactive

peternied merged commit 5b924e6 into opensearch-project:main Jan 30, 2025
23 checks passed

gregschohn reviewed Jan 30, 2025

View reviewed changes

peternied deleted the shadow-jars branch January 30, 2025 19:37

peternied restored the shadow-jars branch February 11, 2025 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for ES6 using indexes created in ES5 #1253

Support for ES6 using indexes created in ES5 #1253

peternied commented Jan 28, 2025 •

edited

Loading

chelma left a comment

chelma commented Jan 28, 2025

peternied commented Jan 28, 2025

codecov bot commented Jan 29, 2025 •

edited

Loading

chelma left a comment

chelma left a comment

gregschohn left a comment

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

gregschohn Jan 30, 2025

Support for ES6 using indexes created in ES5 #1253

Support for ES6 using indexes created in ES5 #1253

Conversation

peternied commented Jan 28, 2025 • edited Loading

Description

Issues Resolved

Check List

chelma left a comment

Choose a reason for hiding this comment

chelma commented Jan 28, 2025

peternied commented Jan 28, 2025

codecov bot commented Jan 29, 2025 • edited Loading

Codecov Report

chelma left a comment

Choose a reason for hiding this comment

chelma left a comment

Choose a reason for hiding this comment

gregschohn 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

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

peternied commented Jan 28, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading