Skip to content

Commit c588580

Browse files
github-actions[bot]github-actions[bot]
committed
Update Shallow Snapshot flows to support remote path type & hash algo (#12988)
Signed-off-by: Ashish Singh <ssashish@amazon.com> (cherry picked from commit e713175) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 4bea236 commit c588580

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‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.opensearch.client.Requests;
1919
import org.opensearch.cluster.ClusterState;
2020
import org.opensearch.cluster.metadata.IndexMetadata;
21+
import org.opensearch.common.Nullable;
2122
import org.opensearch.common.io.PathUtils;
2223
import org.opensearch.common.settings.Settings;
2324
import org.opensearch.common.util.io.IOUtils;
@@ -46,6 +47,7 @@
4647
import java.util.Arrays;
4748
import java.util.List;
4849
import java.util.Map;
50+
import java.util.Objects;
4951
import java.util.Optional;
5052
import java.util.concurrent.ExecutionException;
5153
import java.util.stream.Collectors;
@@ -226,7 +228,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
226228

227229
indexDocuments(client, indexName1, randomIntBetween(5, 10));
228230
ensureGreen(indexName1);
229-
validatePathType(indexName1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
231+
validatePathType(indexName1, PathType.FIXED);
230232

231233
logger.info("--> snapshot");
232234
SnapshotInfo snapshotInfo = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1)));
@@ -243,7 +245,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
243245
.get();
244246
assertEquals(RestStatus.ACCEPTED, restoreSnapshotResponse.status());
245247
ensureGreen(restoredIndexName1version1);
246-
validatePathType(restoredIndexName1version1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
248+
validatePathType(restoredIndexName1version1, PathType.FIXED);
247249

248250
client(clusterManagerNode).admin()
249251
.cluster()
@@ -269,16 +271,50 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
269271
validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A);
270272

271273
// Validating that custom data has not changed for indexes which were created before the cluster setting got updated
272-
validatePathType(indexName1, PathType.FIXED, PathHashAlgorithm.FNV_1A);
274+
validatePathType(indexName1, PathType.FIXED);
275+
276+
// Create Snapshot of index 2
277+
String snapshotName2 = "test-restore-snapshot2";
278+
snapshotInfo = createSnapshot(snapshotRepoName, snapshotName2, new ArrayList<>(List.of(indexName2)));
279+
assertEquals(SnapshotState.SUCCESS, snapshotInfo.state());
280+
assertTrue(snapshotInfo.successfulShards() > 0);
281+
assertEquals(snapshotInfo.totalShards(), snapshotInfo.successfulShards());
282+
283+
// Update cluster settings to FIXED
284+
client(clusterManagerNode).admin()
285+
.cluster()
286+
.prepareUpdateSettings()
287+
.setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), PathType.FIXED))
288+
.get();
289+
290+
// Close index 2
291+
assertAcked(client().admin().indices().prepareClose(indexName2));
292+
restoreSnapshotResponse = client.admin()
293+
.cluster()
294+
.prepareRestoreSnapshot(snapshotRepoName, snapshotName2)
295+
.setWaitForCompletion(false)
296+
.setIndices(indexName2)
297+
.get();
298+
assertEquals(RestStatus.ACCEPTED, restoreSnapshotResponse.status());
299+
ensureGreen(indexName2);
300+
301+
// Validating that custom data has not changed for testindex2 which was created before the cluster setting got updated
302+
validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A);
273303
}
274304

275-
private void validatePathType(String index, PathType pathType, PathHashAlgorithm pathHashAlgorithm) {
305+
private void validatePathType(String index, PathType pathType) {
306+
validatePathType(index, pathType, null);
307+
}
308+
309+
private void validatePathType(String index, PathType pathType, @Nullable PathHashAlgorithm pathHashAlgorithm) {
276310
ClusterState state = client().admin().cluster().prepareState().execute().actionGet().getState();
277311
// Validate that the remote_store custom data is present in index metadata for the created index.
278312
Map<String, String> remoteCustomData = state.metadata().index(index).getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
279313
assertNotNull(remoteCustomData);
280314
assertEquals(pathType.name(), remoteCustomData.get(PathType.NAME));
281-
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
315+
if (Objects.nonNull(pathHashAlgorithm)) {
316+
assertEquals(pathHashAlgorithm.name(), remoteCustomData.get(PathHashAlgorithm.NAME));
317+
}
282318
}
283319

284320
public void testRestoreInSameRemoteStoreEnabledIndex() throws IOException {

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

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,9 @@ public MetadataCreateIndexService(
210210

211211
// Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction.
212212
createIndexTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_KEY, true);
213+
Supplier<Version> minNodeVersionSupplier = () -> clusterService.state().nodes().getMinNodeVersion();
213214
remoteStorePathStrategyResolver = isRemoteDataAttributePresent(settings)
214-
? new RemoteStorePathStrategyResolver(clusterService.getClusterSettings())
215+
? new RemoteStorePathStrategyResolver(clusterService.getClusterSettings(), minNodeVersionSupplier)
215216
: null;
216217
}
217218

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

603599
private ClusterState applyCreateIndexRequestWithV1Templates(

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import java.util.List;
6565
import java.util.Locale;
6666
import java.util.Map;
67+
import java.util.Objects;
6768
import java.util.Optional;
6869
import java.util.concurrent.TimeUnit;
6970
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.
@@ -987,6 +989,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
987989
*/
988990
widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0);
989991
assignedOnRemoteNode = RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
992+
remoteStorePathStrategy = determineRemoteStorePathStrategy();
990993

991994
setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
992995
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‎

Lines changed: 77 additions & 4 deletions
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‎

Lines changed: 14 additions & 3 deletions
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)