jojochuang commented on code in PR #8678: URL: https://github.com/apache/ozone/pull/8678#discussion_r2196381889
########## hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java: ########## @@ -769,6 +770,15 @@ public static String getSnapshotPath(OzoneConfiguration conf, OM_DB_NAME + checkpointDirName; } + public static String extractSnapshotIDFromCheckpointDirName(String snapshotPath) { Review Comment: this public static method does not have test coverage, is only used by OMDBCheckpointServletInodeBasedXfer, and does not use any internal variables or method inside OmSnapshotManager. It should be moved into OMDBCheckpointServletInodeBasedXfer as a private method. Otherwise if it is intended to be used as is later, please add javadoc and test coverage. ########## hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java: ########## @@ -358,5 +360,13 @@ public String getName() { public String getLocationConfigKey() { return OMConfigKeys.OZONE_OM_DB_DIRS; } + + public static List<String> getAllColumnFamilies() { Review Comment: ```suggestion /** * Returns a list of all column family names defined in the database. * * @return list of column family names */ public static List<String> getAllColumnFamilies() { ``` ########## hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java: ########## @@ -278,13 +276,150 @@ void testContentsOfTarballWithSnapshot() throws Exception { assertFalse(hardlinkFilePath.toFile().exists()); } + /** + * Verifies that a manually added entry to the snapshot's delete table + * is persisted and can be retrieved from snapshot db loaded from OM DB checkpoint. + */ + @Test + public void testSnapshotDBConsistency() throws Exception { + String volumeName = "vol" + RandomStringUtils.secure().nextNumeric(5); + String bucketName = "buck" + RandomStringUtils.secure().nextNumeric(5); + AtomicReference<DBCheckpoint> realCheckpoint = new AtomicReference<>(); + setupClusterAndMocks(volumeName, bucketName, realCheckpoint); + List<OzoneSnapshot> snapshots = new ArrayList<>(); + client.getObjectStore().listSnapshot(volumeName, bucketName, "", null) + .forEachRemaining(snapshots::add); + OzoneSnapshot snapshotToModify = snapshots.get(0); + String dummyKey = "dummyKey"; + writeDummyKeyToDeleteTableOfSnapshotDB(snapshotToModify, bucketName, volumeName, dummyKey); + // Get the tarball. + omDbCheckpointServletMock.doGet(requestMock, responseMock); + String testDirName = folder.resolve("testDir").toString(); + String newDbDirName = testDirName + OM_KEY_PREFIX + OM_DB_NAME; + File newDbDir = new File(newDbDirName); + assertTrue(newDbDir.mkdirs()); + FileUtil.unTar(tempFile, newDbDir); + Set<Path> allPathsInTarball = getAllPathsInTarball(newDbDir); + // create hardlinks now + OmSnapshotUtils.createHardLinks(newDbDir.toPath()); + for (Path old : allPathsInTarball) { + assertTrue(old.toFile().delete()); + } + Path snapshotDbDir = Paths.get(newDbDir.toPath().toString(), OM_SNAPSHOT_CHECKPOINT_DIR, + OM_DB_NAME + "-" + snapshotToModify.getSnapshotId()); + deleteWalFiles(snapshotDbDir); + assertTrue(Files.exists(snapshotDbDir)); + String value = getValueFromSnapshotDeleteTable(dummyKey, snapshotDbDir.toString()); + assertNotNull(value); + } + + private static void deleteWalFiles(Path snapshotDbDir) throws IOException { + try (Stream<Path> filesInTarball = Files.list(snapshotDbDir)) { + List<Path> files = filesInTarball.filter(p -> p.toString().contains(".log")) + .collect(Collectors.toList()); + for (Path p : files) { + Files.delete(p); + } + } + } + + private static Set<Path> getAllPathsInTarball(File newDbDir) throws IOException { + Set<Path> allPathsInTarball = new HashSet<>(); + try (Stream<Path> filesInTarball = Files.list(newDbDir.toPath())) { + List<Path> files = filesInTarball.collect(Collectors.toList()); + for (Path p : files) { + File file = p.toFile(); + if (file.getName().equals(OmSnapshotManager.OM_HARDLINK_FILE)) { + continue; + } + allPathsInTarball.add(p); + } + } + return allPathsInTarball; + } + + private void writeDummyKeyToDeleteTableOfSnapshotDB(OzoneSnapshot snapshotToModify, String bucketName, + String volumeName, String keyName) + throws IOException { + try (UncheckedAutoCloseableSupplier<OmSnapshot> supplier = om.getOmSnapshotManager() + .getSnapshot(snapshotToModify.getSnapshotId())) { + OmSnapshot omSnapshot = supplier.get(); + OmKeyInfo dummyOmKeyInfo = + new OmKeyInfo.Builder().setBucketName(bucketName).setVolumeName(volumeName).setKeyName(keyName) + .setReplicationConfig(StandaloneReplicationConfig.getInstance(ONE)).build(); + RepeatedOmKeyInfo dummyRepeatedKeyInfo = + new RepeatedOmKeyInfo.Builder().setOmKeyInfos(Collections.singletonList(dummyOmKeyInfo)).build(); + omSnapshot.getMetadataManager().getDeletedTable().put(dummyOmKeyInfo.getKeyName(), dummyRepeatedKeyInfo); + } + } + + private void setupClusterAndMocks(String volumeName, String bucketName, + AtomicReference<DBCheckpoint> realCheckpoint) throws Exception { + setupCluster(); + setupMocks(); + om.getKeyManager().getSnapshotSstFilteringService().pause(); + when(requestMock.getParameter(OZONE_DB_CHECKPOINT_INCLUDE_SNAPSHOT_DATA)).thenReturn("true"); + // Create a "spy" dbstore keep track of the checkpoint. + writeData(volumeName, bucketName, true); + DBStore dbStore = om.getMetadataManager().getStore(); + DBStore spyDbStore = spy(dbStore); + when(spyDbStore.getCheckpoint(true)).thenAnswer(b -> { + DBCheckpoint checkpoint = spy(dbStore.getCheckpoint(true)); + // Don't delete the checkpoint, because we need to compare it + // with the snapshot data. + doNothing().when(checkpoint).cleanupCheckpoint(); + realCheckpoint.set(checkpoint); + return checkpoint; + }); + // Init the mock with the spyDbstore + doCallRealMethod().when(omDbCheckpointServletMock).initialize(any(), any(), + eq(false), any(), any(), eq(false)); + omDbCheckpointServletMock.initialize(spyDbStore, om.getMetrics().getDBCheckpointMetrics(), + false, + om.getOmAdminUsernames(), om.getOmAdminGroups(), false); + when(responseMock.getOutputStream()).thenReturn(servletOutputStream); + } + + String getValueFromSnapshotDeleteTable(String key, String snapshotDB) { Review Comment: ```suggestion private String getValueFromSnapshotDeleteTable(String key, String snapshotDB) { ``` -- 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: issues-unsubscr...@ozone.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org For additional commands, e-mail: issues-h...@ozone.apache.org