Skip to content

Commit 6b53314

Browse files
ashking94ashking94
committed
Update Shallow Snapshot flows to support remote path type & hash algo
Signed-off-by: Ashish Singh <ssashish@amazon.com>
1 parent fb5d036 commit 6b53314

File tree

12 files changed

+225
-77
lines changed

12 files changed

+225
-77
lines changed

‎server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.opensearch.client.Requests;
2121
import org.opensearch.cluster.ClusterState;
2222
import org.opensearch.cluster.metadata.IndexMetadata;
23+
import org.opensearch.common.Nullable;
2324
import org.opensearch.common.io.PathUtils;
2425
import org.opensearch.common.settings.Settings;
2526
import org.opensearch.common.util.io.IOUtils;
@@ -47,6 +48,7 @@
4748
import java.util.Arrays;
4849
import java.util.List;
4950
import java.util.Map;
51+
import java.util.Objects;
5052
import java.util.Optional;
5153
import java.util.concurrent.ExecutionException;
5254
import java.util.stream.Collectors;
@@ -284,7 +286,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
284286

285287
indexDocuments(client, indexName1, randomIntBetween(5, 10));
286288
ensureGreen(indexName1);
287-
validatePathType(indexName1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
289+
validatePathType(indexName1, PathType.FIXED);
288290

289291
logger.info("--> snapshot");
290292
SnapshotInfo snapshotInfo = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1)));
@@ -301,7 +303,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
301303
.get();
302304
assertEquals(RestStatus.ACCEPTED, restoreSnapshotResponse.status());
303305
ensureGreen(restoredIndexName1version1);
304-
validatePathType(restoredIndexName1version1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
306+
validatePathType(restoredIndexName1version1, PathType.FIXED);
305307

306308
client(clusterManagerNode).admin()
307309
.cluster()
@@ -327,16 +329,22 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
327329
validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A);
328330

329331
// Validating that custom data has not changed for indexes which were created before the cluster setting got updated
330-
validatePathType(indexName1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
332+
validatePathType(indexName1, PathType.FIXED);
331333
}
332334

333-
private void validatePathType(String index, PathType pathType, PathHashAlgorithm pathHashAlgorithm) {
335+
private void validatePathType(String index, PathType pathType) {
336+
validatePathType(index, pathType, null);
337+
}
338+
339+
private void validatePathType(String index, PathType pathType, @Nullable PathHashAlgorithm pathHashAlgorithm) {
334340
ClusterState state = client().admin().cluster().prepareState().execute().actionGet().getState();
335341
// Validate that the remote_store custom data is present in index metadata for the created index.
336342
Map<String, String> remoteCustomData = state.metadata().index(index).getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
337343
assertNotNull(remoteCustomData);
338344
assertEquals(pathType.name(), remoteCustomData.get(PathType.NAME));
339-
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
345+
if (Objects.nonNull(pathHashAlgorithm)) {
346+
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
347+
}
340348
}
341349

342350
public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException {

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,9 @@ public MetadataCreateIndexService(
206206

207207
// Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction.
208208
createIndexTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_KEY, true);
209+
Supplier<Version> minNodeVersionSupplier = () -> clusterService.state().nodes().getMinNodeVersion();
209210
remoteStorePathStrategyResolver = isRemoteDataAttributePresent(settings)
210-
? new RemoteStorePathStrategyResolver(clusterService.getClusterSettings())
211+
? new RemoteStorePathStrategyResolver(clusterService.getClusterSettings(), minNodeVersionSupplier)
211212
: null;
212213
}
213214

@@ -575,22 +576,18 @@ public void addRemoteStorePathStrategyInCustomData(IndexMetadata.Builder tmpImdB
575576
if (remoteStorePathStrategyResolver != null) {
576577
// It is possible that remote custom data exists already. In such cases, we need to only update the path type
577578
// in the remote store custom data map.
578-
Map<String, String> existingRemoteCustomData = tmpImdBuilder.removeCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
579-
Map<String, String> remoteCustomData = existingRemoteCustomData == null
580-
? new HashMap<>()
581-
: new HashMap<>(existingRemoteCustomData);
579+
Map<String, String> existingCustomData = tmpImdBuilder.removeCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
580+
assert assertNullOldType == false || Objects.isNull(existingCustomData);
581+
582582
// Determine the path type for use using the remoteStorePathResolver.
583583
RemoteStorePathStrategy newPathStrategy = remoteStorePathStrategyResolver.get();
584-
String oldPathType = remoteCustomData.put(PathType.NAME, newPathStrategy.getType().name());
585-
String oldHashAlgorithm = remoteCustomData.put(PathHashAlgorithm.NAME, newPathStrategy.getHashAlgorithm().name());
586-
assert !assertNullOldType || (Objects.isNull(oldPathType) && Objects.isNull(oldHashAlgorithm));
584+
Map<String, String> remoteCustomData = new HashMap<>();
585+
remoteCustomData.put(PathType.NAME, newPathStrategy.getType().name());
586+
if (Objects.nonNull(newPathStrategy.getHashAlgorithm())) {
587+
remoteCustomData.put(PathHashAlgorithm.NAME, newPathStrategy.getHashAlgorithm().name());
588+
}
587589
logger.trace(
588-
() -> new ParameterizedMessage(
589-
"Added newPathStrategy={}, replaced oldPathType={} oldHashAlgorithm={}",
590-
newPathStrategy,
591-
oldPathType,
592-
oldHashAlgorithm
593-
)
590+
() -> new ParameterizedMessage("Added newStrategy={}, replaced oldStrategy={}", remoteCustomData, existingCustomData)
594591
);
595592
tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData);
596593
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import java.util.List;
6464
import java.util.Locale;
6565
import java.util.Map;
66+
import java.util.Objects;
6667
import java.util.Optional;
6768
import java.util.concurrent.TimeUnit;
6869
import java.util.function.Consumer;
@@ -1910,12 +1911,11 @@ public void setDocIdFuzzySetFalsePositiveProbability(double docIdFuzzySetFalsePo
19101911

19111912
public RemoteStorePathStrategy getRemoteStorePathStrategy() {
19121913
Map<String, String> remoteCustomData = indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
1913-
if (remoteCustomData != null
1914-
&& remoteCustomData.containsKey(PathType.NAME)
1915-
&& remoteCustomData.containsKey(PathHashAlgorithm.NAME)) {
1914+
if (remoteCustomData != null && remoteCustomData.containsKey(PathType.NAME)) {
19161915
PathType pathType = PathType.parseString(remoteCustomData.get(PathType.NAME));
1917-
PathHashAlgorithm pathHashAlgorithm = PathHashAlgorithm.parseString(remoteCustomData.get(PathHashAlgorithm.NAME));
1918-
return new RemoteStorePathStrategy(pathType, pathHashAlgorithm);
1916+
String hashAlgoStr = remoteCustomData.get(PathHashAlgorithm.NAME);
1917+
PathHashAlgorithm hashAlgorithm = Objects.nonNull(hashAlgoStr) ? PathHashAlgorithm.parseString(hashAlgoStr) : null;
1918+
return new RemoteStorePathStrategy(pathType, hashAlgorithm);
19191919
}
19201920
return new RemoteStorePathStrategy(PathType.FIXED);
19211921
}

‎server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@
1313
import org.opensearch.common.hash.FNV1a;
1414
import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput;
1515

16+
import java.util.HashMap;
1617
import java.util.Locale;
18+
import java.util.Map;
19+
import java.util.Objects;
1720
import java.util.Set;
1821

22+
import static java.util.Collections.unmodifiableMap;
1923
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
2024
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA;
2125

@@ -78,9 +82,10 @@ public String getName() {
7882
*/
7983
@PublicApi(since = "2.14.0")
8084
public enum PathType {
81-
FIXED {
85+
FIXED(0) {
8286
@Override
8387
public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
88+
assert Objects.isNull(hashAlgorithm) : "hashAlgorithm is expected to be null with fixed remote store path type";
8489
// Hash algorithm is not used in FIXED path type
8590
return pathInput.basePath()
8691
.add(pathInput.indexUUID())
@@ -94,7 +99,7 @@ boolean requiresHashAlgorithm() {
9499
return false;
95100
}
96101
},
97-
HASHED_PREFIX {
102+
HASHED_PREFIX(1) {
98103
@Override
99104
public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
100105
// TODO - We need to implement this, keeping the same path as Fixed for sake of multiple tests that can fail otherwise.
@@ -112,6 +117,33 @@ boolean requiresHashAlgorithm() {
112117
}
113118
};
114119

120+
private final int code;
121+
122+
PathType(int code) {
123+
this.code = code;
124+
}
125+
126+
public int getCode() {
127+
return code;
128+
}
129+
130+
private static final Map<Integer, PathType> CODE_TO_ENUM;
131+
static {
132+
PathType[] values = values();
133+
Map<Integer, PathType> codeToStatus = new HashMap<>(values.length);
134+
for (PathType value : values) {
135+
codeToStatus.put(value.code, value);
136+
}
137+
CODE_TO_ENUM = unmodifiableMap(codeToStatus);
138+
}
139+
140+
/**
141+
* Turn a status code into a {@link PathType}.
142+
*/
143+
public static PathType fromCode(int code) {
144+
return CODE_TO_ENUM.get(code);
145+
}
146+
115147
/**
116148
* This method generates the path for the given path input which constitutes multiple fields and characteristics
117149
* of the data.
@@ -131,7 +163,7 @@ public BlobPath path(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
131163
return generatePath(pathInput, hashAlgorithm);
132164
}
133165

134-
abstract BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm);
166+
protected abstract BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm);
135167

136168
abstract boolean requiresHashAlgorithm();
137169

@@ -158,7 +190,7 @@ public static PathType parseString(String pathType) {
158190
@PublicApi(since = "2.14.0")
159191
public enum PathHashAlgorithm {
160192

161-
FNV_1A {
193+
FNV_1A(0) {
162194
@Override
163195
long hash(PathInput pathInput) {
164196
String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType()
@@ -167,6 +199,33 @@ long hash(PathInput pathInput) {
167199
}
168200
};
169201

202+
private final int code;
203+
204+
PathHashAlgorithm(int code) {
205+
this.code = code;
206+
}
207+
208+
public int getCode() {
209+
return code;
210+
}
211+
212+
private static final Map<Integer, PathHashAlgorithm> CODE_TO_ENUM;
213+
static {
214+
PathHashAlgorithm[] values = values();
215+
Map<Integer, PathHashAlgorithm> codeToStatus = new HashMap<>(values.length);
216+
for (PathHashAlgorithm value : values) {
217+
codeToStatus.put(value.code, value);
218+
}
219+
CODE_TO_ENUM = unmodifiableMap(codeToStatus);
220+
}
221+
222+
/**
223+
* Turn a status code into a {@link PathHashAlgorithm}.
224+
*/
225+
public static PathHashAlgorithm fromCode(int code) {
226+
return CODE_TO_ENUM.get(code);
227+
}
228+
170229
abstract long hash(PathInput pathInput);
171230

172231
public static PathHashAlgorithm parseString(String pathHashAlgorithm) {

‎server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public RemoteStorePathStrategy(PathType type) {
3636
}
3737

3838
public RemoteStorePathStrategy(PathType type, PathHashAlgorithm hashAlgorithm) {
39-
assert type.requiresHashAlgorithm() == false || Objects.nonNull(hashAlgorithm);
39+
assert (type.requiresHashAlgorithm() == false && Objects.isNull(hashAlgorithm)) || Objects.nonNull(hashAlgorithm);
4040
this.type = Objects.requireNonNull(type);
4141
this.hashAlgorithm = hashAlgorithm;
4242
}
@@ -55,7 +55,7 @@ public String toString() {
5555
}
5656

5757
public BlobPath generatePath(PathInput pathInput) {
58-
return type.generatePath(pathInput, hashAlgorithm);
58+
return type.path(pathInput, hashAlgorithm);
5959
}
6060

6161
/**

‎server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@
88

99
package org.opensearch.index.remote;
1010

11+
import org.opensearch.Version;
1112
import org.opensearch.common.settings.ClusterSettings;
1213
import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm;
1314
import org.opensearch.index.remote.RemoteStoreEnums.PathType;
1415
import org.opensearch.indices.IndicesService;
1516

17+
import java.util.function.Supplier;
18+
1619
/**
1720
* Determines the {@link RemoteStorePathStrategy} at the time of index metadata creation.
1821
*
@@ -22,13 +25,22 @@ public class RemoteStorePathStrategyResolver {
2225

2326
private volatile PathType type;
2427

25-
public RemoteStorePathStrategyResolver(ClusterSettings clusterSettings) {
28+
private final Supplier<Version> minNodeVersionSupplier;
29+
30+
public RemoteStorePathStrategyResolver(ClusterSettings clusterSettings, Supplier<Version> minNodeVersionSupplier) {
31+
this.minNodeVersionSupplier = minNodeVersionSupplier;
2632
type = clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING);
2733
clusterSettings.addSettingsUpdateConsumer(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING, this::setType);
2834
}
2935

3036
public RemoteStorePathStrategy get() {
31-
return new RemoteStorePathStrategy(type, PathHashAlgorithm.FNV_1A);
37+
PathType pathType;
38+
PathHashAlgorithm pathHashAlgorithm;
39+
// Min node version check ensures that we are enabling the new prefix type only when all the nodes understand it.
40+
pathType = Version.CURRENT.compareTo(minNodeVersionSupplier.get()) <= 0 ? type : PathType.FIXED;
41+
// If the path type is fixed, hash algorithm is not applicable.
42+
pathHashAlgorithm = pathType == PathType.FIXED ? null : PathHashAlgorithm.FNV_1A;
43+
return new RemoteStorePathStrategy(pathType, pathHashAlgorithm);
3244
}
3345

3446
private void setType(PathType type) {

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@
5858
import org.opensearch.index.engine.Engine;
5959
import org.opensearch.index.engine.EngineException;
6060
import org.opensearch.index.mapper.MapperService;
61-
import org.opensearch.index.remote.RemoteStoreEnums.PathType;
62-
import org.opensearch.index.remote.RemoteStorePathStrategy;
6361
import org.opensearch.index.seqno.SequenceNumbers;
6462
import org.opensearch.index.snapshots.IndexShardRestoreFailedException;
6563
import org.opensearch.index.snapshots.blobstore.RemoteStoreShardShallowCopySnapshot;
@@ -412,8 +410,7 @@ void recoverFromSnapshotAndRemoteStore(
412410
remoteStoreRepository,
413411
indexUUID,
414412
shardId,
415-
new RemoteStorePathStrategy(PathType.FIXED)
416-
// TODO - The path type needs to be obtained from RemoteStoreShardShallowCopySnapshot
413+
shallowCopyShardMetadata.getRemoteStorePathStrategy()
417414
);
418415
sourceRemoteDirectory.initializeToSpecificCommit(
419416
primaryTerm,

0 commit comments

Comments
 (0)