Copilot commented on code in PR #8982:
URL: https://github.com/apache/ozone/pull/8982#discussion_r2311588303
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java:
##########
@@ -95,55 +119,62 @@ private File writeToYaml(String snapshotName) throws
IOException {
// Check YAML file exists
assertTrue(yamlFile.exists());
- return yamlFile;
+ return Pair.of(yamlFile, previousSnapshotId);
}
@Test
public void testWriteToYaml() throws IOException {
- File yamlFile = writeToYaml("snapshot1");
+ Pair<File, UUID> yamlFilePrevIdPair = writeToYaml("snapshot1");
+ File yamlFile = yamlFilePrevIdPair.getLeft();
+ UUID prevSnapId = yamlFilePrevIdPair.getRight();
// Read from YAML file
OmSnapshotLocalDataYaml snapshotData =
OmSnapshotLocalDataYaml.getFromYamlFile(yamlFile);
// Verify fields
- assertEquals(42, snapshotData.getVersion());
+ assertEquals(44, snapshotData.getVersion());
Review Comment:
The test expects version 44 but the setup code sets version to 42. This
causes a test assertion failure - the expected version should be 42 to match
the setup.
```suggestion
assertEquals(42, snapshotData.getVersion());
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java:
##########
@@ -256,4 +321,26 @@ public static OmSnapshotLocalDataYaml
getFromYamlStream(InputStream input) throw
return dataYaml;
}
+
+ private static class OmSnapshotLocalDataYamlProvider {
+ private static volatile Yaml instance = getInstance();
Review Comment:
This creates infinite recursion during class initialization. The static
field should be initialized to null, not by calling getInstance().
```suggestion
private static volatile Yaml instance = null;
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -602,31 +607,22 @@ private static void
deleteKeysFromDelKeyTableInSnapshotScope(
* @param store AOS or snapshot DB for uncompacted or compacted snapshot
respectively.
* @return a Map of (table, set of SST files corresponding to the table)
*/
- private static Map<String, Set<String>> getSnapshotSSTFileList(RDBStore
store)
+ private static List<LiveFileMetaData> getSnapshotSSTFileList(RDBStore store)
throws IOException {
- Map<String, Set<String>> sstFileList = new HashMap<>();
- List<LiveFileMetaData> liveFileMetaDataList =
store.getDb().getLiveFilesMetaData();
- liveFileMetaDataList.forEach(lfm -> {
- String cfName = StringUtils.bytes2String(lfm.columnFamilyName());
- if (COLUMN_FAMILIES_TO_TRACK_IN_SNAPSHOT.contains(cfName)) {
- sstFileList.computeIfAbsent(cfName, k -> new
HashSet<>()).add(lfm.fileName());
- }
- });
- return sstFileList;
+ return store.getDb().getLiveFilesMetaData().stream()
+ .filter(lfm ->
COLUMN_FAMILIES_TO_TRACK_IN_SNAPSHOT.contains(StringUtils.bytes2String(lfm.columnFamilyName())))
+ .collect(Collectors.toList());
}
/**
* Creates and writes snapshot local properties to a YAML file with
uncompacted SST file list.
- * @param omMetadataManager the metadata manager
- * @param snapshotInfo The metadata of snapshot to be created
- * @param store The store used to get uncompacted SST file list from.
+ * @param snapshotStore snapshot metadata manager.
*/
- public static void createNewOmSnapshotLocalDataFile(
- OMMetadataManager omMetadataManager, SnapshotInfo snapshotInfo, RDBStore
store)
- throws IOException {
- Path snapshotLocalDataPath =
Paths.get(getSnapshotLocalPropertyYamlPath(omMetadataManager, snapshotInfo));
+ public static void createNewOmSnapshotLocalDataFile(RDBStore snapshotStore,
SnapshotInfo snapshotInfo) throws IOException {
+ Path snapshotLocalDataPath =
Paths.get(getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath()));
Review Comment:
The method signature changed to accept
snapshotStore.getDbLocation().toPath() but getSnapshotLocalPropertyYamlPath
likely expects different parameters. This may cause a compilation error or
runtime failure.
```suggestion
Path snapshotLocalDataPath =
getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath());
```
--
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]
