Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit b0c39bc

Browse files
ashking94shiv0408
ashking94
authored and
shiv0408
committedApr 25, 2024
Update Shallow Snapshot flows to support remote path type & hash algo (opensearch-project#12988)
Signed-off-by: Ashish Singh <ssashish@amazon.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
1 parent d33320d commit b0c39bc

File tree

12 files changed

+508
-131
lines changed

12 files changed

+508
-131
lines changed
 

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

+41-5
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,50 @@ 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);
333+
334+
// Create Snapshot of index 2
335+
String snapshotName2 = "test-restore-snapshot2";
336+
snapshotInfo = createSnapshot(snapshotRepoName, snapshotName2, new ArrayList<>(List.of(indexName2)));
337+
assertEquals(SnapshotState.SUCCESS, snapshotInfo.state());
338+
assertTrue(snapshotInfo.successfulShards() > 0);
339+
assertEquals(snapshotInfo.totalShards(), snapshotInfo.successfulShards());
340+
341+
// Update cluster settings to FIXED
342+
client(clusterManagerNode).admin()
343+
.cluster()
344+
.prepareUpdateSettings()
345+
.setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), PathType.FIXED))
346+
.get();
347+
348+
// Close index 2
349+
assertAcked(client().admin().indices().prepareClose(indexName2));
350+
restoreSnapshotResponse = client.admin()
351+
.cluster()
352+
.prepareRestoreSnapshot(snapshotRepoName, snapshotName2)
353+
.setWaitForCompletion(false)
354+
.setIndices(indexName2)
355+
.get();
356+
assertEquals(RestStatus.ACCEPTED, restoreSnapshotResponse.status());
357+
ensureGreen(indexName2);
358+
359+
// Validating that custom data has not changed for testindex2 which was created before the cluster setting got updated
360+
validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A);
331361
}
332362

333-
private void validatePathType(String index, PathType pathType, PathHashAlgorithm pathHashAlgorithm) {
363+
private void validatePathType(String index, PathType pathType) {
364+
validatePathType(index, pathType, null);
365+
}
366+
367+
private void validatePathType(String index, PathType pathType, @Nullable PathHashAlgorithm pathHashAlgorithm) {
334368
ClusterState state = client().admin().cluster().prepareState().execute().actionGet().getState();
335369
// Validate that the remote_store custom data is present in index metadata for the created index.
336370
Map<String, String> remoteCustomData = state.metadata().index(index).getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
337371
assertNotNull(remoteCustomData);
338372
assertEquals(pathType.name(), remoteCustomData.get(PathType.NAME));
339-
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
373+
if (Objects.nonNull(pathHashAlgorithm)) {
374+
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
375+
}
340376
}
341377

342378
public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException {

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

+18-22
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

@@ -572,28 +573,23 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
572573
* @param assertNullOldType flag to verify that the old remote store path type is null
573574
*/
574575
public void addRemoteStorePathStrategyInCustomData(IndexMetadata.Builder tmpImdBuilder, boolean assertNullOldType) {
575-
if (remoteStorePathStrategyResolver != null) {
576-
// It is possible that remote custom data exists already. In such cases, we need to only update the path type
577-
// 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);
582-
// Determine the path type for use using the remoteStorePathResolver.
583-
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));
587-
logger.trace(
588-
() -> new ParameterizedMessage(
589-
"Added newPathStrategy={}, replaced oldPathType={} oldHashAlgorithm={}",
590-
newPathStrategy,
591-
oldPathType,
592-
oldHashAlgorithm
593-
)
594-
);
595-
tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData);
576+
if (remoteStorePathStrategyResolver == null) {
577+
return;
578+
}
579+
// It is possible that remote custom data exists already. In such cases, we need to only update the path type
580+
// in the remote store custom data map.
581+
Map<String, String> existingCustomData = tmpImdBuilder.removeCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
582+
assert assertNullOldType == false || Objects.isNull(existingCustomData);
583+
584+
// Determine the path type for use using the remoteStorePathResolver.
585+
RemoteStorePathStrategy newPathStrategy = remoteStorePathStrategyResolver.get();
586+
Map<String, String> remoteCustomData = new HashMap<>();
587+
remoteCustomData.put(PathType.NAME, newPathStrategy.getType().name());
588+
if (Objects.nonNull(newPathStrategy.getHashAlgorithm())) {
589+
remoteCustomData.put(PathHashAlgorithm.NAME, newPathStrategy.getHashAlgorithm().name());
596590
}
591+
logger.trace(() -> new ParameterizedMessage("Added newStrategy={}, replaced oldStrategy={}", remoteCustomData, existingCustomData));
592+
tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData);
597593
}
598594

599595
private ClusterState applyCreateIndexRequestWithV1Templates(

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

+13-6
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;
@@ -764,6 +765,7 @@ public static IndexMergePolicy fromString(String text) {
764765
private volatile String defaultSearchPipeline;
765766
private final boolean widenIndexSortType;
766767
private final boolean assignedOnRemoteNode;
768+
private final RemoteStorePathStrategy remoteStorePathStrategy;
767769

768770
/**
769771
* The maximum age of a retention lease before it is considered expired.
@@ -988,6 +990,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
988990
*/
989991
widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0);
990992
assignedOnRemoteNode = RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
993+
remoteStorePathStrategy = determineRemoteStorePathStrategy();
991994

992995
setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
993996
setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING));
@@ -1908,15 +1911,19 @@ public void setDocIdFuzzySetFalsePositiveProbability(double docIdFuzzySetFalsePo
19081911
this.docIdFuzzySetFalsePositiveProbability = docIdFuzzySetFalsePositiveProbability;
19091912
}
19101913

1911-
public RemoteStorePathStrategy getRemoteStorePathStrategy() {
1914+
private RemoteStorePathStrategy determineRemoteStorePathStrategy() {
19121915
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)) {
1916+
assert remoteCustomData == null || remoteCustomData.containsKey(PathType.NAME);
1917+
if (remoteCustomData != null && remoteCustomData.containsKey(PathType.NAME)) {
19161918
PathType pathType = PathType.parseString(remoteCustomData.get(PathType.NAME));
1917-
PathHashAlgorithm pathHashAlgorithm = PathHashAlgorithm.parseString(remoteCustomData.get(PathHashAlgorithm.NAME));
1918-
return new RemoteStorePathStrategy(pathType, pathHashAlgorithm);
1919+
String hashAlgoStr = remoteCustomData.get(PathHashAlgorithm.NAME);
1920+
PathHashAlgorithm hashAlgorithm = Objects.nonNull(hashAlgoStr) ? PathHashAlgorithm.parseString(hashAlgoStr) : null;
1921+
return new RemoteStorePathStrategy(pathType, hashAlgorithm);
19191922
}
19201923
return new RemoteStorePathStrategy(PathType.FIXED);
19211924
}
1925+
1926+
public RemoteStorePathStrategy getRemoteStorePathStrategy() {
1927+
return remoteStorePathStrategy;
1928+
}
19221929
}

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

+77-4
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,19 @@
88

99
package org.opensearch.index.remote;
1010

11+
import org.apache.logging.log4j.message.ParameterizedMessage;
1112
import org.opensearch.common.annotation.PublicApi;
1213
import org.opensearch.common.blobstore.BlobPath;
1314
import org.opensearch.common.hash.FNV1a;
1415
import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput;
1516

17+
import java.util.HashMap;
1618
import java.util.Locale;
19+
import java.util.Map;
20+
import java.util.Objects;
1721
import java.util.Set;
1822

23+
import static java.util.Collections.unmodifiableMap;
1924
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
2025
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA;
2126

@@ -78,9 +83,10 @@ public String getName() {
7883
*/
7984
@PublicApi(since = "2.14.0")
8085
public enum PathType {
81-
FIXED {
86+
FIXED(0) {
8287
@Override
8388
public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
89+
assert Objects.isNull(hashAlgorithm) : "hashAlgorithm is expected to be null with fixed remote store path type";
8490
// Hash algorithm is not used in FIXED path type
8591
return pathInput.basePath()
8692
.add(pathInput.indexUUID())
@@ -94,7 +100,7 @@ boolean requiresHashAlgorithm() {
94100
return false;
95101
}
96102
},
97-
HASHED_PREFIX {
103+
HASHED_PREFIX(1) {
98104
@Override
99105
public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
100106
// TODO - We need to implement this, keeping the same path as Fixed for sake of multiple tests that can fail otherwise.
@@ -112,6 +118,40 @@ boolean requiresHashAlgorithm() {
112118
}
113119
};
114120

121+
private final int code;
122+
123+
PathType(int code) {
124+
this.code = code;
125+
}
126+
127+
public int getCode() {
128+
return code;
129+
}
130+
131+
private static final Map<Integer, PathType> CODE_TO_ENUM;
132+
133+
static {
134+
PathType[] values = values();
135+
Map<Integer, PathType> codeToStatus = new HashMap<>(values.length);
136+
for (PathType value : values) {
137+
int code = value.code;
138+
if (codeToStatus.containsKey(code)) {
139+
throw new IllegalStateException(
140+
new ParameterizedMessage("{} has same code as {}", codeToStatus.get(code), value).getFormattedMessage()
141+
);
142+
}
143+
codeToStatus.put(code, value);
144+
}
145+
CODE_TO_ENUM = unmodifiableMap(codeToStatus);
146+
}
147+
148+
/**
149+
* Turn a status code into a {@link PathType}.
150+
*/
151+
public static PathType fromCode(int code) {
152+
return CODE_TO_ENUM.get(code);
153+
}
154+
115155
/**
116156
* This method generates the path for the given path input which constitutes multiple fields and characteristics
117157
* of the data.
@@ -131,7 +171,7 @@ public BlobPath path(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
131171
return generatePath(pathInput, hashAlgorithm);
132172
}
133173

134-
abstract BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm);
174+
protected abstract BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm);
135175

136176
abstract boolean requiresHashAlgorithm();
137177

@@ -158,7 +198,7 @@ public static PathType parseString(String pathType) {
158198
@PublicApi(since = "2.14.0")
159199
public enum PathHashAlgorithm {
160200

161-
FNV_1A {
201+
FNV_1A(0) {
162202
@Override
163203
long hash(PathInput pathInput) {
164204
String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType()
@@ -167,6 +207,39 @@ long hash(PathInput pathInput) {
167207
}
168208
};
169209

210+
private final int code;
211+
212+
PathHashAlgorithm(int code) {
213+
this.code = code;
214+
}
215+
216+
public int getCode() {
217+
return code;
218+
}
219+
220+
private static final Map<Integer, PathHashAlgorithm> CODE_TO_ENUM;
221+
static {
222+
PathHashAlgorithm[] values = values();
223+
Map<Integer, PathHashAlgorithm> codeToStatus = new HashMap<>(values.length);
224+
for (PathHashAlgorithm value : values) {
225+
int code = value.code;
226+
if (codeToStatus.containsKey(code)) {
227+
throw new IllegalStateException(
228+
new ParameterizedMessage("{} has same code as {}", codeToStatus.get(code), value).getFormattedMessage()
229+
);
230+
}
231+
codeToStatus.put(code, value);
232+
}
233+
CODE_TO_ENUM = unmodifiableMap(codeToStatus);
234+
}
235+
236+
/**
237+
* Turn a status code into a {@link PathHashAlgorithm}.
238+
*/
239+
public static PathHashAlgorithm fromCode(int code) {
240+
return CODE_TO_ENUM.get(code);
241+
}
242+
170243
abstract long hash(PathInput pathInput);
171244

172245
public static PathHashAlgorithm parseString(String pathHashAlgorithm) {

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

+14-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.opensearch.index.remote;
1010

11+
import org.apache.logging.log4j.message.ParameterizedMessage;
1112
import org.opensearch.common.Nullable;
1213
import org.opensearch.common.annotation.PublicApi;
1314
import org.opensearch.common.blobstore.BlobPath;
@@ -36,11 +37,21 @@ public RemoteStorePathStrategy(PathType type) {
3637
}
3738

3839
public RemoteStorePathStrategy(PathType type, PathHashAlgorithm hashAlgorithm) {
39-
assert type.requiresHashAlgorithm() == false || Objects.nonNull(hashAlgorithm);
40-
this.type = Objects.requireNonNull(type);
40+
Objects.requireNonNull(type, "pathType can not be null");
41+
if (isCompatible(type, hashAlgorithm) == false) {
42+
throw new IllegalArgumentException(
43+
new ParameterizedMessage("pathType={} pathHashAlgorithm={} are incompatible", type, hashAlgorithm).getFormattedMessage()
44+
);
45+
}
46+
this.type = type;
4147
this.hashAlgorithm = hashAlgorithm;
4248
}
4349

50+
public static boolean isCompatible(PathType type, PathHashAlgorithm hashAlgorithm) {
51+
return (type.requiresHashAlgorithm() == false && Objects.isNull(hashAlgorithm))
52+
|| (type.requiresHashAlgorithm() && Objects.nonNull(hashAlgorithm));
53+
}
54+
4455
public PathType getType() {
4556
return type;
4657
}
@@ -55,7 +66,7 @@ public String toString() {
5566
}
5667

5768
public BlobPath generatePath(PathInput pathInput) {
58-
return type.generatePath(pathInput, hashAlgorithm);
69+
return type.path(pathInput, hashAlgorithm);
5970
}
6071

6172
/**

0 commit comments

Comments
 (0)
Please sign in to comment.