yihua commented on code in PR #12826:
URL: https://github.com/apache/hudi/pull/12826#discussion_r1975977880
##########
hudi-common/src/test/java/org/apache/hudi/common/table/timeline/versioning/BaseTestCommitMetadataSerDe.java:
##########
@@ -72,7 +85,7 @@ protected void testEmptyMetadataSerDe() throws Exception {
assertTrue(serialized.isPresent());
// Deserialize
- HoodieCommitMetadata deserialized = serDe.deserialize(instant,
serialized.get(), HoodieCommitMetadata.class);
+ HoodieCommitMetadata deserialized = serDe.deserialize(instant, new
ByteArrayInputStream(serialized.get()), () -> true, HoodieCommitMetadata.class);
Review Comment:
Similar here. Revisit all `() -> true` to see if they should be `() ->
false` (non-empty file).
##########
hudi-common/src/test/java/org/apache/hudi/common/table/timeline/versioning/BaseTestCommitMetadataSerDe.java:
##########
@@ -133,15 +146,113 @@ protected void testReplaceCommitMetadataSerDe() throws
Exception {
// Serialize and deserialize
Option<byte[]> serialized = serDe.serialize(metadata);
assertTrue(serialized.isPresent());
- HoodieReplaceCommitMetadata deserialized = serDe.deserialize(instant,
serialized.get(), HoodieReplaceCommitMetadata.class);
+ HoodieReplaceCommitMetadata deserialized = serDe.deserialize(instant, new
ByteArrayInputStream(serialized.get()), () -> true,
HoodieReplaceCommitMetadata.class);
// Verify
verifyReplaceCommitMetadata(deserialized);
verifyWriteStat(deserialized.getPartitionToWriteStats().get(TEST_PARTITION_PATH).get(0));
verifyReplaceFileIds(deserialized.getPartitionToReplaceFileIds());
}
- private HoodieWriteStat createTestWriteStat() {
+ private StoragePathInfo generateFileStatus(String filePath) {
+ return new StoragePathInfo(new StoragePath(filePath), 1, true, (short) 2,
1000000L, 1);
+ }
+
+ @Test
+ protected void testRollbackMetadataSerDe() throws Exception {
+ // Create rollback metadata
+ HoodieRollbackMetadata metadata = new HoodieRollbackMetadata();
+
+ // Create rollback stat for partition2 with log files
+ Map<StoragePathInfo, Long> commandBlocksCount = new HashMap<>();
+
commandBlocksCount.put(generateFileStatus("file:///path/1/partition2/.log.1"),
1L);
+ HoodieRollbackPartitionMetadata rollbackPartitionMetadata = new
HoodieRollbackPartitionMetadata();
+ rollbackPartitionMetadata.setPartitionPath("p1");
+ rollbackPartitionMetadata.setSuccessDeleteFiles(Arrays.asList("f1"));
+ rollbackPartitionMetadata.setFailedDeleteFiles(new ArrayList<>());
+ rollbackPartitionMetadata.setRollbackLogFiles(new HashMap<>());
+ Map<String, HoodieRollbackPartitionMetadata> partitionMetadataMap = new
HashMap<>();
+ partitionMetadataMap.put("p1", rollbackPartitionMetadata);
+ metadata.setPartitionMetadata(partitionMetadataMap);
+
+ // Set instant to rollback
+ metadata.setInstantsRollback(Collections.singletonList(new
HoodieInstantInfo("001", HoodieTimeline.COMMIT_ACTION)));
+
+ // Set other metadata fields
+ metadata.setStartRollbackTime("002");
+ metadata.setTimeTakenInMillis(100);
+ metadata.setTotalFilesDeleted(1);
+ metadata.setCommitsRollback(Arrays.asList("111", "222"));
+
+ // Create SerDe instance and test instant
+ CommitMetadataSerDe serDe = getSerDe();
+ HoodieInstant instant = createTestInstant("rollback", "002");
+
+ // Serialize and deserialize
+ Option<byte[]> serialized =
TimelineMetadataUtils.serializeRollbackMetadata(metadata);
+ assertTrue(serialized.isPresent());
+ HoodieRollbackMetadata deserialized = serDe.deserialize(instant, new
ByteArrayInputStream(serialized.get()), () -> true,
HoodieRollbackMetadata.class);
+
+ // Verify
+ assertNotNull(deserialized);
+
+ // Verify other fields
+ assertEquals(Collections.singletonList(new HoodieInstantInfo("001",
HoodieTimeline.COMMIT_ACTION)),
+ deserialized.getInstantsRollback());
+ assertEquals("002", deserialized.getStartRollbackTime());
+ assertEquals(100, deserialized.getTimeTakenInMillis());
+ assertEquals(1, deserialized.getTotalFilesDeleted());
+ }
+
+ @Test
+ protected void testEmptyFile() throws Exception {
+ // Create SerDe instance and test instant
+ CommitMetadataSerDe serDe = getSerDe();
+ HoodieInstant instant = createTestInstant("rollback", "002");
+
+ // Serialize and deserialize
+ Option<byte[]> serialized = Option.of(new byte[]{});
+ HoodieRollbackMetadata deserialized = serDe.deserialize(instant, new
ByteArrayInputStream(serialized.get()), () -> true,
HoodieRollbackMetadata.class);
Review Comment:
This is one of the only handful of places that should use `() -> true` as
the test intends to cover empty file case.
##########
hudi-common/src/test/java/org/apache/hudi/common/table/timeline/versioning/BaseTestCommitMetadataSerDe.java:
##########
@@ -178,7 +289,7 @@ private HoodieWriteStat createTestWriteStat() {
return writeStat;
}
- private void verifyCommitMetadata(HoodieCommitMetadata metadata) {
+ public void verifyCommitMetadata(HoodieCommitMetadata metadata) {
Review Comment:
Could this be `protected`?
##########
hudi-common/src/test/java/org/apache/hudi/common/table/timeline/versioning/BaseTestCommitMetadataSerDe.java:
##########
@@ -194,7 +305,7 @@ private void
verifyReplaceCommitMetadata(HoodieReplaceCommitMetadata metadata) {
assertEquals("test-value-2",
metadata.getExtraMetadata().get("test-key-2"));
}
- private void verifyWriteStat(HoodieWriteStat stat) {
+ public void verifyWriteStat(HoodieWriteStat stat) {
Review Comment:
Could this be `protected`?
--
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]
