Enhance searchable snapshots to enable a read-only view of older snapshots by kartg · Pull Request #5812 · opensearch-project/OpenSearch

kartg · 2023-01-11T00:37:01Z

Description

This change is a forward-port of #5429 - the changes were made on 2.x first because main contains some backwards-incompatible changes that made development and testing of the feature difficult.

The commits in this PR consist of cherry-picking the commit from 2.x followed by incremental changes to make the code buildable and the functionality operational in main:

The first commit is the cherry-pick; its diff is identical to change merged to 2.x. The second commit fixes some minor merge issues.
The third commit fixes multiple aspects of version resolution in a cascading manner. Firstly, the Lucene 9 libraries no longer hold constants for Lucene 7 and below, so a more explicit declaration is necessary. This in turn requires an update to derive DECLARED_VERSIONS from the Version class rather than the LegacyESVersion class (to ignore legacy versions), which then required fixing the minimumIndexCompatibleVersion logic for LegacyESVersion. Finally, the testMinimumIndexCompatibilityVersion unit test was updated to use accurate version identifiers. At this point, all unit tests pass.
The fourth commit reverts the removal of some backwards compatibility logic in IndexMetadata (removed in #4702) and IndexMetadataGenerations (removed in #4728). It also adds a unit test for the latter class.
The fifth commit in the PR improves readability by using LegacyESVersion constants and includes some other minor fixes to documentation and code.

Readability fixes and the unit tests for IndexMetadataGenerations will be backported to 2.x in a separate PR.

Issues Resolved

closes #5451

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-01-11T01:04:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/9243/
CommitID: 6ba9d54
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-01-11T01:18:59Z

Gradle Check (Jenkins) Run Completed with:

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

@mch2

…shots (opensearch-project#5429) * Enhance searchable snapshots to enable a read-only view of older snapshots This change removes the guardrails around N-1 backward compatibility and uses Lucene's "expert" APIs to read snapshots (Lucene segments) older than N-1 on a best-effort basis. The functionality is gated by an additional feature flag, separate from the searchable snapshots flag. Note that the Lucene integration is rather inefficient because the necessary "expert" Lucene APIs are still package-private. Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Added some unit tests This change also includes a test index ZIP file for the unit tests. The change also introduces a bug fix in the readAnySegmentsInfo method to close the reader before returning the SegmentInfos object - this avoids dangling/open file handles. Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Incorporating PR feedback Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Incorporate PR comments from andrross Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Remove use of IndexSetting for minimum version for snapshots backwards compatibility Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Moved ES 6.3.0 test data to a subdirectory This change also includes an update to the file name to clarify that it is an ES index, and changing the associated markdown file name to just README.md. All tests that reference this ZIP file have corresponding changes to the path they reference. Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Update unit tests to use try-with-resources Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Added FeatureFlagSetter helper class Also refactored unit test classes to use the helper class. Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Incorporating PR feedback from @mch2 Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Fix IndexSettingsTests Updated the asserts in IndexSettingsTests to account for the new defaulting behavior. Signed-off-by: Kartik Ganesh <gkart@amazon.com> Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Note that the unit tests are still failing at this commit since the Lucene 9 libraries no longer hold constants for Lucene 7 and below, so the fromId logic resolves the Lucene version to 8. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

This change fixes resolution of the Lucene version for legacy versions since the Lucene 9 libraries no longer hold constants for Lucene 7 and below. The change also updates DECLARED_VERSIONS to derive from the Versions class rather than LegacyESVersions (thereby ignoring legacy versions). This in turn required a change to the minimumIndexCompatibleVersion logic for LegacyESVersion. Finally, the testMinimumIndexCompatibilityVersion unit test was updated to use accurate version identifiers. All unit tests pass and the code compiles, but actual functionality is still broken because some backwards compatibility logic was removed in the current branch that is retained in 2.x Signed-off-by: Kartik Ganesh <gkart@amazon.com>

This reverts changes made in opensearch-project#4728 and opensearch-project#4702. These were only made in main and not backported to 2.x This change also adds unit tests for IndexMetadataGenerations Signed-off-by: Kartik Ganesh <gkart@amazon.com>

This commit also includes a correction to documentation, and removes the unnecessary "afterWriteSnapBlob" runnable from BlobStoreRepository Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

github-actions · 2023-01-12T00:43:41Z

Gradle Check (Jenkins) Run Completed with:

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

mch2 · 2023-01-12T19:31:53Z

server/src/test/java/org/opensearch/VersionTests.java

-        assertEquals(LegacyESVersion.fromId(7000099), Version.fromId(2000099).minimumIndexCompatibilityVersion());
-        assertEquals(LegacyESVersion.fromId(7000099), Version.fromId(2010000).minimumIndexCompatibilityVersion());
-        assertEquals(LegacyESVersion.fromId(7000099), Version.fromId(2000001).minimumIndexCompatibilityVersion());
+        assertEquals(LegacyESVersion.fromId(7000099), Version.fromId(2000099 ^ MASK).minimumIndexCompatibilityVersion());


Why is the mask now required?

The simple answer is that MASK is now required because the computeMinIndexCompatVersion logic has diverged between LegacyESVersion and Version.

Previously, logic to handle both types of versions were intertwined within the same method. This test then (incorrectly) created LegacyESVersion objects in the Version.fromId invocations, but the asserts still passed since the logic was co-mingled.

Now that the two classes have different logic to compute the minimum index-compatible version, the test breaks because LegacyESVersion 2.0 is (correctly) not compatible with LegacyESVersion 7.0. Adding the MASK ensures that a (OpenSearch) Version 2.0 object is created, which is compatible with LegacyESVersion 7.0

CHANGELOG.md

This feature was released in 2.5.0 so it no longer needs to be listed in the changelog. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

github-actions · 2023-01-13T20:19:18Z

Gradle Check (Jenkins) Run Completed with:

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

server/src/main/java/org/opensearch/Version.java

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

github-actions · 2023-01-13T22:52:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/9401/
CommitID: 54e21db

…roject#5812 Signed-off-by: Kartik Ganesh <gkart@amazon.com>

… from #5812 (#5922) * Backporting unit tests for IndexMetadataGenerations from #5812 Signed-off-by: Kartik Ganesh <gkart@amazon.com> * Fix javadoc for getExtendedCompatibilitySnapshotVersion Signed-off-by: Kartik Ganesh <gkart@amazon.com> Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg marked this pull request as ready for review January 11, 2023 00:51

kartg requested review from Bukhtawar, CEHENKLE, Rishikesh1159, VachaShah, adnapibar, anasalkouz, andrross, dblock, dreamer-89, gbbafna, kotwanikunal, mch2, nknize, owaiskazi19, reta, ryanbogan, saratvemulapalli, setiah, shwetathareja, tlfeng and xuezhou25 as code owners January 11, 2023 00:51

kartg mentioned this pull request Jan 11, 2023

[Proposal] Enable searchable snapshots to read legacy versions #5451

Closed

kartg added 5 commits January 11, 2023 15:45

Fixed compile issues after cherry-pick

f4bc00c

Note that the unit tests are still failing at this commit since the Lucene 9 libraries no longer hold constants for Lucene 7 and below, so the fromId logic resolves the Lucene version to 8. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Declared LegacyESVersion constants for better readability

535f71f

This commit also includes a correction to documentation, and removes the unnecessary "afterWriteSnapBlob" runnable from BlobStoreRepository Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Fixing PR reference in CHANGELOG

f0bfd4b

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg force-pushed the backwardCompat branch from a9a67e8 to f0bfd4b Compare January 12, 2023 00:12

mch2 reviewed Jan 12, 2023

View reviewed changes

mch2 approved these changes Jan 12, 2023

View reviewed changes

andrross reviewed Jan 13, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Revert CHANGELOG update

d5fd0c9

This feature was released in 2.5.0 so it no longer needs to be listed in the changelog. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

andrross approved these changes Jan 13, 2023

View reviewed changes

reta reviewed Jan 13, 2023

View reviewed changes

server/src/main/java/org/opensearch/Version.java Outdated Show resolved Hide resolved

Remove outdated comment

54e21db

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg merged commit 095c222 into opensearch-project:main Jan 13, 2023

kartg added a commit to kartg/OpenSearch that referenced this pull request Jan 17, 2023

Backporting unit tests for IndexMetadataGenerations from opensearch-p…

330e7f5

…roject#5812 Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg mentioned this pull request Jan 18, 2023

Backporting unit tests for IndexMetadataGenerations and a javadoc fix from #5812 #5922

Merged

6 tasks

kartg mentioned this pull request Jan 20, 2023

[Meta] GA for searchable snapshots reading legacy versions #5948

Closed

3 tasks

kartg deleted the backwardCompat branch November 20, 2023 23:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance searchable snapshots to enable a read-only view of older snapshots#5812

Enhance searchable snapshots to enable a read-only view of older snapshots#5812
kartg merged 8 commits intoopensearch-project:mainfrom
kartg:backwardCompat

kartg commented Jan 11, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Jan 11, 2023

Uh oh!

github-actions bot commented Jan 11, 2023

Uh oh!

github-actions bot commented Jan 12, 2023

Uh oh!

mch2 Jan 12, 2023

Uh oh!

kartg Jan 12, 2023 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2023

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kartg commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

github-actions bot commented Jan 11, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Jan 11, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Jan 12, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

mch2 Jan 12, 2023

Choose a reason for hiding this comment

Uh oh!

kartg Jan 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kartg commented Jan 11, 2023 •

edited

Loading

kartg Jan 12, 2023 •

edited

Loading