Skip to content

Commit f3e909b

Browse files
kartgkartg
committed
Some code improvements, and incorporating more PR feedback
Signed-off-by: Kartik Ganesh <[email protected]>
1 parent 8888c54 commit f3e909b

File tree

11 files changed

+48
-31
lines changed

11 files changed

+48
-31
lines changed

‎server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,7 @@ public Builder addBlocks(IndexMetadata indexMetadata) {
394394
if (IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.get(indexMetadata.getSettings())) {
395395
addIndexBlock(indexName, IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK);
396396
}
397-
if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()
398-
.equals(indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) {
397+
if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) {
399398
addIndexBlock(indexName, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE);
400399
}
401400
return this;

‎server/src/main/java/org/opensearch/cluster/routing/RoutingPool.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public static RoutingPool getShardPool(ShardRouting shard, RoutingAllocation all
6161
*/
6262
public static RoutingPool getIndexPool(IndexMetadata indexMetadata) {
6363
Settings indexSettings = indexMetadata.getSettings();
64-
if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(indexSettings.get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) {
64+
if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexSettings.get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()))) {
6565
return REMOTE_CAPABLE;
6666
}
6767
return LOCAL_ONLY;

‎server/src/main/java/org/opensearch/common/lucene/Lucene.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public static SegmentInfos readSegmentInfos(Directory directory) throws IOExcept
169169
* "expert" readLatestCommit API is currently package-private in Lucene. First, all commits in
170170
* the given {@link Directory} are listed - this result includes older Lucene commits. Then,
171171
* the latest index commit is opened via {@link DirectoryReader} by including a minimum supported
172-
* Lucene major version - this is based on the minimum index compatiblity version of OpenSearch.
172+
* Lucene major version based on the minimum compatibility of the given {@link org.opensearch.Version}.
173173
*/
174174
public static SegmentInfos readSegmentInfosExtendedCompatbility(Directory directory, org.opensearch.Version minimumVersion)
175175
throws IOException {

‎server/src/main/java/org/opensearch/index/IndexSettings.java

+27-1
Original file line numberDiff line numberDiff line change
@@ -575,8 +575,9 @@ public final class IndexSettings {
575575
Property.InternalIndex
576576
);
577577

578-
public static final Setting<String> SEARCHABLE_SNAPSHOT_MINIMUM_VERSION = Setting.simpleString(
578+
public static final Setting<Integer> SEARCHABLE_SNAPSHOT_MINIMUM_VERSION = Setting.intSetting(
579579
"index.searchable_snapshot.minversion",
580+
0,
580581
Property.IndexScope,
581582
Property.InternalIndex
582583
);
@@ -591,6 +592,8 @@ public final class IndexSettings {
591592
private final boolean isRemoteStoreEnabled;
592593
private final String remoteStoreRepository;
593594
private final boolean isRemoteTranslogStoreEnabled;
595+
private final boolean isRemoteSnapshot;
596+
private Version extendedCompatibilitySnapshotVersion;
594597
// volatile fields are updated via #updateIndexMetadata(IndexMetadata) under lock
595598
private volatile Settings settings;
596599
private volatile IndexMetadata indexMetadata;
@@ -753,6 +756,13 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
753756
isRemoteStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false);
754757
remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY);
755758
isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false);
759+
isRemoteSnapshot = IndexModule.Type.REMOTE_SNAPSHOT.match(this);
760+
if (isRemoteSnapshot) {
761+
Integer minVersion = settings.getAsInt(SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.getKey(), 0);
762+
if (minVersion > 0) {
763+
extendedCompatibilitySnapshotVersion = LegacyESVersion.fromId(minVersion);
764+
}
765+
}
756766
this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings);
757767
this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings);
758768
this.queryStringAnalyzeWildcard = QUERY_STRING_ANALYZE_WILDCARD.get(nodeSettings);
@@ -1018,6 +1028,22 @@ public boolean isRemoteTranslogStoreEnabled() {
10181028
return isRemoteTranslogStoreEnabled;
10191029
}
10201030

1031+
/**
1032+
* Returns true if this is remote/searchable snapshot
1033+
*/
1034+
public boolean isRemoteSnapshot() {
1035+
return isRemoteSnapshot;
1036+
}
1037+
1038+
/**
1039+
* If this is a remote snapshot and the extended compatibility
1040+
* feature flag is enabled, this returns the minimum {@link Version}
1041+
* supported. In all other cases, the return value is null.
1042+
*/
1043+
public Version getExtendedCompatibilitySnapshotVersion() {
1044+
return extendedCompatibilitySnapshotVersion;
1045+
}
1046+
10211047
/**
10221048
* Returns the node settings. The settings returned from {@link #getSettings()} are a merged version of the
10231049
* index settings and the node settings where node settings are overwritten by index settings.

‎server/src/main/java/org/opensearch/index/shard/IndexShard.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -1984,7 +1984,7 @@ public void openEngineAndRecoverFromTranslog() throws IOException {
19841984
};
19851985

19861986
// Do not load the global checkpoint if this is a remote snapshot index
1987-
if (isRemoteSnapshotIndex() == false) {
1987+
if (indexSettings.isRemoteSnapshot() == false) {
19881988
loadGlobalCheckpointToReplicationTracker();
19891989
}
19901990

@@ -2040,10 +2040,12 @@ private void innerOpenEngineAndTranslog(LongSupplier globalCheckpointSupplier) t
20402040

20412041
private boolean assertSequenceNumbersInCommit() throws IOException {
20422042
final Map<String, String> userData;
2043-
if (isRemoteSnapshotIndex() && IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.exists(indexSettings.getSettings())) {
2044-
int minimumVersion = Integer.parseInt(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.get(indexSettings.getSettings()));
2043+
if (indexSettings.isRemoteSnapshot() && indexSettings.getExtendedCompatibilitySnapshotVersion() != null) {
20452044
// Inefficient method to support reading old Lucene indexes
2046-
userData = Lucene.readSegmentInfosExtendedCompatbility(store.directory(), LegacyESVersion.fromId(minimumVersion)).getUserData();
2045+
userData = Lucene.readSegmentInfosExtendedCompatbility(
2046+
store.directory(),
2047+
indexSettings.getExtendedCompatibilitySnapshotVersion()
2048+
).getUserData();
20472049
} else {
20482050
userData = SegmentInfos.readLatestCommit(store.directory()).getUserData();
20492051
}
@@ -2061,10 +2063,6 @@ private boolean assertSequenceNumbersInCommit() throws IOException {
20612063
return true;
20622064
}
20632065

2064-
private boolean isRemoteSnapshotIndex() {
2065-
return IndexModule.Type.REMOTE_SNAPSHOT.match(indexSettings);
2066-
}
2067-
20682066
private void onNewEngine(Engine newEngine) {
20692067
assert Thread.holdsLock(engineMutex);
20702068
refreshListeners.setCurrentRefreshLocationSupplier(newEngine::getTranslogLastWriteLocation);

‎server/src/main/java/org/opensearch/index/store/Store.java

+5-8
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import org.apache.lucene.util.BytesRefBuilder;
6565
import org.apache.lucene.util.Version;
6666
import org.opensearch.ExceptionsHelper;
67-
import org.opensearch.LegacyESVersion;
6867
import org.opensearch.common.Nullable;
6968
import org.opensearch.common.UUIDs;
7069
import org.opensearch.common.bytes.BytesReference;
@@ -87,7 +86,6 @@
8786
import org.opensearch.env.NodeEnvironment;
8887
import org.opensearch.env.ShardLock;
8988
import org.opensearch.env.ShardLockObtainFailedException;
90-
import org.opensearch.index.IndexModule;
9189
import org.opensearch.index.IndexSettings;
9290
import org.opensearch.index.engine.CombinedDeletionPolicy;
9391
import org.opensearch.index.engine.Engine;
@@ -223,10 +221,8 @@ public Directory directory() {
223221
public SegmentInfos readLastCommittedSegmentsInfo() throws IOException {
224222
failIfCorrupted();
225223
try {
226-
if (IndexModule.Type.REMOTE_SNAPSHOT.match(indexSettings)
227-
&& IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.exists(indexSettings.getSettings())) {
228-
int minimumVersion = Integer.parseInt(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.get(indexSettings.getSettings()));
229-
return readAnySegmentsInfo(directory(), LegacyESVersion.fromId(minimumVersion));
224+
if (indexSettings.isRemoteSnapshot() && indexSettings.getExtendedCompatibilitySnapshotVersion() != null) {
225+
return readSegmentInfosExtendedCompatbility(directory(), indexSettings.getExtendedCompatibilitySnapshotVersion());
230226
} else {
231227
return readSegmentsInfo(null, directory());
232228
}
@@ -239,7 +235,7 @@ public SegmentInfos readLastCommittedSegmentsInfo() throws IOException {
239235
/**
240236
* Returns the segments info for the given commit or for the latest commit if the given commit is <code>null</code>.
241237
* This method will throw an exception if the index is older than the standard backwards compatibility
242-
* policy ( current major - 1). See also {@link #readAnySegmentsInfo(Directory, org.opensearch.Version)}.
238+
* policy ( current major - 1). See also {@link #readSegmentInfosExtendedCompatbility(Directory, org.opensearch.Version)}.
243239
*
244240
* @throws IOException if the index is corrupted or the segments file is not present
245241
*/
@@ -264,7 +260,8 @@ private static SegmentInfos readSegmentsInfo(IndexCommit commit, Directory direc
264260
*
265261
* @throws IOException if the index is corrupted or the segments file is not present
266262
*/
267-
private static SegmentInfos readAnySegmentsInfo(Directory directory, org.opensearch.Version minimumVersion) throws IOException {
263+
private static SegmentInfos readSegmentInfosExtendedCompatbility(Directory directory, org.opensearch.Version minimumVersion)
264+
throws IOException {
268265
try {
269266
return Lucene.readSegmentInfosExtendedCompatbility(directory, minimumVersion);
270267
} catch (EOFException eof) {

‎server/src/main/java/org/opensearch/indices/IndicesService.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -767,17 +767,16 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) {
767767
if (idxSettings.isSegRepEnabled()) {
768768
return new NRTReplicationEngineFactory();
769769
}
770-
if (IndexModule.Type.REMOTE_SNAPSHOT.match(idxSettings)) {
771-
if (IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.exists(idxSettings.getSettings())) {
772-
int minimumVersion = Integer.parseInt(IndexSettings.SEARCHABLE_SNAPSHOT_MINIMUM_VERSION.get(idxSettings.getSettings()));
770+
if (idxSettings.isRemoteSnapshot()) {
771+
if (idxSettings.getExtendedCompatibilitySnapshotVersion() != null) {
773772
return config -> new ReadOnlyEngine(
774773
config,
775774
new SeqNoStats(0, 0, 0),
776775
new TranslogStats(),
777776
true,
778777
Function.identity(),
779778
false,
780-
LegacyESVersion.fromId(minimumVersion)
779+
idxSettings.getExtendedCompatibilitySnapshotVersion()
781780
);
782781
} else {
783782
return config -> new ReadOnlyEngine(

‎server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import org.opensearch.common.unit.TimeValue;
5555
import org.opensearch.common.util.CancellableThreads;
5656
import org.opensearch.common.util.concurrent.AbstractRunnable;
57-
import org.opensearch.index.IndexModule;
5857
import org.opensearch.index.IndexNotFoundException;
5958
import org.opensearch.index.engine.RecoveryEngineException;
6059
import org.opensearch.index.mapper.MapperException;
@@ -246,7 +245,7 @@ private void doRecovery(final long recoveryId, final StartRecoveryRequest preExi
246245
logger.trace("{} preparing shard for peer recovery", recoveryTarget.shardId());
247246
indexShard.prepareForIndexRecovery();
248247
final boolean hasRemoteTranslog = recoveryTarget.state().getPrimary() == false && indexShard.isRemoteTranslogEnabled();
249-
final boolean hasNoTranslog = IndexModule.Type.REMOTE_SNAPSHOT.match(indexShard.indexSettings());
248+
final boolean hasNoTranslog = indexShard.indexSettings().isRemoteSnapshot();
250249
final boolean verifyTranslog = (hasRemoteTranslog || hasNoTranslog) == false;
251250
final long startingSeqNo = indexShard.recoverLocallyAndFetchStartSeqNo(!hasRemoteTranslog);
252251
assert startingSeqNo == UNASSIGNED_SEQ_NO || recoveryTarget.state().getStage() == RecoveryState.Stage.TRANSLOG

‎server/src/main/java/org/opensearch/indices/recovery/RecoveryTarget.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import org.opensearch.common.bytes.BytesReference;
4646
import org.opensearch.common.lucene.Lucene;
4747
import org.opensearch.common.util.CancellableThreads;
48-
import org.opensearch.index.IndexModule;
4948
import org.opensearch.index.engine.Engine;
5049
import org.opensearch.index.mapper.MapperException;
5150
import org.opensearch.index.seqno.ReplicationTracker;
@@ -407,7 +406,7 @@ public void cleanFiles(
407406
// their own commit points and therefore do not modify the commit user data
408407
// in their store. In these cases, reuse the primary's translog UUID.
409408
final boolean reuseTranslogUUID = indexShard.indexSettings().isSegRepEnabled()
410-
|| IndexModule.Type.REMOTE_SNAPSHOT.match(indexShard.indexSettings());
409+
|| indexShard.indexSettings().isRemoteSnapshot();
411410
if (reuseTranslogUUID) {
412411
final String translogUUID = store.getMetadata().getCommitUserData().get(TRANSLOG_UUID_KEY);
413412
Translog.createEmptyTranslog(

‎server/src/main/java/org/opensearch/snapshots/RestoreService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ public ClusterState execute(ClusterState currentState) {
462462
request.ignoreIndexSettings()
463463
);
464464
final boolean isSearchableSnapshot = FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)
465-
&& IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(request.storageType().toString());
465+
&& IndexModule.Type.REMOTE_SNAPSHOT.match(request.storageType().toString());
466466
if (isSearchableSnapshot) {
467467
snapshotIndexMetadata = addSnapshotToIndexSettings(
468468
snapshotIndexMetadata,

‎server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public static void validateSnapshotsBackingAnyIndex(
161161
for (ObjectCursor<IndexMetadata> cursor : metadata.values()) {
162162
IndexMetadata indexMetadata = cursor.value;
163163
String storeType = indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey());
164-
if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(storeType)) {
164+
if (IndexModule.Type.REMOTE_SNAPSHOT.match(storeType)) {
165165
String snapshotId = indexMetadata.getSettings().get(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey());
166166
if (uuidToSnapshotId.get(snapshotId) != null) {
167167
snapshotsToBeNotDeleted.add(uuidToSnapshotId.get(snapshotId).getName());

0 commit comments

Comments
 (0)