Skip to content

Commit

Permalink
Adding more tests, and some more code improvements
Browse files Browse the repository at this point in the history 
Signed-off-by: Kartik Ganesh <gkart@amazon.com>
  • Loading branch information
@kartg
kartg committed Dec 6, 2022
1 parent f3e909b commit 54a8ebf
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 18 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME,
IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID
),
FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_BWC,
FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY,
List.of(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION)
);

Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/org/opensearch/common/util/FeatureFlags.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ public class FeatureFlags {

/**
* Gates the ability for Searchable Snapshots to read snapshots that are older than the
* guaranteed backward compatibilty for OpenSearch (one prior major version) on a best effort basis.
* guaranteed backward compatibility for OpenSearch (one prior major version) on a best effort basis.
*/
public static final String SEARCHABLE_SNAPSHOT_EXTENDED_BWC =
"opensearch.experimental.feature.searchable_snapshot.extended_bwc.enabled";
public static final String SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY =
"opensearch.experimental.feature.searchable_snapshot.extended_compatibility.enabled";

/**
* Gates the functionality of extensions.
Expand Down
42 changes: 28 additions & 14 deletions server/src/main/java/org/opensearch/snapshots/RestoreService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_BWC;
import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY;
import static org.opensearch.common.util.set.Sets.newHashSet;
import static org.opensearch.snapshots.SnapshotUtils.filterIndices;

Expand Down Expand Up @@ -443,14 +443,7 @@ public ClusterState execute(ClusterState currentState) {
// We have some indices to restore
ImmutableOpenMap.Builder<ShardId, RestoreInProgress.ShardRestoreStatus> shardsBuilder = ImmutableOpenMap
.builder();
final Version minIndexCompatibilityVersion;
if (isSearchableSnapshotsExtendedBWCEnabled()) {
minIndexCompatibilityVersion = LegacyESVersion.fromId(6000099).minimumIndexCompatibilityVersion();
} else {
minIndexCompatibilityVersion = currentState.getNodes()
.getMaxNodeVersion()
.minimumIndexCompatibilityVersion();
}

for (Map.Entry<String, String> indexEntry : indices.entrySet()) {
String renamedIndexName = indexEntry.getKey();
String index = indexEntry.getValue();
Expand All @@ -477,6 +470,17 @@ public ClusterState execute(ClusterState currentState) {
repositoryData.resolveIndexId(index),
isSearchableSnapshot
);
final Version minIndexCompatibilityVersion;
if (isSearchableSnapshot && isSearchableSnapshotsExtendedCompatibilityEnabled()) {
final int minVersion = IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.get(
snapshotIndexMetadata.getSettings()
);
minIndexCompatibilityVersion = LegacyESVersion.fromId(minVersion).minimumIndexCompatibilityVersion();
} else {
minIndexCompatibilityVersion = currentState.getNodes()
.getMaxNodeVersion()
.minimumIndexCompatibilityVersion();
}
try {
snapshotIndexMetadata = metadataIndexUpgradeService.upgradeIndexMetadata(
snapshotIndexMetadata,
Expand Down Expand Up @@ -1258,15 +1262,25 @@ private static IndexMetadata addSnapshotToIndexSettings(IndexMetadata metadata,
.put(IndexSettings.SEARCHABLE_SNAPSHOT_INDEX_ID.getKey(), indexId.getId())
.put(metadata.getSettings());
// Check for extended backwards compatibility feature flag
if (isSearchableSnapshotsExtendedBWCEnabled()) {
// currently, the oldest snapshot version we can successfully read is ES 6.0
settingsBuilder.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), 6000099);
if (isSearchableSnapshotsExtendedCompatibilityEnabled()) {
settingsBuilder.put(
IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(),
getSearchableSnapshotsExtendedCompatibilityMinimumVersion()
);
}
return IndexMetadata.builder(metadata).settings(settingsBuilder.build()).build();
}

private static boolean isSearchableSnapshotsExtendedBWCEnabled() {
/**
* Currently, the oldest snapshot version we can successfully read using extended
* compatibility for searchable snapshots is ES 6.0
*/
private static int getSearchableSnapshotsExtendedCompatibilityMinimumVersion() {
return 6000099;
}

private static boolean isSearchableSnapshotsExtendedCompatibilityEnabled() {
return org.opensearch.Version.CURRENT.after(org.opensearch.Version.V_2_4_0)
&& FeatureFlags.isEnabled(SEARCHABLE_SNAPSHOT_EXTENDED_BWC);
&& FeatureFlags.isEnabled(SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY);
}
}
44 changes: 44 additions & 0 deletions server/src/test/java/org/opensearch/index/IndexSettingsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.index;

import org.opensearch.LegacyESVersion;
import org.opensearch.Version;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.AbstractScopedSettings;
Expand Down Expand Up @@ -943,4 +944,47 @@ public void testSetRemoteRepositoryFailsWhenEmptyString() {
);
assertEquals("Setting index.remote_store.repository should be provided with non-empty repository ID", iae.getMessage());
}

public void testExtendedCompatibilityVersionForRemoteSnapshot() {
int version = randomIntBetween(1000000, 6999999);
Version expected = LegacyESVersion.fromId(version);
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())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), version)
.build()
);
IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
assertTrue(settings.isRemoteSnapshot());
assertEquals(expected, settings.getExtendedCompatibilitySnapshotVersion());
}

public void testExtendedCompatibilityVersionForNonSnapshot() {
IndexMetadata metadata = newIndexMeta(
"index",
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), 99)
.build()
);
IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
assertFalse(settings.isRemoteSnapshot());
assertNull(settings.getExtendedCompatibilitySnapshotVersion());
}

public void testExtendedCompatibilityVersionMissingKey() {
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());
assertNull(settings.getExtendedCompatibilitySnapshotVersion());
}
}
59 changes: 59 additions & 0 deletions server/src/test/java/org/opensearch/index/store/StoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.apache.lucene.util.Version;
import org.hamcrest.Matchers;
import org.opensearch.ExceptionsHelper;
import org.opensearch.LegacyESVersion;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.UUIDs;
import org.opensearch.common.io.stream.InputStreamStreamInput;
Expand All @@ -76,6 +77,7 @@
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;
Expand Down Expand Up @@ -1257,6 +1259,63 @@ public void testSegmentReplicationDiff() {
assertTrue(diff.identical.isEmpty());
}

public void testReadSegmentsFromOldIndices() throws IOException {
int version = 6030099;
int expectedIndexCreatedVersionMajor = LegacyESVersion.fromId(version).luceneVersion.major;
final String pathToTestIndex = "/indices/bwc/testIndex-6.3.0.zip";
Path tmp = createTempDir();
TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp);
final ShardId shardId = new ShardId("index", "_na_", 1);
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())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), String.valueOf(version))
.build()
);
Store store = new Store(shardId, indexSettings, StoreTests.newMockFSDirectory(tmp), new DummyShardLock(shardId));
assertEquals(expectedIndexCreatedVersionMajor, store.readLastCommittedSegmentsInfo().getIndexCreatedVersionMajor());
store.close();
}

public void testReadSegmentsFromOldIndicesFailure() throws IOException {
final String pathToTestIndex = "/indices/bwc/testIndex-6.3.0.zip";
final ShardId shardId = new ShardId("index", "_na_", 1);
ArrayList<Settings> testSettingsList = new ArrayList<>();
// non-snapshot use-case
testSettingsList.add(
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT)
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), String.valueOf(6030099))
.build()
);
// remote snapshot, but without the min-version key
testSettingsList.add(
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT)
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.build()
);
// remote snapshot, but min-version mismatch
testSettingsList.add(
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, org.opensearch.Version.CURRENT)
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), String.valueOf(2000099 ^ org.opensearch.Version.MASK))
.build()
);
for (Settings testSettings : testSettingsList) {
Path tmp = createTempDir();
TestUtil.unzip(getClass().getResourceAsStream(pathToTestIndex), tmp);
IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("index", testSettings);
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();
Expand Down

0 comments on commit 54a8ebf

Please sign in to comment.