hemantk-12 commented on code in PR #7200:
URL: https://github.com/apache/ozone/pull/7200#discussion_r1762429962
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ tableIterator.seek(startKey);
+ }
+ int currentCount = 0;
+ while (tableIterator.hasNext() && currentCount < count) {
+ Table.KeyValue<String, V> kv = tableIterator.next();
+ if (kv != null) {
+ entries.add(Table.newKeyValue(kv.getKey(),
valueFunction.apply(kv.getValue())));
+ currentCount++;
+ }
+ }
+ return entries;
+ }
+
+
Review Comment:
nit: remove extra line.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ tableIterator.seek(startKey);
+ }
+ int currentCount = 0;
+ while (tableIterator.hasNext() && currentCount < count) {
+ Table.KeyValue<String, V> kv = tableIterator.next();
+ if (kv != null) {
+ entries.add(Table.newKeyValue(kv.getKey(),
valueFunction.apply(kv.getValue())));
+ currentCount++;
+ }
+ }
+ return entries;
+ }
+
+
+ @Override
+ public List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
+ .map(vol -> metadataManager.getBucketKeyPrefix(vol, bucket));
+ try (TableIterator<String, ? extends Table.KeyValue<String, String>>
+ renamedKeyIter =
metadataManager.getSnapshotRenamedTable().iterator(bucketPrefix.orElse(""))) {
+ return getTableEntries(startKey, renamedKeyIter, Function.identity(),
count);
+ }
+ }
+
+ @Override
+ public List<Table.KeyValue<String, List<OmKeyInfo>>> getDeletedKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
+ .map(vol -> metadataManager.getBucketKeyPrefix(vol, bucket));
+ List<Table.KeyValue<String, List<OmKeyInfo>>> deletedKeyEntries = new
ArrayList<>(count);
Review Comment:
This is not used anywhere.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ tableIterator.seek(startKey);
+ }
+ int currentCount = 0;
+ while (tableIterator.hasNext() && currentCount < count) {
+ Table.KeyValue<String, V> kv = tableIterator.next();
+ if (kv != null) {
+ entries.add(Table.newKeyValue(kv.getKey(),
valueFunction.apply(kv.getValue())));
+ currentCount++;
+ }
+ }
+ return entries;
+ }
+
+
+ @Override
+ public List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
Review Comment:
nit: can you please change `count` to something better like `size` or
`batch` which says that it is a set of the next N keys from StartKey?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
Review Comment:
```suggestion
// Seek the start key if it is not null. The next key would be ensured
to start with the bucket prefix.
// Details: {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(prefix)}.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ tableIterator.seek(startKey);
+ }
+ int currentCount = 0;
+ while (tableIterator.hasNext() && currentCount < count) {
+ Table.KeyValue<String, V> kv = tableIterator.next();
+ if (kv != null) {
Review Comment:
Do we need this null check? `tableIterator.hasNext()` should do that check?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveTableKeysRequest.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om.request.snapshot;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.TransactionInfo;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import
org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveTableKeysResponse;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.upgrade.DisallowedUntilLayoutVersion;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveTableKeysRequest;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf;
+import static
org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.FILESYSTEM_SNAPSHOT;
+
+/**
+ * Handles OMSnapshotMoveTableKeysRequest Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
+ */
+public class OMSnapshotMoveTableKeysRequest extends OMClientRequest {
Review Comment:
Note: As a separate task, we should restrict direct access to the snapshot's
internal APIs.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java:
##########
@@ -138,37 +150,25 @@ public static void checkSnapshotActive(SnapshotInfo
snapInfo,
}
}
+
/**
- * Get the next non deleted snapshot in the snapshot chain.
+ * Get the next in the snapshot chain.
Review Comment:
```suggestion
* Get the next snapshot in the snapshot chain.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -119,6 +122,29 @@ ListKeysResult listKeys(String volumeName, String
bucketName, String startKey,
*/
PendingKeysDeletion getPendingDeletionKeys(int count) throws IOException;
+ /**
+ * Returns a list rename entries from the snapshotRenamedTable.
+ *
+ * @param count max number of keys to return.
+ * @return a Pair of list of {@link
org.apache.hadoop.hdds.utils.db.Table.KeyValue} representing the keys in the
+ * underlying metadataManager.
+ * @throws IOException
+ */
+ List<Table.KeyValue<String, String>> getRenamesKeyEntries(
Review Comment:
nit: Is there a reason to return the List of Pairs and not the Map? If it is
just to maintain the order, LinkedHashMap could be used. I feel that the map
improves the readability.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java:
##########
@@ -138,37 +150,25 @@ public static void checkSnapshotActive(SnapshotInfo
snapInfo,
}
}
+
/**
- * Get the next non deleted snapshot in the snapshot chain.
+ * Get the next in the snapshot chain.
*/
- public static SnapshotInfo getNextActiveSnapshot(SnapshotInfo snapInfo,
- SnapshotChainManager chainManager, OzoneManager ozoneManager)
+ public static SnapshotInfo getNextSnapshot(OzoneManager ozoneManager,
+ SnapshotChainManager chainManager,
+ SnapshotInfo snapInfo)
throws IOException {
-
// If the snapshot is deleted in the previous run, then the in-memory
// SnapshotChainManager might throw NoSuchElementException as the snapshot
// is removed in-memory but OMDoubleBuffer has not flushed yet.
Review Comment:
I don't understand this comment. Is it talking about the null check or
`chain.hasNextPathSnapshot()`?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +663,61 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ @Override
+ public List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
Review Comment:
IMO, code is hard to read this way. I prefer to change it later than having
these checks now.
If you want to keep it, maybe add a helper function that returns a prefix or
empty string in case of null values. Because this one-liner is doing too much
mapping which makes it harder to read.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1976,6 +2032,27 @@ public Table.KeyValue<String, OmKeyInfo>
getPendingDeletionDir()
return null;
}
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs()
+ throws IOException {
+ return this.getPendingDeletionDirs(null, null);
+ }
+
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs(String volume,
+
String bucket)
+ throws IOException {
+
+ // Either both volume & bucket should be null or none of them should be
null.
+ if (!StringUtils.isBlank(volume) && StringUtils.isBlank(bucket) ||
+ StringUtils.isBlank(volume) && !StringUtils.isBlank(bucket)) {
+ throw new IOException("One of volume : " + volume + ", bucket: " +
bucket + " is empty. Either both should be " +
+ "empty or none of the arguments should be empty");
+ }
+ return StringUtils.isBlank(volume) ?
metadataManager.getDeletedDirTable().iterator() :
Review Comment:
I'm OK keeping it this way as long as iterator respects the prefix and
startKey.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveTableKeysRequest.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om.request.snapshot;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.TransactionInfo;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import
org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveTableKeysResponse;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.upgrade.DisallowedUntilLayoutVersion;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveTableKeysRequest;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf;
+import static
org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.FILESYSTEM_SNAPSHOT;
+
+/**
+ * Handles OMSnapshotMoveTableKeysRequest Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
+ */
+public class OMSnapshotMoveTableKeysRequest extends OMClientRequest {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(OMSnapshotMoveTableKeysRequest.class);
+
+ public OMSnapshotMoveTableKeysRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @DisallowedUntilLayoutVersion(FILESYSTEM_SNAPSHOT)
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
TermIndex termIndex) {
+ OmMetadataManagerImpl omMetadataManager = (OmMetadataManagerImpl)
ozoneManager.getMetadataManager();
+ SnapshotChainManager snapshotChainManager =
omMetadataManager.getSnapshotChainManager();
+
+ SnapshotMoveTableKeysRequest moveTableKeysRequest =
getOmRequest().getSnapshotMoveTableKeysRequest();
+
+ OMClientResponse omClientResponse;
+ OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
OmResponseUtil.getOMResponseBuilder(getOmRequest());
+ try {
+ SnapshotInfo fromSnapshot = SnapshotUtils.getSnapshotInfo(ozoneManager,
+ snapshotChainManager,
fromProtobuf(moveTableKeysRequest.getFromSnapshotID()));
+ // If there is no Non-Deleted Snapshot move the
+ // keys to Active Object Store.
+ SnapshotInfo nextSnapshot = SnapshotUtils.getNextSnapshot(ozoneManager,
snapshotChainManager, fromSnapshot);
+
+ // If next snapshot is not active then ignore move. Since this could be
a redundant operations.
+ if (nextSnapshot != null && nextSnapshot.getSnapshotStatus() !=
SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE) {
+ throw new OMException("Next snapshot : " + nextSnapshot + " in chain
is not active.",
+ OMException.ResultCodes.INVALID_SNAPSHOT_ERROR);
+ }
+
+ String expectedBucketKeyPrefix =
omMetadataManager.getBucketKeyPrefix(fromSnapshot.getVolumeName(),
Review Comment:
nit: why `expectedBucketKeyPrefix` and not just `keyPrefix` or
`bucketKeyPrefix`?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +664,53 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ private <V, R> List<Table.KeyValue<String, R>> getTableEntries(String
startKey,
+ TableIterator<String, ? extends Table.KeyValue<String, V>>
tableIterator,
+ Function<V, R> valueFunction, int count) throws IOException {
+ List<Table.KeyValue<String, R>> entries = new ArrayList<>();
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
Review Comment:
Don't we need to `seekToFirst` when `startKey` is null?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveTableKeysRequest.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om.request.snapshot;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.TransactionInfo;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import
org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveTableKeysResponse;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.upgrade.DisallowedUntilLayoutVersion;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveTableKeysRequest;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf;
+import static
org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.FILESYSTEM_SNAPSHOT;
+
+/**
+ * Handles OMSnapshotMoveTableKeysRequest Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
+ */
+public class OMSnapshotMoveTableKeysRequest extends OMClientRequest {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(OMSnapshotMoveTableKeysRequest.class);
+
+ public OMSnapshotMoveTableKeysRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @DisallowedUntilLayoutVersion(FILESYSTEM_SNAPSHOT)
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
TermIndex termIndex) {
+ OmMetadataManagerImpl omMetadataManager = (OmMetadataManagerImpl)
ozoneManager.getMetadataManager();
+ SnapshotChainManager snapshotChainManager =
omMetadataManager.getSnapshotChainManager();
+
+ SnapshotMoveTableKeysRequest moveTableKeysRequest =
getOmRequest().getSnapshotMoveTableKeysRequest();
+
+ OMClientResponse omClientResponse;
+ OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
OmResponseUtil.getOMResponseBuilder(getOmRequest());
+ try {
+ SnapshotInfo fromSnapshot = SnapshotUtils.getSnapshotInfo(ozoneManager,
+ snapshotChainManager,
fromProtobuf(moveTableKeysRequest.getFromSnapshotID()));
+ // If there is no Non-Deleted Snapshot move the
+ // keys to Active Object Store.
+ SnapshotInfo nextSnapshot = SnapshotUtils.getNextSnapshot(ozoneManager,
snapshotChainManager, fromSnapshot);
+
+ // If next snapshot is not active then ignore move. Since this could be
a redundant operations.
+ if (nextSnapshot != null && nextSnapshot.getSnapshotStatus() !=
SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE) {
+ throw new OMException("Next snapshot : " + nextSnapshot + " in chain
is not active.",
+ OMException.ResultCodes.INVALID_SNAPSHOT_ERROR);
+ }
+
+ String expectedBucketKeyPrefix =
omMetadataManager.getBucketKeyPrefix(fromSnapshot.getVolumeName(),
+ fromSnapshot.getBucketName());
+ String expectedBucketKeyPrefixFSO =
omMetadataManager.getBucketKeyPrefixFSO(fromSnapshot.getVolumeName(),
+ fromSnapshot.getBucketName());
+
+ // Filter only deleted keys with atlest one keyInfo per key.
Review Comment:
typo:
```suggestion
// Filter only deleted keys with at least one keyInfo per key.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveTableKeysRequest.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om.request.snapshot;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.TransactionInfo;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import
org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveTableKeysResponse;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.upgrade.DisallowedUntilLayoutVersion;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveTableKeysRequest;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf;
+import static
org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.FILESYSTEM_SNAPSHOT;
+
+/**
+ * Handles OMSnapshotMoveTableKeysRequest Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
+ */
+public class OMSnapshotMoveTableKeysRequest extends OMClientRequest {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(OMSnapshotMoveTableKeysRequest.class);
+
+ public OMSnapshotMoveTableKeysRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @DisallowedUntilLayoutVersion(FILESYSTEM_SNAPSHOT)
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
TermIndex termIndex) {
+ OmMetadataManagerImpl omMetadataManager = (OmMetadataManagerImpl)
ozoneManager.getMetadataManager();
+ SnapshotChainManager snapshotChainManager =
omMetadataManager.getSnapshotChainManager();
+
+ SnapshotMoveTableKeysRequest moveTableKeysRequest =
getOmRequest().getSnapshotMoveTableKeysRequest();
+
+ OMClientResponse omClientResponse;
+ OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
OmResponseUtil.getOMResponseBuilder(getOmRequest());
+ try {
+ SnapshotInfo fromSnapshot = SnapshotUtils.getSnapshotInfo(ozoneManager,
+ snapshotChainManager,
fromProtobuf(moveTableKeysRequest.getFromSnapshotID()));
+ // If there is no Non-Deleted Snapshot move the
+ // keys to Active Object Store.
+ SnapshotInfo nextSnapshot = SnapshotUtils.getNextSnapshot(ozoneManager,
snapshotChainManager, fromSnapshot);
+
+ // If next snapshot is not active then ignore move. Since this could be
a redundant operations.
+ if (nextSnapshot != null && nextSnapshot.getSnapshotStatus() !=
SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE) {
+ throw new OMException("Next snapshot : " + nextSnapshot + " in chain
is not active.",
+ OMException.ResultCodes.INVALID_SNAPSHOT_ERROR);
+ }
+
+ String expectedBucketKeyPrefix =
omMetadataManager.getBucketKeyPrefix(fromSnapshot.getVolumeName(),
+ fromSnapshot.getBucketName());
+ String expectedBucketKeyPrefixFSO =
omMetadataManager.getBucketKeyPrefixFSO(fromSnapshot.getVolumeName(),
+ fromSnapshot.getBucketName());
+
+ // Filter only deleted keys with atlest one keyInfo per key.
+ Set<String> keys = new HashSet<>();
+ List<SnapshotMoveKeyInfos> deletedKeys =
+ moveTableKeysRequest.getDeletedKeysList().stream()
+ .filter(snapshotMoveKeyInfos ->
!snapshotMoveKeyInfos.getKeyInfosList().isEmpty())
+ .collect(Collectors.toList());
Review Comment:
This is an extra traversal. Anyways you are traversing it in loop 101-112.
Same for other rename and dir.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -838,6 +838,28 @@ public String getBucketKey(String volume, String bucket) {
return builder.toString();
}
+ /**
+ * Given a volume and bucket, return the corresponding DB key prefix.
+ *
+ * @param volume - Volume name
+ * @param bucket - Bucket name
+ */
+ @Override
+ public String getBucketKeyPrefix(String volume, String bucket) {
+ return getOzoneKey(volume, bucket, OM_KEY_PREFIX);
+ }
+
+ /**
+ * Given a volume and bucket, return the corresponding DB key prefix.
Review Comment:
nit: `... for FSO`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java:
##########
@@ -86,6 +88,13 @@ public static SnapshotInfo getSnapshotInfo(final
OzoneManager ozoneManager,
return snapshotInfo;
}
+ public static SnapshotInfo getSnapshotInfo(OzoneManager ozoneManager,
+ SnapshotChainManager chainManager,
+ UUID snapshotId) throws
IOException {
+ String tableKey = chainManager.getTableKey(snapshotId);
Review Comment:
It is fine if a particular operation fails but make sure that it doesn't
crash OM.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveTableKeysRequest.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om.request.snapshot;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.TransactionInfo;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import
org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveTableKeysResponse;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.upgrade.DisallowedUntilLayoutVersion;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveTableKeysRequest;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf;
+import static
org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.FILESYSTEM_SNAPSHOT;
+
+/**
+ * Handles OMSnapshotMoveTableKeysRequest Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
+ */
+public class OMSnapshotMoveTableKeysRequest extends OMClientRequest {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(OMSnapshotMoveTableKeysRequest.class);
+
+ public OMSnapshotMoveTableKeysRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @DisallowedUntilLayoutVersion(FILESYSTEM_SNAPSHOT)
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
TermIndex termIndex) {
+ OmMetadataManagerImpl omMetadataManager = (OmMetadataManagerImpl)
ozoneManager.getMetadataManager();
+ SnapshotChainManager snapshotChainManager =
omMetadataManager.getSnapshotChainManager();
+
+ SnapshotMoveTableKeysRequest moveTableKeysRequest =
getOmRequest().getSnapshotMoveTableKeysRequest();
+
+ OMClientResponse omClientResponse;
+ OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
OmResponseUtil.getOMResponseBuilder(getOmRequest());
+ try {
+ SnapshotInfo fromSnapshot = SnapshotUtils.getSnapshotInfo(ozoneManager,
+ snapshotChainManager,
fromProtobuf(moveTableKeysRequest.getFromSnapshotID()));
+ // If there is no Non-Deleted Snapshot move the
+ // keys to Active Object Store.
Review Comment:
```suggestion
// If there is no Non-Deleted Snapshot move the keys to Active Object
Store.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
