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 0000000000000..db86a76153b25 Binary files /dev/null and b/server/src/test/resources/indices/bwc/es-6.3.0/testIndex-es-6.3.0.zip differ diff --git a/test/framework/src/main/java/org/opensearch/test/FeatureFlagSetter.java b/test/framework/src/main/java/org/opensearch/test/FeatureFlagSetter.java new file mode 100644 index 0000000000000..26e884e707964 --- /dev/null +++ b/test/framework/src/main/java/org/opensearch/test/FeatureFlagSetter.java @@ -0,0 +1,39 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.test; + +import org.opensearch.common.SuppressForbidden; + +import java.security.AccessController; +import java.security.PrivilegedAction; + +/** + * Helper class that wraps the lifecycle of setting and finally clearing of + * a {@link org.opensearch.common.util.FeatureFlags} string in an {@link AutoCloseable}. + */ +public class FeatureFlagSetter implements AutoCloseable { + + private final String flag; + + private FeatureFlagSetter(String flag) { + this.flag = flag; + } + + @SuppressForbidden(reason = "Enables setting of feature flags") + public static final FeatureFlagSetter set(String flag) { + AccessController.doPrivileged((PrivilegedAction) () -> 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)); + } +}