This is an automated email from the ASF dual-hosted git repository.
siyao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 10aaa57472 HDDS-8530. [Snapshot] Fix for OM crash on restart due to
snapshot chain manager corruption (#4677)
10aaa57472 is described below
commit 10aaa57472ed328334311049094470c4d657d857
Author: Hemant Kumar <[email protected]>
AuthorDate: Wed May 17 18:28:02 2023 -0700
HDDS-8530. [Snapshot] Fix for OM crash on restart due to snapshot chain
manager corruption (#4677)
---
.../hadoop/ozone/om/helpers/SnapshotInfo.java | 10 +-
.../ozone/om/TestOzoneManagerHASnapshot.java | 121 ++++++++
.../src/main/proto/OmClientProtocol.proto | 1 +
.../apache/hadoop/ozone/om/OmSnapshotManager.java | 6 +-
.../hadoop/ozone/om/SnapshotChainManager.java | 318 ++++++++++-----------
.../request/snapshot/OMSnapshotCreateRequest.java | 5 +-
.../request/snapshot/OMSnapshotDeleteRequest.java | 2 +-
.../hadoop/ozone/om/TestOmSnapshotManager.java | 9 +-
.../ozone/om/request/OMRequestTestUtils.java | 4 +-
.../snapshot/TestOMSnapshotDeleteRequest.java | 3 +-
.../snapshot/TestOMSnapshotCreateResponse.java | 4 +-
.../snapshot/TestOMSnapshotDeleteResponse.java | 4 +-
12 files changed, 295 insertions(+), 192 deletions(-)
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java
index bc297a92fd..31f67fbdba 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java
@@ -27,8 +27,6 @@ import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Snapsho
import com.google.common.base.Preconditions;
-import org.apache.hadoop.util.Time;
-
import java.time.format.DateTimeFormatter;
import java.time.Instant;
import java.time.ZonedDateTime;
@@ -479,15 +477,15 @@ public final class SnapshotInfo implements Auditable {
public static SnapshotInfo newInstance(String volumeName,
String bucketName,
String snapshotName,
- String snapshotId) {
+ String snapshotId,
+ long creationTime) {
SnapshotInfo.Builder builder = new SnapshotInfo.Builder();
- long initialTime = Time.now();
if (StringUtils.isBlank(snapshotName)) {
- snapshotName = generateName(initialTime);
+ snapshotName = generateName(creationTime);
}
builder.setSnapshotID(snapshotId)
.setName(snapshotName)
- .setCreationTime(initialTime)
+ .setCreationTime(creationTime)
.setDeletionTime(INVALID_TIMESTAMP)
.setPathPreviousSnapshotID(INITIAL_SNAPSHOT_ID)
.setGlobalPreviousSnapshotID(INITIAL_SNAPSHOT_ID)
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHASnapshot.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHASnapshot.java
new file mode 100644
index 0000000000..3f0267e845
--- /dev/null
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHASnapshot.java
@@ -0,0 +1,121 @@
+/*
+ * 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;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.awaitility.Awaitility.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests snapshot in OM HA setup.
+ */
+@Timeout(300)
+public class TestOzoneManagerHASnapshot extends TestOzoneManagerHA {
+
+ /**
+ * Test snapshotNames are unique among OM nodes when snapshotName is not
+ * passed or empty.
+ */
+ @Test
+ public void testSnapshotNameConsistency() throws Exception {
+ OzoneBucket ozoneBucket = setupBucket();
+ String volumeName = ozoneBucket.getVolumeName();
+ String bucketName = ozoneBucket.getName();
+
+ createKey(ozoneBucket);
+
+ getObjectStore().createSnapshot(volumeName, bucketName, "");
+ List<OzoneManager> ozoneManagers = getCluster().getOzoneManagersList();
+ List<String> snapshotNames = new ArrayList<>();
+
+ for (OzoneManager ozoneManager : ozoneManagers) {
+ await().atMost(Duration.ofSeconds(120))
+ .until(() -> {
+ String snapshotPrefix = OM_KEY_PREFIX + volumeName +
+ OM_KEY_PREFIX + bucketName;
+ SnapshotInfo snapshotInfo = null;
+ try (TableIterator<String, ?
+ extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = ozoneManager.getMetadataManager()
+ .getSnapshotInfoTable().iterator(snapshotPrefix)) {
+ while (iterator.hasNext()) {
+ snapshotInfo = iterator.next().getValue();
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+
+ if (snapshotInfo != null) {
+ snapshotNames.add(snapshotInfo.getName());
+ }
+ return snapshotInfo != null;
+ });
+ }
+
+ assertEquals(1, snapshotNames.stream().distinct().count());
+ assertNotNull(snapshotNames.get(0));
+ assertTrue(snapshotNames.get(0).startsWith("s"));
+ }
+
+ @Test
+ public void testSnapshotChainManagerRestore() throws Exception {
+ List<OzoneBucket> ozoneBuckets = new ArrayList<>();
+ List<String> volumeNames = new ArrayList<>();
+ List<String> bucketNames = new ArrayList<>();
+
+ for (int i = 0; i < 10; i++) {
+ OzoneBucket ozoneBucket = setupBucket();
+ ozoneBuckets.add(ozoneBucket);
+ volumeNames.add(ozoneBucket.getVolumeName());
+ bucketNames.add(ozoneBucket.getName());
+ }
+
+ for (int i = 0; i < 100; i++) {
+ int index = i % 10;
+ createKey(ozoneBuckets.get(index));
+ String snapshot1 = "snapshot-" + RandomStringUtils.randomNumeric(5);
+ getObjectStore().createSnapshot(volumeNames.get(index),
+ bucketNames.get(index), snapshot1);
+ }
+
+ // Restart leader OM
+ OzoneManager omLeader = getCluster().getOMLeader();
+ getCluster().shutdownOzoneManager(omLeader);
+ getCluster().restartOzoneManager(omLeader, true);
+
+ await().atMost(Duration.ofSeconds(180))
+ .until(() -> getCluster().getOMLeader() != null);
+ assertNotNull(getCluster().getOMLeader());
+ }
+}
diff --git
a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
index 2cced1d0af..e115319e9d 100644
--- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
+++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
@@ -1687,6 +1687,7 @@ message CreateSnapshotRequest {
optional string bucketName = 2;
optional string snapshotName = 3;
optional string snapshotId = 4;
+ optional uint64 creationTime = 5;
}
message ListSnapshotRequest {
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
index 3a870d1f07..03daef3dd1 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -401,8 +401,10 @@ public final class OmSnapshotManager implements
AutoCloseable {
.writeLock().unlock();
}
- LOG.info("Created checkpoint : {} for snapshot {}",
- dbCheckpoint.getCheckpointLocation(), snapshotInfo.getName());
+ if (dbCheckpoint != null) {
+ LOG.info("Created checkpoint : {} for snapshot {}",
+ dbCheckpoint.getCheckpointLocation(), snapshotInfo.getName());
+ }
final RocksDBCheckpointDiffer dbCpDiffer =
store.getRocksDBCheckpointDiffer();
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
index 8dc6c703e0..c3ec6d1d61 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
@@ -33,26 +33,27 @@ import org.slf4j.LoggerFactory;
/**
* This class is used for creating and accessing Snapshot Chains.
- *
+ * <p>
* The snapshot chain maintains the in-memory sequence of snapshots
* created in chronological order. There are two such snapshots maintained
* i.) Path based snapshot chain, sequence of snapshots created for a
* given /volume/bucket
* ii.) Global snapshot chain, sequence of all snapshots created in order
- *
+ * <p>
* On start, the snapshot chains are initialized from the on disk
* SnapshotInfoTable from the om RocksDB.
*/
public class SnapshotChainManager {
- private LinkedHashMap<String, SnapshotChainInfo> snapshotChainGlobal;
- private Map<String, LinkedHashMap<String, SnapshotChainInfo>>
- snapshotChainPath;
- private Map<String, String> latestPathSnapshotID;
- private String latestGlobalSnapshotID;
- private Map<String, String> snapshotIdToTableKey;
private static final Logger LOG =
LoggerFactory.getLogger(SnapshotChainManager.class);
+ private final LinkedHashMap<String, SnapshotChainInfo> snapshotChainGlobal;
+ private final Map<String, LinkedHashMap<String, SnapshotChainInfo>>
+ snapshotChainPath;
+ private String latestGlobalSnapshotID;
+ private final Map<String, String> latestPathSnapshotID;
+ private final Map<String, String> snapshotIdToTableKey;
+
public SnapshotChainManager(OMMetadataManager metadataManager)
throws IOException {
snapshotChainGlobal = new LinkedHashMap<>();
@@ -83,10 +84,10 @@ public class SnapshotChainManager {
}
if (prevGlobalID != null &&
!snapshotChainGlobal.containsKey(prevGlobalID)) {
- throw new IOException("Snapshot Chain corruption: "
- + "previous snapshotID given but no associated snapshot "
- + "found in snapshot chain: SnapshotID "
- + prevGlobalID);
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "Previous snapshotId: %s is set for snapshotId: %s but no " +
+ "associated snapshot found in snapshot chain.", prevGlobalID,
+ snapshotID));
}
snapshotChainGlobal.put(snapshotID,
new SnapshotChainInfo(snapshotID, prevGlobalID, null));
@@ -115,10 +116,10 @@ public class SnapshotChainManager {
(!snapshotChainPath
.get(snapshotPath)
.containsKey(prevPathID)))) {
- throw new IOException("Snapshot Chain corruption: "
- + "previous snapshotID given but no associated snapshot "
- + "found in snapshot chain: SnapshotID "
- + prevPathID);
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "Previous snapshotId: %s is set for snapshotId: %s but no " +
+ "associated snapshot found in snapshot chain.", prevPathID,
+ snapshotID));
}
if (prevPathID != null &&
@@ -151,14 +152,16 @@ public class SnapshotChainManager {
String prev =
snapshotChainGlobal.get(snapshotID).getPreviousSnapshotID();
if (prev != null && !snapshotChainGlobal.containsKey(prev)) {
- throw new IOException("Snapshot chain corruption: snapshot node to be "
- + "deleted has prev node element not found in snapshot chain: "
- + "SnapshotID " + prev);
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "SnapshotId: %s to be deleted has previous snapshotId: %s " +
+ "but associated snapshot is not found in snapshot chain.",
+ snapshotID, prev));
}
if (next != null && !snapshotChainGlobal.containsKey(next)) {
- throw new IOException("Snapshot chain corruption: snapshot node to be "
- + "deleted has next node element not found in snapshot chain: "
- + "SnapshotID " + next);
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "SnapshotId: {%s} to be deleted has next snapshotId: %s " +
+ "but associated snapshot is not found in snapshot chain.",
+ snapshotID, next));
}
snapshotChainGlobal.remove(snapshotID);
if (next != null) {
@@ -173,8 +176,8 @@ public class SnapshotChainManager {
}
} else {
// snapshotID not found in snapshot chain, log warning and return
- LOG.warn("Snapshot chain: snapshotID not found: SnapshotID {}",
- snapshotID);
+ LOG.warn("Snapshot chain corruption. SnapshotID: {} is not found in " +
+ "snapshot chain.", snapshotID);
status = false;
}
@@ -183,53 +186,47 @@ public class SnapshotChainManager {
private boolean deleteSnapshotPath(String snapshotPath,
String snapshotID) throws IOException {
- boolean status = true;
if (snapshotChainPath.containsKey(snapshotPath) &&
- snapshotChainPath
- .get(snapshotPath)
- .containsKey(snapshotID)) {
+ snapshotChainPath.get(snapshotPath).containsKey(snapshotID)) {
// reset prev and next snapshot entries in chain ordered list
// for node removal
- String next = snapshotChainPath
+ String nextSnapshotID = snapshotChainPath
.get(snapshotPath)
.get(snapshotID)
.getNextSnapshotID();
- String prev = snapshotChainPath
+ String previousSnapshotID = snapshotChainPath
.get(snapshotPath)
.get(snapshotID)
.getPreviousSnapshotID();
- if (prev != null &&
- !snapshotChainPath
- .get(snapshotPath)
- .containsKey(prev)) {
- throw new IOException("Snapshot chain corruption: snapshot node to "
- + "be deleted has prev node element not found in snapshot "
- + "chain: Snapshot path " + snapshotPath + ", SnapshotID "
- + prev);
+ if (previousSnapshotID != null &&
+ !snapshotChainPath.get(snapshotPath)
+ .containsKey(previousSnapshotID)) {
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "SnapshotId: %s at snapshotPath: %s to be deleted has " +
+ "previous snapshotId: %s but associated snapshot is not " +
+ "found in snapshot chain.", snapshotID, snapshotPath,
+ previousSnapshotID));
}
- if (next != null && !snapshotChainPath
- .get(snapshotPath)
- .containsKey(next)) {
- throw new IOException("Snapshot chain corruption: snapshot node to "
- + "be deleted has next node element not found in snapshot "
- + "chain: Snapshot path " + snapshotPath + ", SnapshotID "
- + next);
+ if (nextSnapshotID != null && !snapshotChainPath.get(snapshotPath)
+ .containsKey(nextSnapshotID)) {
+ throw new IOException(String.format("Snapshot chain corruption. " +
+ "SnapshotId: %s at snapshotPath: %s to be deleted has next " +
+ "snapshotId: %s but associated snapshot is not found in " +
+ "snapshot chain.", snapshotID, snapshotPath,
+ nextSnapshotID));
}
- snapshotChainPath
- .get(snapshotPath)
- .remove(snapshotID);
- if (next != null) {
- snapshotChainPath
- .get(snapshotPath)
- .get(next)
- .setPreviousSnapshotID(prev);
+
+ snapshotChainPath.get(snapshotPath).remove(snapshotID);
+ if (nextSnapshotID != null) {
+ snapshotChainPath.get(snapshotPath)
+ .get(nextSnapshotID)
+ .setPreviousSnapshotID(previousSnapshotID);
}
- if (prev != null) {
- snapshotChainPath
- .get(snapshotPath)
- .get(prev)
- .setNextSnapshotID(next);
+ if (previousSnapshotID != null) {
+ snapshotChainPath.get(snapshotPath)
+ .get(previousSnapshotID)
+ .setNextSnapshotID(nextSnapshotID);
}
// remove path if no entries
if (snapshotChainPath.get(snapshotPath).isEmpty()) {
@@ -238,20 +235,17 @@ public class SnapshotChainManager {
// remove from latest list if necessary
if (latestPathSnapshotID.get(snapshotPath).equals(snapshotID)) {
latestPathSnapshotID.remove(snapshotPath);
- if (prev != null) {
- latestPathSnapshotID.put(snapshotPath, prev);
+ if (previousSnapshotID != null) {
+ latestPathSnapshotID.put(snapshotPath, previousSnapshotID);
}
}
-
+ return true;
} else {
// snapshotID not found in snapshot chain, log warning and return
- LOG.warn("Snapshot chain: snapshotID not found: Snapshot path {}," +
- " SnapshotID {}",
- snapshotPath, snapshotID);
- status = false;
+ LOG.warn("Snapshot chain corruption. SnapshotId: {} is not in chain " +
+ "found for snapshot path {}.", snapshotID, snapshotPath);
+ return false;
}
-
- return status;
}
/**
@@ -259,7 +253,7 @@ public class SnapshotChainManager {
* @param metadataManager OMMetadataManager
*/
private void loadFromSnapshotInfoTable(OMMetadataManager metadataManager)
- throws IOException {
+ throws IOException {
// read from snapshotInfo table to populate
// snapshot chains - both global and local path
try (TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
@@ -275,38 +269,37 @@ public class SnapshotChainManager {
kv = keyIter.next();
snaps.put(kv.getValue().getCreationTime(), kv.getValue());
}
- for (SnapshotInfo sinfo : snaps.values()) {
- addSnapshot(sinfo);
+ for (SnapshotInfo snapshotInfo : snaps.values()) {
+ addSnapshot(snapshotInfo);
}
}
}
/**
* Add snapshot to snapshot chain.
- * @param sinfo SnapshotInfo of snapshot to add to chain.
+ * @param snapshotInfo SnapshotInfo of snapshot to add to chain.
*/
- public void addSnapshot(SnapshotInfo sinfo) throws IOException {
- addSnapshotGlobal(sinfo.getSnapshotID(),
- sinfo.getGlobalPreviousSnapshotID());
- addSnapshotPath(sinfo.getSnapshotPath(),
- sinfo.getSnapshotID(),
- sinfo.getPathPreviousSnapshotID());
+ public void addSnapshot(SnapshotInfo snapshotInfo) throws IOException {
+ addSnapshotGlobal(snapshotInfo.getSnapshotID(),
+ snapshotInfo.getGlobalPreviousSnapshotID());
+ addSnapshotPath(snapshotInfo.getSnapshotPath(),
+ snapshotInfo.getSnapshotID(),
snapshotInfo.getPathPreviousSnapshotID());
// store snapshot ID to snapshot DB table key in the map
- snapshotIdToTableKey.put(sinfo.getSnapshotID(), sinfo.getTableKey());
+ snapshotIdToTableKey.put(snapshotInfo.getSnapshotID(),
+ snapshotInfo.getTableKey());
}
/**
* Delete snapshot from snapshot chain.
- * @param sinfo SnapshotInfo of snapshot to remove from chain.
+ * @param snapshotInfo SnapshotInfo of snapshot to remove from chain.
* @return boolean
*/
- public boolean deleteSnapshot(SnapshotInfo sinfo) throws IOException {
- boolean status;
-
- status = deleteSnapshotGlobal(sinfo.getSnapshotID()) &&
- deleteSnapshotPath(sinfo.getSnapshotPath(), sinfo.getSnapshotID());
+ public boolean deleteSnapshot(SnapshotInfo snapshotInfo) throws IOException {
+ boolean status = deleteSnapshotGlobal(snapshotInfo.getSnapshotID()) &&
+ deleteSnapshotPath(snapshotInfo.getSnapshotPath(),
+ snapshotInfo.getSnapshotID());
if (status) {
- snapshotIdToTableKey.remove(sinfo.getSnapshotID());
+ snapshotIdToTableKey.remove(snapshotInfo.getSnapshotID());
}
return status;
}
@@ -336,19 +329,13 @@ public class SnapshotChainManager {
* @param snapshotID String, snapshot UUID
* @return boolean
*/
- public boolean hasNextGlobalSnapshot(String snapshotID)
- throws NoSuchElementException {
- boolean hasNext = false;
+ public boolean hasNextGlobalSnapshot(String snapshotID) {
if (!snapshotChainGlobal.containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotID {}", snapshotID);
- throw new NoSuchElementException("No snapshot: " + snapshotID);
- }
- if (snapshotChainGlobal
- .get(snapshotID)
- .getNextSnapshotID() != null) {
- hasNext = true;
+ LOG.error("No snapshot for provided snapshotId: {}", snapshotID);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain.", snapshotID));
}
- return hasNext;
+ return snapshotChainGlobal.get(snapshotID).getNextSnapshotID() != null;
}
/**
@@ -357,16 +344,14 @@ public class SnapshotChainManager {
* @return String, snapshot UUID of next snapshot in chain from
* snapshotID
*/
- public String nextGlobalSnapshot(String snapshotID)
- throws NoSuchElementException {
+ public String nextGlobalSnapshot(String snapshotID) {
if (!hasNextGlobalSnapshot(snapshotID)) {
- LOG.error("no following snapshot for provided snapshotID {}",
snapshotID);
- throw new NoSuchElementException("no following snapshot from: "
- + snapshotID);
+ LOG.error("No following snapshot found in snapshot chain for provided " +
+ "snapshotId: {}.", snapshotID);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain.", snapshotID));
}
- return snapshotChainGlobal
- .get(snapshotID)
- .getNextSnapshotID();
+ return snapshotChainGlobal.get(snapshotID).getNextSnapshotID();
}
/**
@@ -375,19 +360,15 @@ public class SnapshotChainManager {
* @param snapshotID String, snapshot UUID
* @return boolean
*/
- public boolean hasPreviousGlobalSnapshot(String snapshotID)
- throws NoSuchElementException {
- boolean hasPrevious = false;
+ public boolean hasPreviousGlobalSnapshot(String snapshotID) {
if (!snapshotChainGlobal.containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotID {}", snapshotID);
- throw new NoSuchElementException("No snapshot: " + snapshotID);
+ LOG.error("No snapshot found in snapshot chain for provided " +
+ "snapshotId: {}.", snapshotID);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain.", snapshotID));
}
- if (snapshotChainGlobal
- .get(snapshotID)
- .getPreviousSnapshotID() != null) {
- hasPrevious = true;
- }
- return hasPrevious;
+
+ return snapshotChainGlobal.get(snapshotID).getPreviousSnapshotID() != null;
}
/**
@@ -396,17 +377,14 @@ public class SnapshotChainManager {
* @return String, snapshot UUID of previous snapshot in chain from
* snapshotID
*/
- public String previousGlobalSnapshot(String snapshotID)
- throws NoSuchElementException {
+ public String previousGlobalSnapshot(String snapshotID) {
if (!hasPreviousGlobalSnapshot(snapshotID)) {
- LOG.error("no preceeding snapshot for provided snapshotID {}",
- snapshotID);
- throw new NoSuchElementException("No preceeding snapshot from: "
- + snapshotID);
+ LOG.error("No preceding snapshot found in snapshot chain for provided " +
+ "snapshotId: {}.", snapshotID);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain.", snapshotID));
}
- return snapshotChainGlobal
- .get(snapshotID)
- .getPreviousSnapshotID();
+ return snapshotChainGlobal.get(snapshotID).getPreviousSnapshotID();
}
/**
@@ -416,23 +394,20 @@ public class SnapshotChainManager {
* @param snapshotID String, snapshot UUID
* @return boolean
*/
- public boolean hasNextPathSnapshot(String snapshotPath, String snapshotID)
- throws NoSuchElementException {
- boolean hasNext = false;
+ public boolean hasNextPathSnapshot(String snapshotPath, String snapshotID) {
if (!snapshotChainPath.containsKey(snapshotPath) ||
!snapshotChainPath.get(snapshotPath).containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotPath {} and "
- + " snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("No such snapshot: "
- + snapshotID + "for path: " + snapshotPath);
+ LOG.error("No snapshot found for provided snapshotId: {} and " +
+ "snapshotPath: {}", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain for snapshotPath: %s.", snapshotID,
+ snapshotPath));
}
- if (snapshotChainPath
+
+ return snapshotChainPath
.get(snapshotPath)
.get(snapshotID)
- .getNextSnapshotID() != null) {
- hasNext = true;
- }
- return hasNext;
+ .getNextSnapshotID() != null;
}
/**
@@ -442,23 +417,19 @@ public class SnapshotChainManager {
* @param snapshotID String, snapshot UUID
* @return boolean
*/
- public boolean hasPreviousPathSnapshot(String snapshotPath, String
snapshotID)
- throws NoSuchElementException {
- boolean hasPrevious = false;
+ public boolean hasPreviousPathSnapshot(String snapshotPath,
+ String snapshotID) {
if (!snapshotChainPath.containsKey(snapshotPath) ||
!snapshotChainPath.get(snapshotPath).containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotPath {} and "
- + " snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("No snapshot: " + snapshotID
- + " for snapshot path: " + snapshotPath);
+ LOG.error("No snapshot found for provided snapshotId: {} and " +
+ "snapshotPath: {}", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain for snapshotPath: %s.", snapshotID,
+ snapshotPath));
}
- if (snapshotChainPath
- .get(snapshotPath)
+ return snapshotChainPath.get(snapshotPath)
.get(snapshotID)
- .getPreviousSnapshotID() != null) {
- hasPrevious = true;
- }
- return hasPrevious;
+ .getPreviousSnapshotID() != null;
}
/**
@@ -468,26 +439,24 @@ public class SnapshotChainManager {
* @return String, snapshot UUID of next snapshot in chain from
* snapshotID
*/
- public String nextPathSnapshot(String snapshotPath, String snapshotID)
- throws NoSuchElementException {
+ public String nextPathSnapshot(String snapshotPath, String snapshotID) {
if (!snapshotChainPath.containsKey(snapshotPath) ||
!snapshotChainPath.get(snapshotPath).containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotPath {} and "
- + " snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("No snapshot: " + snapshotID
- + " for snapshot path:" + snapshotPath);
+ LOG.error("No snapshot found for provided snapshotId: {} and " +
+ "snapshotPath: {}", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain for snapshotPath: %s.", snapshotID,
+ snapshotPath));
}
- if (snapshotChainPath
- .get(snapshotPath)
- .get(snapshotID)
+ if (snapshotChainPath.get(snapshotPath).get(snapshotID)
.getNextSnapshotID() == null) {
- LOG.error("no following snapshot for provided snapshotPath {}, "
- + "snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("no following snapshot from: "
- + snapshotID + "for snapshot path:" + snapshotPath);
+ LOG.error("No following snapshot for provided snapshotId {} and " +
+ "snapshotPath {}.", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("No following snapshot " +
+ "found in snapshot chain for snapshotId: %s and snapshotPath: " +
+ "%s.", snapshotID, snapshotPath));
}
- return snapshotChainPath
- .get(snapshotPath)
+ return snapshotChainPath.get(snapshotPath)
.get(snapshotID)
.getNextSnapshotID();
}
@@ -499,23 +468,24 @@ public class SnapshotChainManager {
* @return String, snapshot UUID of previous snapshot in chain from
* snapshotID
*/
- public String previousPathSnapshot(String snapshotPath, String snapshotID)
- throws NoSuchElementException {
+ public String previousPathSnapshot(String snapshotPath, String snapshotID) {
if (!snapshotChainPath.containsKey(snapshotPath) ||
!snapshotChainPath.get(snapshotPath).containsKey(snapshotID)) {
- LOG.error("no snapshot for provided snapshotPath {} and "
- + " snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("No snapshot: " + snapshotID
- + " for snapshot path:" + snapshotPath);
+ LOG.error("No snapshot found for provided snapshotId: {} and " +
+ "snapshotPath: {}", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("SnapshotId: %s is not " +
+ "found in snapshot chain for snapshotPath: %s.", snapshotID,
+ snapshotPath));
}
if (snapshotChainPath
.get(snapshotPath)
.get(snapshotID)
.getPreviousSnapshotID() == null) {
- LOG.error("no preceeding snapshot for provided snapshotPath {}, "
- + "snapshotID {}", snapshotPath, snapshotID);
- throw new NoSuchElementException("no preceeding snapshot from: "
- + snapshotID + "for snapshot path:" + snapshotPath);
+ LOG.error("No preceding snapshot for provided snapshotId: {} and " +
+ "snapshotPath: {}", snapshotID, snapshotPath);
+ throw new NoSuchElementException(String.format("No preceding snapshot " +
+ "found in snapshot chain for snapshotId: %s and snapshotPath: " +
+ "%s.", snapshotID, snapshotPath));
}
return snapshotChainPath
.get(snapshotPath)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
index 0bed12118d..c92fa8f939 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
@@ -45,6 +45,7 @@ import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRespo
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -82,7 +83,8 @@ public class OMSnapshotCreateRequest extends OMClientRequest {
snapshotInfo = SnapshotInfo.newInstance(volumeName,
bucketName,
possibleName,
- snapshotId);
+ snapshotId,
+ createSnapshotRequest.getCreationTime());
snapshotName = snapshotInfo.getName();
snapshotPath = snapshotInfo.getSnapshotPath();
}
@@ -107,6 +109,7 @@ public class OMSnapshotCreateRequest extends
OMClientRequest {
return omRequest.toBuilder().setCreateSnapshotRequest(
omRequest.getCreateSnapshotRequest().toBuilder()
.setSnapshotId(UUID.randomUUID().toString())
+ .setCreationTime(Time.now())
.build()).build();
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
index 33ec087617..cf2481d2c2 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
@@ -208,7 +208,7 @@ public class OMSnapshotDeleteRequest extends
OMClientRequest {
if (snapshotInfo == null) {
// Dummy SnapshotInfo for logging and audit logging when erred
snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName,
- snapshotName, null);
+ snapshotName, null, Time.now());
}
// Perform audit logging outside the lock
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
index cd1589ee63..bbc2dd6b5a 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
@@ -30,6 +30,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils;
+import org.apache.hadoop.util.Time;
import org.apache.ozone.test.GenericTestUtils;
import org.junit.After;
import org.junit.Assert;
@@ -222,8 +223,10 @@ public class TestOmSnapshotManager {
String volumeName, String bucketName) {
String snapshotName = UUID.randomUUID().toString();
String snapshotId = UUID.randomUUID().toString();
- return SnapshotInfo.newInstance(
- volumeName, bucketName, snapshotName, snapshotId);
+ return SnapshotInfo.newInstance(volumeName,
+ bucketName,
+ snapshotName,
+ snapshotId,
+ Time.now());
}
-
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java
index e5b0271a4b..1dac4ac364 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java
@@ -365,7 +365,7 @@ public final class OMRequestTestUtils {
String volumeName, String bucketName, String snapshotName,
OMMetadataManager omMetadataManager) throws IOException {
SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName,
- bucketName, snapshotName, UUID.randomUUID().toString());
+ bucketName, snapshotName, UUID.randomUUID().toString(), Time.now());
addSnapshotToTable(false, 0L, snapshotInfo, omMetadataManager);
}
@@ -376,7 +376,7 @@ public final class OMRequestTestUtils {
String volumeName, String bucketName, String snapshotName,
OMMetadataManager omMetadataManager) throws IOException {
SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName,
bucketName,
- snapshotName, UUID.randomUUID().toString());
+ snapshotName, UUID.randomUUID().toString(), Time.now());
addSnapshotToTable(true, 0L, snapshotInfo, omMetadataManager);
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
index ae28a9cd31..e09eb26806 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
@@ -41,6 +41,7 @@ import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMReque
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type;
+import org.apache.hadoop.util.Time;
import org.apache.ozone.test.LambdaTestUtils;
import org.junit.After;
import org.junit.Assert;
@@ -206,7 +207,7 @@ public class TestOMSnapshotDeleteRequest {
// add key to cache
SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(
- volumeName, bucketName, snapshotName, null);
+ volumeName, bucketName, snapshotName, null, Time.now());
Assert.assertEquals(SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE,
snapshotInfo.getSnapshotStatus());
omMetadataManager.getSnapshotInfoTable().addCacheEntry(
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
index 8ec9e2713c..d50a1539e1 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
@@ -33,6 +33,7 @@ import org.apache.hadoop.ozone.om.OmSnapshotManager;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.util.Time;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -92,7 +93,8 @@ public class TestOMSnapshotCreateResponse {
SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName,
bucketName,
snapshotName,
- snapshotId);
+ snapshotId,
+ Time.now());
// confirm table is empty
Assert.assertEquals(0,
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java
index 296636ed51..a568bf6db6 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java
@@ -34,6 +34,7 @@ import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteS
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type;
+import org.apache.hadoop.util.Time;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -85,7 +86,8 @@ public class TestOMSnapshotDeleteResponse {
SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName,
bucketName,
snapshotName,
- snapshotId);
+ snapshotId,
+ Time.now());
// confirm table is empty
Assert.assertEquals(0,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
