diff --git a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/om/TestSnapshotChainRepair.java b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/om/TestSnapshotChainRepair.java index ba96742c238..e09b07707df 100644 --- a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/om/TestSnapshotChainRepair.java +++ b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/om/TestSnapshotChainRepair.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.repair.om; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.hdds.utils.db.StringCodec; import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksDB; @@ -25,7 +26,6 @@ import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; import org.apache.hadoop.ozone.repair.OzoneRepair; import org.apache.ozone.test.GenericTestUtils; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -34,6 +34,7 @@ import org.mockito.MockedStatic; import org.rocksdb.ColumnFamilyHandle; import org.rocksdb.RocksDB; +import org.rocksdb.RocksDBException; import org.rocksdb.RocksIterator; import org.rocksdb.ColumnFamilyDescriptor; @@ -47,13 +48,13 @@ import static org.apache.ozone.test.IntLambda.withTextFromSystemIn; import static org.apache.hadoop.ozone.OzoneConsts.SNAPSHOT_INFO_TABLE; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; -import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -62,12 +63,14 @@ */ public class TestSnapshotChainRepair { + private static final String VOLUME_NAME = "vol1"; + private static final String BUCKET_NAME = "bucket1"; + private static final String DB_PATH = "testDBPath"; + private ManagedRocksDB managedRocksDB; private RocksDB rocksDB; private ColumnFamilyHandle columnFamilyHandle; - private static final String DB_PATH = "testDBPath"; - private MockedStatic mockedDB; private MockedStatic mockedUtils; @@ -86,18 +89,11 @@ public void setup() throws Exception { @AfterEach public void tearDown() { - IOUtils.closeQuietly(out, err); - - if (mockedDB != null) { - mockedDB.close(); - } - if (mockedUtils != null) { - mockedUtils.close(); - } + IOUtils.closeQuietly(out, err, mockedDB, mockedUtils); } private void setupMockDB(SnapshotInfo snapshotInfo, - List iteratorSnapshots) throws Exception { + SnapshotInfo... snapshots) throws Exception { managedRocksDB = mock(ManagedRocksDB.class); rocksDB = mock(RocksDB.class); @@ -134,28 +130,22 @@ private void setupMockDB(SnapshotInfo snapshotInfo, when(rocksDB.newIterator(columnFamilyHandle)).thenReturn(rocksIterator); // Setup iterator behavior based on provided snapshots - if (iteratorSnapshots.isEmpty()) { + if (snapshots.length == 0) { when(rocksIterator.isValid()).thenReturn(false); } else { - Boolean[] remainingValidResponses = new Boolean[iteratorSnapshots.size()]; - for (int i = 0; i < iteratorSnapshots.size() - 1; i++) { - remainingValidResponses[i] = true; - } - remainingValidResponses[iteratorSnapshots.size() - 1] = false; + Boolean[] remainingValidResponses = new Boolean[snapshots.length + 1]; + Arrays.fill(remainingValidResponses, true); + remainingValidResponses[remainingValidResponses.length - 1] = false; when(rocksIterator.isValid()) .thenReturn(true, remainingValidResponses); ArrayList valueResponses = new ArrayList<>(); - for (SnapshotInfo snap : iteratorSnapshots) { - try { - valueResponses.add(SnapshotInfo.getCodec().toPersistedFormat(snap)); - } catch (IOException e) { - Assertions.fail("Failed to serialize snapshot info"); - } + for (SnapshotInfo snap : snapshots) { + valueResponses.add(SnapshotInfo.getCodec().toPersistedFormat(snap)); } - byte[] firstValue = valueResponses.get(0); - byte[][] remainingValueResponses = valueResponses.subList(1, valueResponses.size()).toArray(new byte[0][]); + byte[] firstValue = SnapshotInfo.getCodec().toPersistedFormat(snapshotInfo); + byte[][] remainingValueResponses = valueResponses.toArray(new byte[0][]); when(rocksIterator.value()) .thenReturn(firstValue, remainingValueResponses); } @@ -164,213 +154,151 @@ private void setupMockDB(SnapshotInfo snapshotInfo, @ParameterizedTest @ValueSource(booleans = {true, false}) public void testSuccessfulRepair(boolean dryRun) throws Exception { - String volumeName = "vol1"; - String bucketName = "bucket1"; - String snapshotName = "snap1"; - String globalPrevSnapshotName = "global-prev-snap1"; - String pathPrevSnapshotName = "path-prev-snap1"; - - UUID snapshotId = UUID.randomUUID(); - UUID globalPrevSnapshotId = UUID.randomUUID(); - UUID pathPrevSnapshotId = UUID.randomUUID(); - - SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName, snapshotName, snapshotId, 0); - SnapshotInfo globalPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, globalPrevSnapshotName, - globalPrevSnapshotId, 0); - SnapshotInfo pathPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, pathPrevSnapshotName, - pathPrevSnapshotId, 0); - - List iteratorSnapshots = Arrays.asList( - snapshotInfo, globalPrevSnapshot, pathPrevSnapshot); + SnapshotInfo snapshotInfo = newSnapshot(); + SnapshotInfo globalPrevSnapshot = newSnapshot(); + SnapshotInfo pathPrevSnapshot = newSnapshot(); List argsList = new ArrayList<>(Arrays.asList( "om", "snapshot", "chain", - volumeName + "/" + bucketName, - snapshotName, + VOLUME_NAME + "/" + BUCKET_NAME, + snapshotInfo.getName(), "--db", DB_PATH, - "--global-previous", globalPrevSnapshotId.toString(), - "--path-previous", pathPrevSnapshotId.toString())); + "--global-previous", globalPrevSnapshot.getSnapshotId().toString(), + "--path-previous", pathPrevSnapshot.getSnapshotId().toString())); if (dryRun) { argsList.add("--dry-run"); } - setupMockDB(snapshotInfo, iteratorSnapshots); + setupMockDB(snapshotInfo, globalPrevSnapshot, pathPrevSnapshot); CommandLine cli = new OzoneRepair().getCmd(); withTextFromSystemIn("y") .execute(() -> cli.execute(argsList.toArray(new String[0]))); - String output = out.getOutput(); - assertTrue(output.contains("Updating SnapshotInfo to")); + assertThat(out.getOutput()).contains("Updating SnapshotInfo to"); - if (dryRun) { - // Verify DB update was NOT called in dry run mode - verify(rocksDB, never()).put( - eq(columnFamilyHandle), - eq(StringCodec.get().toPersistedFormat(snapshotInfo.getTableKey())), - eq(SnapshotInfo.getCodec().toPersistedFormat(snapshotInfo))); - } else { - // Verify DB update was called with correct parameters - verify(rocksDB).put( - eq(columnFamilyHandle), - eq(StringCodec.get().toPersistedFormat(snapshotInfo.getTableKey())), - eq(SnapshotInfo.getCodec().toPersistedFormat(snapshotInfo))); - assertTrue(output.contains("Snapshot Info is updated")); - } + verifyDbWrite(snapshotInfo, !dryRun); } @Test public void testGlobalPreviousMatchesSnapshotId() throws Exception { - String volumeName = "vol1"; - String bucketName = "bucket1"; - String snapshotName = "snap1"; - - UUID snapshotId = UUID.randomUUID(); - // Use same ID for global previous to trigger error - UUID globalPrevSnapshotId = snapshotId; - UUID pathPrevSnapshotId = UUID.randomUUID(); - - SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName, - snapshotName, snapshotId, 0); - SnapshotInfo pathPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, - "path-prev", pathPrevSnapshotId, 0); - - List iteratorSnapshots = Arrays.asList( - snapshotInfo, pathPrevSnapshot); + SnapshotInfo snapshotInfo = newSnapshot(); + SnapshotInfo pathPrevSnapshot = newSnapshot(); String[] args = new String[] { "om", "snapshot", "chain", - volumeName + "/" + bucketName, - snapshotName, + VOLUME_NAME + "/" + BUCKET_NAME, + snapshotInfo.getName(), "--db", DB_PATH, - "--global-previous", globalPrevSnapshotId.toString(), - "--path-previous", pathPrevSnapshotId.toString(), + // Use same ID for global previous to trigger error + "--global-previous", snapshotInfo.getSnapshotId().toString(), + "--path-previous", pathPrevSnapshot.getSnapshotId().toString(), }; - setupMockDB(snapshotInfo, iteratorSnapshots); + setupMockDB(snapshotInfo, pathPrevSnapshot); CommandLine cli = new OzoneRepair().getCmd(); withTextFromSystemIn("y") .execute(() -> cli.execute(args)); - String errorOutput = err.getOutput(); - assertTrue(errorOutput.contains("globalPreviousSnapshotId: '" + globalPrevSnapshotId + - "' is equal to given snapshot's ID")); + assertThat(err.getOutput()).contains("globalPreviousSnapshotId: '" + snapshotInfo.getSnapshotId() + + "' is equal to given snapshot's ID"); + verifyDbWrite(snapshotInfo, false); } @Test public void testPathPreviousMatchesSnapshotId() throws Exception { - String volumeName = "vol1"; - String bucketName = "bucket1"; - String snapshotName = "snap1"; - - UUID snapshotId = UUID.randomUUID(); - UUID globalPrevSnapshotId = UUID.randomUUID(); - // Use same ID for path previous to trigger error - UUID pathPrevSnapshotId = snapshotId; - - SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName, - snapshotName, snapshotId, 0); - SnapshotInfo globalPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, - "global-prev", globalPrevSnapshotId, 0); - - List iteratorSnapshots = Arrays.asList( - snapshotInfo, globalPrevSnapshot); + SnapshotInfo snapshotInfo = newSnapshot(); + SnapshotInfo globalPrevSnapshot = newSnapshot(); String[] args = new String[] { "om", "snapshot", "chain", - volumeName + "/" + bucketName, - snapshotName, + VOLUME_NAME + "/" + BUCKET_NAME, + snapshotInfo.getName(), "--db", DB_PATH, - "--global-previous", globalPrevSnapshotId.toString(), - "--path-previous", pathPrevSnapshotId.toString(), + "--global-previous", globalPrevSnapshot.getSnapshotId().toString(), + // Use same ID for path previous to trigger error + "--path-previous", snapshotInfo.getSnapshotId().toString(), }; - setupMockDB(snapshotInfo, iteratorSnapshots); + setupMockDB(snapshotInfo, globalPrevSnapshot); CommandLine cli = new OzoneRepair().getCmd(); withTextFromSystemIn("y") .execute(() -> cli.execute(args)); - String errorOutput = err.getOutput(); - assertTrue(errorOutput.contains("pathPreviousSnapshotId: '" + pathPrevSnapshotId + - "' is equal to given snapshot's ID")); + assertThat(err.getOutput()).contains("pathPreviousSnapshotId: '" + snapshotInfo.getSnapshotId() + + "' is equal to given snapshot's ID"); + verifyDbWrite(snapshotInfo, false); } @Test public void testGlobalPreviousDoesNotExist() throws Exception { - String volumeName = "vol1"; - String bucketName = "bucket1"; - String snapshotName = "snap1"; - - UUID snapshotId = UUID.randomUUID(); - UUID globalPrevSnapshotId = UUID.randomUUID(); - UUID pathPrevSnapshotId = UUID.randomUUID(); + String nonexistent = UUID.randomUUID().toString(); - SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName, - snapshotName, snapshotId, 0); - SnapshotInfo pathPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, - "path-prev", pathPrevSnapshotId, 0); - - List iteratorSnapshots = Arrays.asList( - snapshotInfo, pathPrevSnapshot); + SnapshotInfo snapshotInfo = newSnapshot(); + SnapshotInfo pathPrevSnapshot = newSnapshot(); String[] args = new String[] { "om", "snapshot", "chain", - volumeName + "/" + bucketName, - snapshotName, + VOLUME_NAME + "/" + BUCKET_NAME, + snapshotInfo.getName(), "--db", DB_PATH, - "--global-previous", globalPrevSnapshotId.toString(), - "--path-previous", pathPrevSnapshotId.toString(), + "--global-previous", nonexistent, + "--path-previous", pathPrevSnapshot.getSnapshotId().toString(), }; - setupMockDB(snapshotInfo, iteratorSnapshots); + setupMockDB(snapshotInfo, pathPrevSnapshot); CommandLine cli = new OzoneRepair().getCmd(); withTextFromSystemIn("y") .execute(() -> cli.execute(args)); - String errorOutput = err.getOutput(); - assertTrue(errorOutput.contains("globalPreviousSnapshotId: '" + globalPrevSnapshotId + - "' does not exist in snapshotInfoTable")); + assertThat(err.getOutput()).contains("globalPreviousSnapshotId: '" + nonexistent + + "' does not exist in snapshotInfoTable"); + verifyDbWrite(snapshotInfo, false); } @Test public void testPathPreviousDoesNotExist() throws Exception { - String volumeName = "vol1"; - String bucketName = "bucket1"; - String snapshotName = "snap1"; - - UUID snapshotId = UUID.randomUUID(); - UUID globalPrevSnapshotId = UUID.randomUUID(); - UUID pathPrevSnapshotId = UUID.randomUUID(); + String nonexistent = UUID.randomUUID().toString(); - SnapshotInfo snapshotInfo = SnapshotInfo.newInstance(volumeName, bucketName, - snapshotName, snapshotId, 0); - SnapshotInfo globalPrevSnapshot = SnapshotInfo.newInstance(volumeName, bucketName, - "global-prev", globalPrevSnapshotId, 0); - - List iteratorSnapshots = Arrays.asList( - snapshotInfo, globalPrevSnapshot); + SnapshotInfo snapshotInfo = newSnapshot(); + SnapshotInfo globalPrevSnapshot = newSnapshot(); String[] args = new String[] { "om", "snapshot", "chain", - volumeName + "/" + bucketName, - snapshotName, + VOLUME_NAME + "/" + BUCKET_NAME, + snapshotInfo.getName(), "--db", DB_PATH, - "--global-previous", globalPrevSnapshotId.toString(), - "--path-previous", pathPrevSnapshotId.toString(), + "--global-previous", globalPrevSnapshot.getSnapshotId().toString(), + "--path-previous", nonexistent, }; - setupMockDB(snapshotInfo, iteratorSnapshots); + setupMockDB(snapshotInfo, globalPrevSnapshot); CommandLine cli = new OzoneRepair().getCmd(); withTextFromSystemIn("y") .execute(() -> cli.execute(args)); - String errorOutput = err.getOutput(); - assertTrue(errorOutput.contains("pathPreviousSnapshotId: '" + pathPrevSnapshotId + - "' does not exist in snapshotInfoTable")); + assertThat(err.getOutput()).contains("pathPreviousSnapshotId: '" + nonexistent + + "' does not exist in snapshotInfoTable"); + verifyDbWrite(snapshotInfo, false); + } + + private static SnapshotInfo newSnapshot() { + String name = RandomStringUtils.insecure().nextAlphanumeric(10); + return SnapshotInfo.newInstance(VOLUME_NAME, BUCKET_NAME, name, UUID.randomUUID(), 0); + } + + private void verifyDbWrite(SnapshotInfo snapshotInfo, boolean updateExpected) throws RocksDBException, IOException { + verify(rocksDB, times(updateExpected ? 1 : 0)).put( + eq(columnFamilyHandle), + eq(StringCodec.get().toPersistedFormat(snapshotInfo.getTableKey())), + eq(SnapshotInfo.getCodec().toPersistedFormat(snapshotInfo))); + if (updateExpected) { + assertThat(out.get()).contains("Snapshot Info is updated"); + } } }