From 85b85b174741a301c7552febc4b980df4767c3f5 Mon Sep 17 00:00:00 2001 From: Kartik Ganesh Date: Thu, 5 Jan 2023 14:00:33 -0800 Subject: [PATCH] Enhance searchable snapshots to enable a read-only view of older snapshots (#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 * 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 * Incorporating PR feedback Signed-off-by: Kartik Ganesh * Incorporate PR comments from andrross Signed-off-by: Kartik Ganesh * Remove use of IndexSetting for minimum version for snapshots backwards compatibility Signed-off-by: Kartik Ganesh * 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 * Update unit tests to use try-with-resources Signed-off-by: Kartik Ganesh * Added FeatureFlagSetter helper class Also refactored unit test classes to use the helper class. Signed-off-by: Kartik Ganesh * Incorporating PR feedback from @mch2 Signed-off-by: Kartik Ganesh * Fix IndexSettingsTests Updated the asserts in IndexSettingsTests to account for the new defaulting behavior. Signed-off-by: Kartik Ganesh Signed-off-by: Kartik Ganesh --- CHANGELOG.md | 1 + .../cluster/block/ClusterBlocks.java | 3 +- .../cluster/metadata/IndexMetadata.java | 7 +- .../cluster/routing/RoutingPool.java | 2 +- .../org/opensearch/common/lucene/Lucene.java | 20 ++++ .../opensearch/common/util/FeatureFlags.java | 7 ++ .../org/opensearch/index/IndexModule.java | 14 ++- .../org/opensearch/index/IndexSettings.java | 28 ++++++ .../index/engine/ReadOnlyEngine.java | 21 +++- .../opensearch/index/shard/IndexShard.java | 14 ++- .../org/opensearch/index/store/Store.java | 30 +++++- .../directory/RemoteSnapshotDirectory.java | 5 + .../opensearch/indices/IndicesService.java | 2 +- .../recovery/PeerRecoveryTargetService.java | 3 +- .../indices/recovery/RecoveryTarget.java | 3 +- .../blobstore/FileRestoreContext.java | 5 + .../opensearch/snapshots/RestoreService.java | 22 +++- .../opensearch/snapshots/SnapshotUtils.java | 2 +- .../opensearch/common/lucene/LuceneTests.java | 94 ++++++++++++++++++ .../common/settings/SettingsModuleTests.java | 73 +++++++------- .../common/util/FeatureFlagTests.java | 33 ++---- .../extensions/ExtensionsManagerTests.java | 7 +- .../opensearch/index/IndexSettingsTests.java | 46 +++++++++ .../index/engine/ReadOnlyEngineTests.java | 54 ++++++++++ .../opensearch/index/store/StoreTests.java | 48 +++++++++ .../resources/indices/bwc/es-6.3.0/README.md | 57 +++++++++++ .../bwc/es-6.3.0/testIndex-es-6.3.0.zip | Bin 0 -> 2467 bytes .../opensearch/test/FeatureFlagSetter.java | 39 ++++++++ 28 files changed, 554 insertions(+), 86 deletions(-) create mode 100644 server/src/test/resources/indices/bwc/es-6.3.0/README.md create mode 100644 server/src/test/resources/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip create mode 100644 test/framework/src/main/java/org/opensearch/test/FeatureFlagSetter.java diff --git a/CHANGELOG.md b/CHANGELOG.md index ab3266e44ace2..3e17b4725c0dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518)), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597)), ([#5615](https://github.com/opensearch-project/OpenSearch/pull/5615))) - Add CI bundle pattern to distribution download ([#5348](https://github.com/opensearch-project/OpenSearch/pull/5348)) - Added @gbbafna as an OpenSearch maintainer ([#5668](https://github.com/opensearch-project/OpenSearch/pull/5668)) +- Experimental support for extended backward compatiblity in searchable snapshots ([#5429](https://github.com/opensearch-project/OpenSearch/pull/5429)) ### Dependencies - Bump bcpg-fips from 1.0.5.1 to 1.0.7.1 ([#5148](https://github.com/opensearch-project/OpenSearch/pull/5148)) diff --git a/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java b/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java index 13d8909b41ff5..1a49e18713901 100644 --- a/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java +++ b/server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java @@ -394,8 +394,7 @@ public Builder addBlocks(IndexMetadata indexMetadata) { if (IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.get(indexMetadata.getSettings())) { addIndexBlock(indexName, IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK); } - if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey() - .equals(indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) { + if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) { addIndexBlock(indexName, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE); } return this; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index ae135e1ad4ff3..4a3cad9e5a7b2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -1825,10 +1825,11 @@ public static IndexMetadata fromXContent(XContentParser parser) throws IOExcepti throw new IllegalArgumentException("Unexpected token " + token); } } - if (Assertions.ENABLED) { + // Reference: + // https://github.com/opensearch-project/OpenSearch/blob/4dde0f2a3b445b2fc61dab29c5a2178967f4a3e3/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java#L1620-L1628 + Version legacyVersion = LegacyESVersion.fromId(6050099); + if (Assertions.ENABLED && Version.indexCreated(builder.settings).onOrAfter(legacyVersion)) { assert mappingVersion : "mapping version should be present for indices"; - } - if (Assertions.ENABLED) { assert settingsVersion : "settings version should be present for indices"; } if (Assertions.ENABLED && Version.indexCreated(builder.settings).onOrAfter(LegacyESVersion.V_7_2_0)) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingPool.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingPool.java index 1a3c366694221..a4ff237460e28 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingPool.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingPool.java @@ -61,7 +61,7 @@ public static RoutingPool getShardPool(ShardRouting shard, RoutingAllocation all */ public static RoutingPool getIndexPool(IndexMetadata indexMetadata) { Settings indexSettings = indexMetadata.getSettings(); - if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(indexSettings.get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) { + if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexSettings.get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) { return REMOTE_CAPABLE; } return LOCAL_ONLY; diff --git a/server/src/main/java/org/opensearch/common/lucene/Lucene.java b/server/src/main/java/org/opensearch/common/lucene/Lucene.java index 2692a8fa2b914..870293e425f8a 100644 --- a/server/src/main/java/org/opensearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/opensearch/common/lucene/Lucene.java @@ -67,6 +67,7 @@ import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.StandardDirectoryReader; import org.apache.lucene.index.StoredFieldVisitor; import org.apache.lucene.index.Terms; import org.apache.lucene.index.VectorValues; @@ -162,6 +163,25 @@ public static SegmentInfos readSegmentInfos(Directory directory) throws IOExcept return SegmentInfos.readLatestCommit(directory); } + /** + * A variant of {@link #readSegmentInfos(Directory)} that supports reading indices written by + * older major versions of Lucene. The underlying implementation is a workaround since the + * "expert" readLatestCommit API is currently package-private in Lucene. First, all commits in + * the given {@link Directory} are listed - this result includes older Lucene commits. Then, + * the latest index commit is opened via {@link DirectoryReader} by including a minimum supported + * Lucene major version based on the minimum compatibility of the given {@link org.opensearch.Version}. + */ + public static SegmentInfos readSegmentInfosExtendedCompatibility(Directory directory, org.opensearch.Version minimumVersion) + throws IOException { + // This list is sorted from oldest to latest + List indexCommits = DirectoryReader.listCommits(directory); + IndexCommit latestCommit = indexCommits.get(indexCommits.size() - 1); + final int minSupportedLuceneMajor = minimumVersion.minimumIndexCompatibilityVersion().luceneVersion.major; + try (StandardDirectoryReader reader = (StandardDirectoryReader) DirectoryReader.open(latestCommit, minSupportedLuceneMajor, null)) { + return reader.getSegmentInfos(); + } + } + /** * Returns an iterable that allows to iterate over all files in this segments info */ diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 31dd621f678ad..9f1a9a43ff54e 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -37,6 +37,13 @@ public class FeatureFlags { */ public static final String SEARCHABLE_SNAPSHOT = "opensearch.experimental.feature.searchable_snapshot.enabled"; + /** + * Gates the ability for Searchable Snapshots to read snapshots that are older than the + * guaranteed backward compatibility for OpenSearch (one prior major version) on a best effort basis. + */ + public static final String SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY = + "opensearch.experimental.feature.searchable_snapshot.extended_compatibility.enabled"; + /** * Gates the functionality of extensions. * Once the feature is ready for production release, this feature flag can be removed. diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index 9f7e3e9fb5eee..076d0b65a6113 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -459,11 +459,19 @@ public boolean match(String setting) { } /** - * Convenience method to check whether the given IndexSettings contains - * an {@link #INDEX_STORE_TYPE_SETTING} set to the value of this type. + * Convenience method to check whether the given {@link IndexSettings} + * object contains an {@link #INDEX_STORE_TYPE_SETTING} set to the value of this type. */ public boolean match(IndexSettings settings) { - return match(INDEX_STORE_TYPE_SETTING.get(settings.getSettings())); + return match(settings.getSettings()); + } + + /** + * Convenience method to check whether the given {@link Settings} + * object contains an {@link #INDEX_STORE_TYPE_SETTING} set to the value of this type. + */ + public boolean match(Settings settings) { + return match(INDEX_STORE_TYPE_SETTING.get(settings)); } } diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 72602077150b4..b735a9e4e3ea0 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -46,6 +46,7 @@ import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.translog.Translog; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.ingest.IngestService; @@ -60,11 +61,13 @@ import java.util.function.Function; import java.util.function.UnaryOperator; +import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING; +import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; /** * This class encapsulates all index level settings and handles settings updates. @@ -585,6 +588,8 @@ public final class IndexSettings { private final boolean isRemoteStoreEnabled; private final String remoteStoreRepository; private final boolean isRemoteTranslogStoreEnabled; + private final boolean isRemoteSnapshot; + private Version extendedCompatibilitySnapshotVersion; // volatile fields are updated via #updateIndexMetadata(IndexMetadata) under lock private volatile Settings settings; private volatile IndexMetadata indexMetadata; @@ -747,6 +752,13 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti isRemoteStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false); remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY); isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false); + isRemoteSnapshot = IndexModule.Type.REMOTE_SNAPSHOT.match(this.settings); + + if (isRemoteSnapshot && FeatureFlags.isEnabled(SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY)) { + extendedCompatibilitySnapshotVersion = SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; + } else { + extendedCompatibilitySnapshotVersion = Version.CURRENT.minimumIndexCompatibilityVersion(); + } this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings); this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings); this.queryStringAnalyzeWildcard = QUERY_STRING_ANALYZE_WILDCARD.get(nodeSettings); @@ -1012,6 +1024,22 @@ public boolean isRemoteTranslogStoreEnabled() { return isRemoteTranslogStoreEnabled; } + /** + * Returns true if this is remote/searchable snapshot + */ + public boolean isRemoteSnapshot() { + return isRemoteSnapshot; + } + + /** + * If this is a remote snapshot and the extended compatibility + * feature flag is enabled, this returns the minimum {@link Version} + * supported. In all other cases, the return value is null. + */ + public Version getExtendedCompatibilitySnapshotVersion() { + return extendedCompatibilitySnapshotVersion; + } + /** * Returns the node settings. The settings returned from {@link #getSettings()} are a merged version of the * index settings and the node settings where node settings are overwritten by index settings. diff --git a/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java index ceea9eaac994c..c20ddcee6da62 100644 --- a/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java @@ -93,6 +93,7 @@ public class ReadOnlyEngine extends Engine { private final CompletionStatsCache completionStatsCache; private final boolean requireCompleteHistory; private final TranslogManager translogManager; + private final Version minimumSupportedVersion; protected volatile TranslogStats translogStats; @@ -119,6 +120,8 @@ public ReadOnlyEngine( ) { super(config); this.requireCompleteHistory = requireCompleteHistory; + // fetch the minimum Version for extended backward compatibility use-cases + this.minimumSupportedVersion = config.getIndexSettings().getExtendedCompatibilitySnapshotVersion(); try { Store store = config.getStore(); store.incRef(); @@ -130,7 +133,11 @@ public ReadOnlyEngine( // we obtain the IW lock even though we never modify the index. // yet this makes sure nobody else does. including some testing tools that try to be messy indexWriterLock = obtainLock ? directory.obtainLock(IndexWriter.WRITE_LOCK_NAME) : null; - this.lastCommittedSegmentInfos = Lucene.readSegmentInfos(directory); + if (isExtendedCompatibility()) { + this.lastCommittedSegmentInfos = Lucene.readSegmentInfosExtendedCompatibility(directory, this.minimumSupportedVersion); + } else { + this.lastCommittedSegmentInfos = Lucene.readSegmentInfos(directory); + } if (seqNoStats == null) { seqNoStats = buildSeqNoStats(config, lastCommittedSegmentInfos); ensureMaxSeqNoEqualsToGlobalCheckpoint(seqNoStats); @@ -223,7 +230,17 @@ protected final OpenSearchDirectoryReader wrapReader( protected DirectoryReader open(IndexCommit commit) throws IOException { assert Transports.assertNotTransportThread("opening index commit of a read-only engine"); - return new SoftDeletesDirectoryReaderWrapper(DirectoryReader.open(commit), Lucene.SOFT_DELETES_FIELD); + DirectoryReader reader; + if (isExtendedCompatibility()) { + reader = DirectoryReader.open(commit, this.minimumSupportedVersion.luceneVersion.major, null); + } else { + reader = DirectoryReader.open(commit); + } + return new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD); + } + + private boolean isExtendedCompatibility() { + return Version.CURRENT.minimumIndexCompatibilityVersion().onOrAfter(this.minimumSupportedVersion); } @Override diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index a108d1696ee93..a800f26a58c08 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -1984,7 +1984,7 @@ public void openEngineAndRecoverFromTranslog() throws IOException { }; // Do not load the global checkpoint if this is a remote snapshot index - if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexSettings) == false) { + if (indexSettings.isRemoteSnapshot() == false) { loadGlobalCheckpointToReplicationTracker(); } @@ -2039,7 +2039,7 @@ private void innerOpenEngineAndTranslog(LongSupplier globalCheckpointSupplier) t } private boolean assertSequenceNumbersInCommit() throws IOException { - final Map userData = SegmentInfos.readLatestCommit(store.directory()).getUserData(); + final Map userData = fetchUserData(); assert userData.containsKey(SequenceNumbers.LOCAL_CHECKPOINT_KEY) : "commit point doesn't contains a local checkpoint"; assert userData.containsKey(MAX_SEQ_NO) : "commit point doesn't contains a maximum sequence number"; assert userData.containsKey(Engine.HISTORY_UUID_KEY) : "commit point doesn't contains a history uuid"; @@ -2054,6 +2054,16 @@ private boolean assertSequenceNumbersInCommit() throws IOException { return true; } + private Map fetchUserData() throws IOException { + if (indexSettings.isRemoteSnapshot() && indexSettings.getExtendedCompatibilitySnapshotVersion() != null) { + // Inefficient method to support reading old Lucene indexes + return Lucene.readSegmentInfosExtendedCompatibility(store.directory(), indexSettings.getExtendedCompatibilitySnapshotVersion()) + .getUserData(); + } else { + return SegmentInfos.readLatestCommit(store.directory()).getUserData(); + } + } + private void onNewEngine(Engine newEngine) { assert Thread.holdsLock(engineMutex); refreshListeners.setCurrentRefreshLocationSupplier(newEngine::getTranslogLastWriteLocation); diff --git a/server/src/main/java/org/opensearch/index/store/Store.java b/server/src/main/java/org/opensearch/index/store/Store.java index 3354f7e8dbacb..de12d9b75dc12 100644 --- a/server/src/main/java/org/opensearch/index/store/Store.java +++ b/server/src/main/java/org/opensearch/index/store/Store.java @@ -221,7 +221,11 @@ public Directory directory() { public SegmentInfos readLastCommittedSegmentsInfo() throws IOException { failIfCorrupted(); try { - return readSegmentsInfo(null, directory()); + if (indexSettings.isRemoteSnapshot() && indexSettings.getExtendedCompatibilitySnapshotVersion() != null) { + return readSegmentInfosExtendedCompatibility(directory(), indexSettings.getExtendedCompatibilitySnapshotVersion()); + } else { + return readSegmentsInfo(null, directory()); + } } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) { markStoreCorrupted(ex); throw ex; @@ -229,7 +233,9 @@ public SegmentInfos readLastCommittedSegmentsInfo() throws IOException { } /** - * Returns the segments info for the given commit or for the latest commit if the given commit is null + * Returns the segments info for the given commit or for the latest commit if the given commit is null. + * This method will throw an exception if the index is older than the standard backwards compatibility + * policy ( current major - 1). See also {@link #readSegmentInfosExtendedCompatibility(Directory, org.opensearch.Version)}. * * @throws IOException if the index is corrupted or the segments file is not present */ @@ -245,7 +251,27 @@ private static SegmentInfos readSegmentsInfo(IndexCommit commit, Directory direc } catch (Exception ex) { throw new CorruptIndexException("Hit unexpected exception while reading segment infos", "commit(" + commit + ")", ex); } + } + /** + * Returns the segments info for the latest commit in the given directory. Unlike + * {@link #readSegmentsInfo(IndexCommit, Directory)}, this method supports reading + * older Lucene indices on a best-effort basis. + * + * @throws IOException if the index is corrupted or the segments file is not present + */ + private static SegmentInfos readSegmentInfosExtendedCompatibility(Directory directory, org.opensearch.Version minimumVersion) + throws IOException { + try { + return Lucene.readSegmentInfosExtendedCompatibility(directory, minimumVersion); + } catch (EOFException eof) { + // TODO this should be caught by lucene - EOF is almost certainly an index corruption + throw new CorruptIndexException("Read past EOF while reading segment infos", "", eof); + } catch (IOException exception) { + throw exception; // IOExceptions like too many open files are not necessarily a corruption - just bubble it up + } catch (Exception ex) { + throw new CorruptIndexException("Hit unexpected exception while reading segment infos", "", ex); + } } final void ensureOpen() { diff --git a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectory.java b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectory.java index 3a2749a6d325b..e723c831b213d 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectory.java @@ -22,6 +22,8 @@ import org.apache.lucene.store.IndexOutput; import org.apache.lucene.store.Lock; import org.apache.lucene.store.NoLockFactory; +import org.opensearch.LegacyESVersion; +import org.opensearch.Version; import org.opensearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot; import org.opensearch.index.store.remote.file.OnDemandBlockSnapshotIndexInput; import org.opensearch.index.store.remote.file.OnDemandVirtualFileSnapshotIndexInput; @@ -35,6 +37,9 @@ * @opensearch.internal */ public final class RemoteSnapshotDirectory extends Directory { + + public static final Version SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION = LegacyESVersion.fromId(6000099); + private static final String VIRTUAL_FILE_PREFIX = BlobStoreRepository.VIRTUAL_DATA_BLOB_PREFIX; private final Map fileInfoMap; diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index afc2af302ed02..9f14ca7d013b2 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -887,7 +887,7 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) { if (idxSettings.isSegRepEnabled()) { return new NRTReplicationEngineFactory(); } - if (IndexModule.Type.REMOTE_SNAPSHOT.match(idxSettings)) { + if (idxSettings.isRemoteSnapshot()) { return config -> new ReadOnlyEngine(config, new SeqNoStats(0, 0, 0), new TranslogStats(), true, Function.identity(), false); } return new InternalEngineFactory(); diff --git a/server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java b/server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java index 556e4db3400e1..afee371adc66c 100644 --- a/server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java +++ b/server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java @@ -54,7 +54,6 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.CancellableThreads; import org.opensearch.common.util.concurrent.AbstractRunnable; -import org.opensearch.index.IndexModule; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.engine.RecoveryEngineException; import org.opensearch.index.mapper.MapperException; @@ -246,7 +245,7 @@ private void doRecovery(final long recoveryId, final StartRecoveryRequest preExi logger.trace("{} preparing shard for peer recovery", recoveryTarget.shardId()); indexShard.prepareForIndexRecovery(); final boolean hasRemoteTranslog = recoveryTarget.state().getPrimary() == false && indexShard.isRemoteTranslogEnabled(); - final boolean hasNoTranslog = IndexModule.Type.REMOTE_SNAPSHOT.match(indexShard.indexSettings()); + final boolean hasNoTranslog = indexShard.indexSettings().isRemoteSnapshot(); final boolean verifyTranslog = (hasRemoteTranslog || hasNoTranslog) == false; final long startingSeqNo = indexShard.recoverLocallyAndFetchStartSeqNo(!hasRemoteTranslog); assert startingSeqNo == UNASSIGNED_SEQ_NO || recoveryTarget.state().getStage() == RecoveryState.Stage.TRANSLOG diff --git a/server/src/main/java/org/opensearch/indices/recovery/RecoveryTarget.java b/server/src/main/java/org/opensearch/indices/recovery/RecoveryTarget.java index c661ad3a461fe..62aad1ce263ad 100644 --- a/server/src/main/java/org/opensearch/indices/recovery/RecoveryTarget.java +++ b/server/src/main/java/org/opensearch/indices/recovery/RecoveryTarget.java @@ -45,7 +45,6 @@ import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.lucene.Lucene; import org.opensearch.common.util.CancellableThreads; -import org.opensearch.index.IndexModule; import org.opensearch.index.engine.Engine; import org.opensearch.index.mapper.MapperException; import org.opensearch.index.seqno.ReplicationTracker; @@ -407,7 +406,7 @@ public void cleanFiles( // their own commit points and therefore do not modify the commit user data // in their store. In these cases, reuse the primary's translog UUID. final boolean reuseTranslogUUID = indexShard.indexSettings().isSegRepEnabled() - || IndexModule.Type.REMOTE_SNAPSHOT.match(indexShard.indexSettings()); + || indexShard.indexSettings().isRemoteSnapshot(); if (reuseTranslogUUID) { final String translogUUID = store.getMetadata().getCommitUserData().get(TRANSLOG_UUID_KEY); Translog.createEmptyTranslog( diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/FileRestoreContext.java b/server/src/main/java/org/opensearch/repositories/blobstore/FileRestoreContext.java index d6cffcfbb8db8..8217e73c01a3c 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/FileRestoreContext.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/FileRestoreContext.java @@ -43,6 +43,7 @@ import org.opensearch.index.snapshots.blobstore.SnapshotFiles; import org.opensearch.index.store.Store; import org.opensearch.index.store.StoreFileMetadata; +import org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory; import org.opensearch.indices.recovery.RecoveryState; import org.opensearch.snapshots.SnapshotId; @@ -198,6 +199,10 @@ public void restore(SnapshotFiles snapshotFiles, Store store, ActionListener shardsBuilder = ImmutableOpenMap .builder(); - final Version minIndexCompatibilityVersion = currentState.getNodes() - .getMaxNodeVersion() - .minimumIndexCompatibilityVersion(); + for (Map.Entry indexEntry : indices.entrySet()) { String renamedIndexName = indexEntry.getKey(); String index = indexEntry.getValue(); @@ -456,7 +456,7 @@ public ClusterState execute(ClusterState currentState) { request.ignoreIndexSettings() ); final boolean isSearchableSnapshot = FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) - && IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(request.storageType().toString()); + && IndexModule.Type.REMOTE_SNAPSHOT.match(request.storageType().toString()); if (isSearchableSnapshot) { snapshotIndexMetadata = addSnapshotToIndexSettings( snapshotIndexMetadata, @@ -471,6 +471,15 @@ public ClusterState execute(ClusterState currentState) { repositoryData.resolveIndexId(index), isSearchableSnapshot ); + final Version minIndexCompatibilityVersion; + if (isSearchableSnapshot && isSearchableSnapshotsExtendedCompatibilityEnabled()) { + minIndexCompatibilityVersion = SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION + .minimumIndexCompatibilityVersion(); + } else { + minIndexCompatibilityVersion = currentState.getNodes() + .getMaxNodeVersion() + .minimumIndexCompatibilityVersion(); + } try { snapshotIndexMetadata = metadataIndexUpgradeService.upgradeIndexMetadata( snapshotIndexMetadata, @@ -1254,4 +1263,9 @@ private static IndexMetadata addSnapshotToIndexSettings(IndexMetadata metadata, .build(); return IndexMetadata.builder(metadata).settings(newSettings).build(); } + + private static boolean isSearchableSnapshotsExtendedCompatibilityEnabled() { + return org.opensearch.Version.CURRENT.after(org.opensearch.Version.V_2_4_0) + && FeatureFlags.isEnabled(SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY); + } } diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 5c2efba008652..2be7cf9d4dbb3 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -173,7 +173,7 @@ public static void validateSnapshotsBackingAnyIndex( for (ObjectCursor cursor : metadata.values()) { IndexMetadata indexMetadata = cursor.value; String storeType = indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()); - if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(storeType)) { + if (IndexModule.Type.REMOTE_SNAPSHOT.match(storeType)) { String snapshotId = indexMetadata.getSettings().get(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey()); if (uuidToSnapshotId.get(snapshotId) != null) { snapshotsToBeNotDeleted.add(uuidToSnapshotId.get(snapshotId).getName()); diff --git a/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java b/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java index 0edcd55cc35c3..bcbe1b0320f49 100644 --- a/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java +++ b/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java @@ -31,6 +31,10 @@ package org.opensearch.common.lucene; +import org.apache.lucene.document.LatLonPoint; +import org.apache.lucene.index.IndexCommit; +import org.apache.lucene.index.IndexFormatTooOldException; +import org.apache.lucene.index.StandardDirectoryReader; import org.apache.lucene.tests.analysis.MockAnalyzer; import org.apache.lucene.analysis.core.KeywordAnalyzer; import org.apache.lucene.document.Document; @@ -71,8 +75,11 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.store.MMapDirectory; import org.apache.lucene.tests.store.MockDirectoryWrapper; +import org.apache.lucene.tests.util.TestUtil; import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; +import org.opensearch.LegacyESVersion; +import org.opensearch.Version; import org.opensearch.common.collect.Tuple; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.internal.io.IOUtils; @@ -87,6 +94,7 @@ import java.io.IOException; import java.io.StringReader; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -368,6 +376,92 @@ public void testNumDocs() throws IOException { dir.close(); } + /** + * Tests whether old segments are readable and queryable based on the data documented + * in the README here. + * + * @throws IOException + */ + public void testReadSegmentInfosExtendedCompatibility() throws IOException { + final String pathToTestIndex = "/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip"; + final Version minVersion = LegacyESVersion.fromId(6000099); + Path tmp = createTempDir(); + TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp); + try (MockDirectoryWrapper dir = newMockFSDirectory(tmp)) { + // The standard API will throw an exception + expectThrows(IndexFormatTooOldException.class, () -> Lucene.readSegmentInfos(dir)); + SegmentInfos si = Lucene.readSegmentInfosExtendedCompatibility(dir, minVersion); + assertEquals(1, Lucene.getNumDocs(si)); + IndexCommit indexCommit = Lucene.getIndexCommit(si, dir); + // uses the "expert" Lucene API + try ( + StandardDirectoryReader reader = (StandardDirectoryReader) DirectoryReader.open( + indexCommit, + minVersion.minimumIndexCompatibilityVersion().luceneVersion.major, + null + ) + ) { + IndexSearcher searcher = newSearcher(reader); + // radius too small, should get no results + assertFalse(Lucene.exists(searcher, LatLonPoint.newDistanceQuery("testLocation", 48.57532, -112.87695, 2))); + assertTrue(Lucene.exists(searcher, LatLonPoint.newDistanceQuery("testLocation", 48.57532, -112.87695, 20000))); + } + } + } + + /** + * Since the implementation in {@link Lucene#readSegmentInfosExtendedCompatibility(Directory, Version)} + * is a workaround, this test verifies that the response from this method is equivalent to + * {@link Lucene#readSegmentInfos(Directory)} if the version is N-1 + */ + public void testReadSegmentInfosExtendedCompatibilityBaseCase() throws IOException { + MockDirectoryWrapper dir = newMockDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + IndexWriter writer = new IndexWriter(dir, iwc); + Document doc = new Document(); + doc.add(new TextField("id", "1", random().nextBoolean() ? Field.Store.YES : Field.Store.NO)); + writer.addDocument(doc); + writer.commit(); + SegmentInfos expectedSI = Lucene.readSegmentInfos(dir); + SegmentInfos actualSI = Lucene.readSegmentInfosExtendedCompatibility(dir, Version.CURRENT); + assertEquals(Lucene.getNumDocs(expectedSI), Lucene.getNumDocs(actualSI)); + assertEquals(expectedSI.getGeneration(), actualSI.getGeneration()); + assertEquals(expectedSI.getSegmentsFileName(), actualSI.getSegmentsFileName()); + assertEquals(expectedSI.getVersion(), actualSI.getVersion()); + assertEquals(expectedSI.getCommitLuceneVersion(), actualSI.getCommitLuceneVersion()); + assertEquals(expectedSI.getMinSegmentLuceneVersion(), actualSI.getMinSegmentLuceneVersion()); + assertEquals(expectedSI.getIndexCreatedVersionMajor(), actualSI.getIndexCreatedVersionMajor()); + assertEquals(expectedSI.getUserData(), actualSI.getUserData()); + + int numDocsToIndex = randomIntBetween(10, 50); + List deleteTerms = new ArrayList<>(); + for (int i = 0; i < numDocsToIndex; i++) { + doc = new Document(); + doc.add(new TextField("id", "doc_" + i, random().nextBoolean() ? Field.Store.YES : Field.Store.NO)); + deleteTerms.add(new Term("id", "doc_" + i)); + writer.addDocument(doc); + } + int numDocsToDelete = randomIntBetween(0, numDocsToIndex); + Collections.shuffle(deleteTerms, random()); + for (int i = 0; i < numDocsToDelete; i++) { + Term remove = deleteTerms.remove(0); + writer.deleteDocuments(remove); + } + writer.commit(); + expectedSI = Lucene.readSegmentInfos(dir); + actualSI = Lucene.readSegmentInfosExtendedCompatibility(dir, Version.CURRENT); + assertEquals(Lucene.getNumDocs(expectedSI), Lucene.getNumDocs(actualSI)); + assertEquals(expectedSI.getGeneration(), actualSI.getGeneration()); + assertEquals(expectedSI.getSegmentsFileName(), actualSI.getSegmentsFileName()); + assertEquals(expectedSI.getVersion(), actualSI.getVersion()); + assertEquals(expectedSI.getCommitLuceneVersion(), actualSI.getCommitLuceneVersion()); + assertEquals(expectedSI.getMinSegmentLuceneVersion(), actualSI.getMinSegmentLuceneVersion()); + assertEquals(expectedSI.getIndexCreatedVersionMajor(), actualSI.getIndexCreatedVersionMajor()); + assertEquals(expectedSI.getUserData(), actualSI.getUserData()); + writer.close(); + dir.close(); + } + public void testCount() throws Exception { Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir); diff --git a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java index 8b53e5fe51635..b145ec0d79311 100644 --- a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java @@ -34,8 +34,9 @@ import org.opensearch.common.inject.ModuleTestCase; import org.opensearch.common.settings.Setting.Property; -import org.opensearch.common.util.FeatureFlagTests; import org.hamcrest.Matchers; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.test.FeatureFlagSetter; import java.util.Arrays; @@ -239,46 +240,48 @@ public void testOldMaxClauseCountSetting() { ); } - public void testDynamicNodeSettingsRegistration() { - FeatureFlagTests.enableFeature(); - Settings settings = Settings.builder().put("some.custom.setting", "2.0").build(); - SettingsModule module = new SettingsModule(settings, Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)); - assertNotNull(module.getClusterSettings().get("some.custom.setting")); - // For unregistered setting the value is expected to be null - assertNull(module.getClusterSettings().get("some.custom.setting2")); - assertInstanceBinding(module, Settings.class, (s) -> s == settings); + public void testDynamicNodeSettingsRegistration() throws Exception { + try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.EXTENSIONS)) { + Settings settings = Settings.builder().put("some.custom.setting", "2.0").build(); + SettingsModule module = new SettingsModule(settings, Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)); + assertNotNull(module.getClusterSettings().get("some.custom.setting")); + // For unregistered setting the value is expected to be null + assertNull(module.getClusterSettings().get("some.custom.setting2")); + assertInstanceBinding(module, Settings.class, (s) -> s == settings); - assertTrue(module.registerDynamicSetting(Setting.floatSetting("some.custom.setting2", 1.0f, Property.NodeScope))); - assertNotNull(module.getClusterSettings().get("some.custom.setting2")); - // verify if some.custom.setting still exists - assertNotNull(module.getClusterSettings().get("some.custom.setting")); + assertTrue(module.registerDynamicSetting(Setting.floatSetting("some.custom.setting2", 1.0f, Property.NodeScope))); + assertNotNull(module.getClusterSettings().get("some.custom.setting2")); + // verify if some.custom.setting still exists + assertNotNull(module.getClusterSettings().get("some.custom.setting")); - // verify exception is thrown when setting registration fails - expectThrows( - SettingsException.class, - () -> module.registerDynamicSetting(Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)) - ); + // verify exception is thrown when setting registration fails + expectThrows( + SettingsException.class, + () -> module.registerDynamicSetting(Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)) + ); + } } - public void testDynamicIndexSettingsRegistration() { - FeatureFlagTests.enableFeature(); - Settings settings = Settings.builder().put("some.custom.setting", "2.0").build(); - SettingsModule module = new SettingsModule(settings, Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)); - assertNotNull(module.getClusterSettings().get("some.custom.setting")); - // For unregistered setting the value is expected to be null - assertNull(module.getIndexScopedSettings().get("index.custom.setting2")); - assertInstanceBinding(module, Settings.class, (s) -> s == settings); + public void testDynamicIndexSettingsRegistration() throws Exception { + try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.EXTENSIONS)) { + Settings settings = Settings.builder().put("some.custom.setting", "2.0").build(); + SettingsModule module = new SettingsModule(settings, Setting.floatSetting("some.custom.setting", 1.0f, Property.NodeScope)); + assertNotNull(module.getClusterSettings().get("some.custom.setting")); + // For unregistered setting the value is expected to be null + assertNull(module.getIndexScopedSettings().get("index.custom.setting2")); + assertInstanceBinding(module, Settings.class, (s) -> s == settings); - assertTrue(module.registerDynamicSetting(Setting.floatSetting("index.custom.setting2", 1.0f, Property.IndexScope))); - assertNotNull(module.getIndexScopedSettings().get("index.custom.setting2")); + assertTrue(module.registerDynamicSetting(Setting.floatSetting("index.custom.setting2", 1.0f, Property.IndexScope))); + assertNotNull(module.getIndexScopedSettings().get("index.custom.setting2")); - // verify if some.custom.setting still exists - assertNotNull(module.getClusterSettings().get("some.custom.setting")); + // verify if some.custom.setting still exists + assertNotNull(module.getClusterSettings().get("some.custom.setting")); - // verify exception is thrown when setting registration fails - expectThrows( - SettingsException.class, - () -> module.registerDynamicSetting(Setting.floatSetting("index.custom.setting2", 1.0f, Property.IndexScope)) - ); + // verify exception is thrown when setting registration fails + expectThrows( + SettingsException.class, + () -> module.registerDynamicSetting(Setting.floatSetting("index.custom.setting2", 1.0f, Property.IndexScope)) + ); + } } } diff --git a/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java b/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java index 05ede515e042c..ca16efdf11d7d 100644 --- a/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java +++ b/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java @@ -8,31 +8,23 @@ package org.opensearch.common.util; -import org.junit.BeforeClass; -import org.opensearch.common.SuppressForbidden; +import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchTestCase; -import java.security.AccessController; -import java.security.PrivilegedAction; - public class FeatureFlagTests extends OpenSearchTestCase { - @SuppressForbidden(reason = "sets the feature flag") - @BeforeClass - public static void enableFeature() { - AccessController.doPrivileged((PrivilegedAction) () -> System.setProperty(FeatureFlags.REPLICATION_TYPE, "true")); - AccessController.doPrivileged((PrivilegedAction) () -> System.setProperty(FeatureFlags.REMOTE_STORE, "true")); - AccessController.doPrivileged((PrivilegedAction) () -> System.setProperty(FeatureFlags.EXTENSIONS, "true")); - } + private final String FLAG_PREFIX = "opensearch.experimental.feature."; - public void testReplicationTypeFeatureFlag() { - String replicationTypeFlag = FeatureFlags.REPLICATION_TYPE; - assertNotNull(System.getProperty(replicationTypeFlag)); - assertTrue(FeatureFlags.isEnabled(replicationTypeFlag)); + public void testFeatureFlagSet() throws Exception { + final String testFlag = FLAG_PREFIX + "testFlag"; + try (FeatureFlagSetter f = FeatureFlagSetter.set(testFlag)) { + assertNotNull(System.getProperty(testFlag)); + assertTrue(FeatureFlags.isEnabled(testFlag)); + } } public void testMissingFeatureFlag() { - String testFlag = "missingFeatureFlag"; + final String testFlag = FLAG_PREFIX + "testFlag"; assertNull(System.getProperty(testFlag)); assertFalse(FeatureFlags.isEnabled(testFlag)); } @@ -42,11 +34,4 @@ public void testNonBooleanFeatureFlag() { assertNotNull(System.getProperty(javaVersionProperty)); assertFalse(FeatureFlags.isEnabled(javaVersionProperty)); } - - public void testRemoteStoreFeatureFlag() { - String remoteStoreFlag = FeatureFlags.REMOTE_STORE; - assertNotNull(System.getProperty(remoteStoreFlag)); - assertTrue(FeatureFlags.isEnabled(remoteStoreFlag)); - } - } diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 47820ee739c49..af51dfa49ed6a 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -60,7 +60,7 @@ import org.opensearch.common.settings.WriteableSetting.SettingType; import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.transport.TransportAddress; -import org.opensearch.common.util.FeatureFlagTests; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.env.Environment; @@ -75,6 +75,7 @@ import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.plugins.PluginInfo; import org.opensearch.rest.RestController; +import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.IndexSettingsModule; import org.opensearch.test.MockLogAppender; import org.opensearch.test.OpenSearchTestCase; @@ -90,6 +91,7 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { + private FeatureFlagSetter featureFlagSetter; private TransportService transportService; private RestController restController; private SettingsModule settingsModule; @@ -137,7 +139,7 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { @Before public void setup() throws Exception { - FeatureFlagTests.enableFeature(); + featureFlagSetter = FeatureFlagSetter.set(FeatureFlags.EXTENSIONS); Settings settings = Settings.builder().put("cluster.name", "test").build(); transport = new MockNioTransport( settings, @@ -207,6 +209,7 @@ public void tearDown() throws Exception { transportService.close(); client.close(); ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + featureFlagSetter.close(); } public void testDiscover() throws Exception { diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index de5ef8851ae80..e0f1066587e5b 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -34,6 +34,7 @@ import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.SuppressForbidden; import org.opensearch.common.settings.AbstractScopedSettings; import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Setting; @@ -41,8 +42,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.translog.Translog; import org.opensearch.indices.replication.common.ReplicationType; +import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; @@ -57,6 +60,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.core.StringContains.containsString; import static org.hamcrest.object.HasToString.hasToString; +import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; public class IndexSettingsTests extends OpenSearchTestCase { @@ -943,4 +947,46 @@ public void testSetRemoteRepositoryFailsWhenEmptyString() { ); assertEquals("Setting index.remote_store.repository should be provided with non-empty repository ID", iae.getMessage()); } + + @SuppressForbidden(reason = "sets the SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY feature flag") + public void testExtendedCompatibilityVersionForRemoteSnapshot() throws Exception { + try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY)) { + IndexMetadata metadata = newIndexMeta( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .build() + ); + IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); + assertTrue(settings.isRemoteSnapshot()); + assertEquals(SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION, settings.getExtendedCompatibilitySnapshotVersion()); + } + } + + public void testExtendedCompatibilityVersionForNonRemoteSnapshot() { + IndexMetadata metadata = newIndexMeta( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey()) + .build() + ); + IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); + assertFalse(settings.isRemoteSnapshot()); + assertEquals(Version.CURRENT.minimumIndexCompatibilityVersion(), settings.getExtendedCompatibilitySnapshotVersion()); + } + + public void testExtendedCompatibilityVersionWithoutFeatureFlag() { + IndexMetadata metadata = newIndexMeta( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .build() + ); + IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); + assertTrue(settings.isRemoteSnapshot()); + assertEquals(Version.CURRENT.minimumIndexCompatibilityVersion(), settings.getExtendedCompatibilitySnapshotVersion()); + } } diff --git a/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java b/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java index 5faa670ba4481..81a364c418c95 100644 --- a/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java +++ b/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java @@ -32,19 +32,30 @@ package org.opensearch.index.engine; import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexFormatTooOldException; import org.apache.lucene.index.IndexReader; import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; import org.opensearch.Version; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.internal.io.IOUtils; +import org.opensearch.index.IndexModule; +import org.opensearch.index.IndexSettings; import org.opensearch.index.mapper.ParsedDocument; import org.opensearch.index.seqno.SeqNoStats; import org.opensearch.index.seqno.SequenceNumbers; import org.opensearch.index.store.Store; import org.opensearch.index.translog.TranslogStats; +import org.opensearch.test.FeatureFlagSetter; +import org.opensearch.test.IndexSettingsModule; import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Path; import java.util.List; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; @@ -228,6 +239,49 @@ public void testReadOnly() throws IOException { } } + public void testReadOldIndices() throws Exception { + IOUtils.close(engine, store); + // The index has one document in it, so the checkpoint cannot be NO_OPS_PERFORMED + final AtomicLong globalCheckpoint = new AtomicLong(0); + final String pathToTestIndex = "/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip"; + Path tmp = createTempDir(); + TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp); + try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY)) { + final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .build() + ); + try (Store store = createStore(newFSDirectory(tmp))) { + EngineConfig config = config(indexSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get); + try ( + ReadOnlyEngine readOnlyEngine = new ReadOnlyEngine(config, null, new TranslogStats(), true, Function.identity(), true) + ) { + assertVisibleCount(readOnlyEngine, 1, false); + } + } + } + } + + public void testReadOldIndicesFailure() throws IOException { + IOUtils.close(engine, store); + // The index has one document in it, so the checkpoint cannot be NO_OPS_PERFORMED + final AtomicLong globalCheckpoint = new AtomicLong(0); + final String pathToTestIndex = "/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip"; + Path tmp = createTempDir(); + TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp); + try (Store store = createStore(newFSDirectory(tmp))) { + EngineConfig config = config(defaultSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get); + try { + new ReadOnlyEngine(config, null, new TranslogStats(), true, Function.identity(), true); + } catch (UncheckedIOException e) { + assertEquals(IndexFormatTooOldException.class, e.getCause().getClass()); + } + } + } + /** * Test that {@link ReadOnlyEngine#verifyEngineBeforeIndexClosing()} never fails * whatever the value of the global checkpoint to check is. diff --git a/server/src/test/java/org/opensearch/index/store/StoreTests.java b/server/src/test/java/org/opensearch/index/store/StoreTests.java index dc6cf4c187f61..7f5340096ab86 100644 --- a/server/src/test/java/org/opensearch/index/store/StoreTests.java +++ b/server/src/test/java/org/opensearch/index/store/StoreTests.java @@ -67,15 +67,18 @@ import org.hamcrest.Matchers; import org.opensearch.ExceptionsHelper; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.SuppressForbidden; import org.opensearch.common.UUIDs; import org.opensearch.common.io.stream.InputStreamStreamInput; import org.opensearch.common.io.stream.OutputStreamStreamOutput; import org.opensearch.common.lucene.Lucene; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.internal.io.IOUtils; import org.opensearch.env.ShardLock; import org.opensearch.index.Index; +import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.seqno.ReplicationTracker; @@ -84,6 +87,7 @@ import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.indices.store.TransportNodesListShardStoreMetadata; import org.opensearch.test.DummyShardLock; +import org.opensearch.test.FeatureFlagSetter; import org.opensearch.test.IndexSettingsModule; import org.opensearch.test.OpenSearchTestCase; @@ -116,6 +120,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; +import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; import static org.opensearch.test.VersionUtils.randomVersion; public class StoreTests extends OpenSearchTestCase { @@ -1257,6 +1262,49 @@ public void testSegmentReplicationDiff() { assertTrue(diff.identical.isEmpty()); } + @SuppressForbidden(reason = "sets the SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY feature flag") + public void testReadSegmentsFromOldIndices() throws Exception { + int expectedIndexCreatedVersionMajor = SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION.luceneVersion.major; + final String pathToTestIndex = "/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip"; + Path tmp = createTempDir(); + TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp); + final ShardId shardId = new ShardId("index", "_na_", 1); + Store store = null; + + try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY)) { + IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .build() + ); + store = new Store(shardId, indexSettings, StoreTests.newMockFSDirectory(tmp), new DummyShardLock(shardId)); + assertEquals(expectedIndexCreatedVersionMajor, store.readLastCommittedSegmentsInfo().getIndexCreatedVersionMajor()); + } finally { + if (store != null) { + store.close(); + } + } + } + + public void testReadSegmentsFromOldIndicesFailure() throws IOException { + final String pathToTestIndex = "/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip"; + final ShardId shardId = new ShardId("index", "_na_", 1); + Path tmp = createTempDir(); + TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp); + IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( + "index", + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey()) + .build() + ); + Store store = new Store(shardId, indexSettings, StoreTests.newMockFSDirectory(tmp), new DummyShardLock(shardId)); + assertThrows(IndexFormatTooOldException.class, store::readLastCommittedSegmentsInfo); + store.close(); + } + private void commitRandomDocs(Store store) throws IOException { IndexWriter writer = indexRandomDocs(store); writer.commit(); diff --git a/server/src/test/resources/indices/bwc/es-6.3.0/README.md b/server/src/test/resources/indices/bwc/es-6.3.0/README.md new file mode 100644 index 0000000000000..a9f969475aaad --- /dev/null +++ b/server/src/test/resources/indices/bwc/es-6.3.0/README.md @@ -0,0 +1,57 @@ +# README for _testIndex-es-6.3.0.zip_ + +This zip file holds a Lucene index created using ElasticSearch 6.3.0. +It was created by running the underlying commands against a single-node cluster, +then compressing the contents of the underlying Lucene index directory i.e. +the files under `/data/nodes/0/indices//0/index`. +The index contains one document. + +## Commands + +``` +curl -X PUT -H 'Content-Type: application/json' 'localhost:9200/testindex?pretty' -d' +{ + "settings": { + "number_of_shards": 1, + "number_of_replicas": 0 + }, + "mappings": { + "testData": { + "properties": { + "id": { "type": "keyword" }, + "isTestData": { "type": "boolean" }, + "testNum": { "type": "short" }, + "testRange": {"type": "integer_range" }, + "testMessage": { + "type": "text", + "fields": { + "length": { + "type": "token_count", + "analyzer": "standard" + } + } + }, + "testBlob": { "type": "binary", "index": false }, + "testDate": { "type": "date" }, + "testLocation": { "type": "geo_point"} + } + } + } +}' + +curl -X POST "localhost:9200/testindex/testData/?pretty" -H 'Content-Type: application/json' -d' +{ + "id": "testData1", + "isTestData": true, + "testNum": 99, + "testRange": { + "gte": 0, + "lte": 100 + }, + "testMessage": "The OpenSearch Project", + "testBlob": "VGhlIE9wZW5TZWFyY2ggUHJvamVjdA==", + "testDate": "1970-01-02", + "testLocation": "48.553532,-113.022881" +} +' +``` diff --git a/server/src/test/resources/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip b/server/src/test/resources/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip new file mode 100644 index 0000000000000000000000000000000000000000..db86a76153b25b599dd5ba0a37042b1f44592e27 GIT binary patch literal 2467 zcmZ{mc|26>AIHbomnI^z#MqY@F&M?z`MH)t80%PO#+oc+@5)Y5GE9sy&DhsML=oB7 z<{}|=DZ5DBOehL>s^5LJyzc$uJkN8U^LoF|>-%}W=X^d^=8Q~y004ju@Y*%rPVGjQ z=rH~61_J=VLZ97ELQr0qhIXH%p>8XSxw{=ZOd#e4Y~(z~sGmP@%m$ z^=WHtzpdu!?QAdTJ6eA?zF+uN>7p!DFHRIcY=~1Hq$a10#9LkMl%p~4^jd9vunJ>P zjE=&{b8BS7ZAXd5%1Wg`1qbG%T>1@_0EJkPLc2&wB~zCdB-x3V=?NMXeo1gVO0mCi z?RSSw8&#;d2*{)2w@x2o!SI4<;!tX$n!e@y6lbG6$0AP!I9Jma!!c?Zb+Xfs?GIwC z9*V|U0c1h-Ix7RLeyX@`DkEu|2>{qV{6odqKkUsdrcI8K1usUlu^a(sCea4!eXAuH zHXzSv72K9X#Zr2vQwc)Gj;FTkG-nz+x5o+;#zYgwQ=f|{sHY-}?%p#QDov{@s4}%X ze5wQ&q7PR~C2QK+T^YBp8XFa?J@f)rRd2)Ex{$YX@YlhfuD6e5aD|72w8zo^CBg14 z7UkG1c|7K05Ij7r9kY>p`}#rT=FB}w;ab;S5iXuJnbc?}sD<%0VQh=1XN*HO$DMbe zFblo-@KF3#WDJQ?TUGPyUVBr`zMkPir(tS7(&vJ2fL8tfP@ z+|qxf#TEA;5P6yW7%Bl@^Bz??+^x4b7H8IU`{O`>&1&}YCutE=dyEAmPtd~5o0Da7 zNY|7i1YxWqHLW)zzY+De)^h0iTHFYMn6YHdEQ?0js!@aO`Veg5)I|Lpan=g>Q=Blb zOSVH=6j;ksq$EVV`|7X}d!B$Q6=h_M4y^yc5ukITZnYT`uzlw8qb5(W<_VWi+>|md zrCyyG4fp-r@+UnEY%$4APPF;*RhIde3ykpv60EGi&YKSG;Tj3WCU-%KxPWtK=Mn*} zO!nnAztz9}tCn#UTO`zvR$(t=zc_u1p|Oi6D4|t$JH9s)%spOx2%s>#E~UYx0wf1@ zz5;?s;tVhpvy1y_URpK4XEw}p-yGIS32Q)cUfICyM~sXZ7p<(AAYOp_7XsU&{2VQY zkvV6%Kq>lK);!aPQg$yUL&V3WlbrRU4YG~SJs!u-M(jtrMM#bDuChmjCmXo z3UJU6QEW5(F|@N%%W1EX@%SP~6&~T6WkX`ux;f-K}$(i6Czo z5!E^OSg?GbsN|`f-UKk`Dmy&LDc{@VWFz%HXvIF)gRb+pljfz^f0gRliSG7B5J- zBCq;pFN1IV^tG_`!RB7H_Dgu6CFzN4I!>^1Cu)grJ=Ehtc9kMrk0 zW0}zTGC{fz1kf+$Z(e}){b~j4->l#QH;G8jCnDzaa&t!h#Rd|4rs4^LmjwlJr`#DJ zWuHAkFMH6;2kyyiZ*TR~8@1Jsx8grbY-f&7j}=k1MJpVvgS{?^mKTzf$@;B#DtUB` zGzDb3G3Y8*zDY~fy4gnz54dEy{c5~#c@L1?Zsj`QNuBl{9{hn`62M8MEfJBG$B7u^ zgdIn7IR{_zwGL76cRJ%6%pzc=Mb$QBgd0N7WcJNx^-KeihES-vxg;+muOt@B>(vy= z*tU4r)*fs}VxQxK4jBR;jf3w?wacmZDnZp{OQ)3DqEuY+uW4jynG!!bJ`bFtfB>#)M8``QU$V`6adb(G7!6-iRhFLRz_7Wvm6ux zO6GKh0Qp@qnb{N)ftNzU5lOC@lsrV24>&vDfya%AAQ5teC=O0pVwAiiZdrNxPA*vl z=Z+Hp0cRUL0YS>9h`Kou^Cb8&F2`mC*T@3v6kn1L>Ghg}*E*ViARFiClI3R0ZoP0s zY!>OUD9VGkIfh*FuqUH(#5}DaTrj!gubvn6WKlnBo1h?>TfLRZ8&Nw~SydK$O3gIRj9X@xL?3KV(V#m({mK@+) () -> System.setProperty(flag, "true")); + return new FeatureFlagSetter(flag); + } + + @SuppressForbidden(reason = "Clears the set feature flag on close") + @Override + public void close() throws Exception { + AccessController.doPrivileged((PrivilegedAction) () -> System.clearProperty(this.flag)); + } +}