yihua commented on code in PR #12826:
URL: https://github.com/apache/hudi/pull/12826#discussion_r1975940635
##########
hudi-common/src/main/java/org/apache/hudi/common/util/CleanerUtils.java:
##########
@@ -169,22 +168,14 @@ public static Option<HoodieInstant>
getEarliestCommitToRetain(
public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, HoodieInstant cleanInstant)
throws IOException {
CleanPlanMigrator cleanPlanMigrator = new CleanPlanMigrator(metaClient);
- HoodieCleanerPlan cleanerPlan =
TimelineMetadataUtils.deserializeAvroMetadata(
-
metaClient.getActiveTimeline().readCleanerInfoAsBytes(cleanInstant).get(),
HoodieCleanerPlan.class);
+ HoodieCleanerPlan cleanerPlan =
metaClient.getActiveTimeline().loadCleanerPlan(cleanInstant);
return cleanPlanMigrator.upgradeToLatest(cleanerPlan,
cleanerPlan.getVersion());
}
- /**
- * Get Latest version of cleaner plan corresponding to a clean instant.
- *
- * @param metaClient Hoodie Table Meta Client
- * @return Cleaner plan corresponding to clean instant
- * @throws IOException
- */
- public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, byte[] details)
+ public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, InputStream in)
Review Comment:
Similar here. Could we only have one API
`getCleanerPlan(HoodieTableMetaClient metaClient, HoodieInstant cleanInstant)`?
##########
hudi-common/src/main/java/org/apache/hudi/common/util/CleanerUtils.java:
##########
@@ -107,21 +109,18 @@ public static HoodieCleanMetadata
convertCleanMetadata(String startCleanTime,
*/
public static HoodieCleanMetadata getCleanerMetadata(HoodieTableMetaClient
metaClient, HoodieInstant cleanInstant)
throws IOException {
- CleanMetadataMigrator metadataMigrator = new
CleanMetadataMigrator(metaClient);
- HoodieCleanMetadata cleanMetadata =
TimelineMetadataUtils.deserializeHoodieCleanMetadata(
-
metaClient.getActiveTimeline().readCleanerInfoAsBytes(cleanInstant).get());
- return metadataMigrator.upgradeToLatest(cleanMetadata,
cleanMetadata.getVersion());
+ HoodieCleanMetadata cleanMetadata =
metaClient.getActiveTimeline().loadHoodieCleanMetadata(cleanInstant);
+ return upgradeCleanMetadata(metaClient, cleanMetadata);
}
- /**
- * Get Latest Version of Hoodie Cleaner Metadata - Output of cleaner
operation.
- * @return Latest version of Clean metadata corresponding to clean instant
- * @throws IOException
- */
- public static HoodieCleanMetadata getCleanerMetadata(HoodieTableMetaClient
metaClient, byte[] details)
+ public static HoodieCleanMetadata getCleanerMetadata(HoodieTableMetaClient
metaClient, InputStream inputStream)
throws IOException {
+ HoodieCleanMetadata cleanMetadata = deserializeAvroMetadata(inputStream,
HoodieCleanMetadata.class);
+ return upgradeCleanMetadata(metaClient, cleanMetadata);
+ }
Review Comment:
Is this needed? OR should all callers use
`getCleanerMetadata(HoodieTableMetaClient metaClient, HoodieInstant
cleanInstant)` to get cleaner metadata?
##########
hudi-common/src/main/java/org/apache/hudi/common/util/CleanerUtils.java:
##########
@@ -107,21 +109,18 @@ public static HoodieCleanMetadata
convertCleanMetadata(String startCleanTime,
*/
public static HoodieCleanMetadata getCleanerMetadata(HoodieTableMetaClient
metaClient, HoodieInstant cleanInstant)
throws IOException {
- CleanMetadataMigrator metadataMigrator = new
CleanMetadataMigrator(metaClient);
- HoodieCleanMetadata cleanMetadata =
TimelineMetadataUtils.deserializeHoodieCleanMetadata(
-
metaClient.getActiveTimeline().readCleanerInfoAsBytes(cleanInstant).get());
- return metadataMigrator.upgradeToLatest(cleanMetadata,
cleanMetadata.getVersion());
+ HoodieCleanMetadata cleanMetadata =
metaClient.getActiveTimeline().loadHoodieCleanMetadata(cleanInstant);
+ return upgradeCleanMetadata(metaClient, cleanMetadata);
Review Comment:
nit: We should merge these cleaner metadata utils to `HoodieTimeline` class.
We can take that up in a follow-up PR with a new JIRA ticket.
##########
hudi-common/src/main/java/org/apache/hudi/common/util/CleanerUtils.java:
##########
@@ -169,22 +168,14 @@ public static Option<HoodieInstant>
getEarliestCommitToRetain(
public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, HoodieInstant cleanInstant)
throws IOException {
CleanPlanMigrator cleanPlanMigrator = new CleanPlanMigrator(metaClient);
- HoodieCleanerPlan cleanerPlan =
TimelineMetadataUtils.deserializeAvroMetadata(
-
metaClient.getActiveTimeline().readCleanerInfoAsBytes(cleanInstant).get(),
HoodieCleanerPlan.class);
+ HoodieCleanerPlan cleanerPlan =
metaClient.getActiveTimeline().loadCleanerPlan(cleanInstant);
return cleanPlanMigrator.upgradeToLatest(cleanerPlan,
cleanerPlan.getVersion());
}
- /**
- * Get Latest version of cleaner plan corresponding to a clean instant.
- *
- * @param metaClient Hoodie Table Meta Client
- * @return Cleaner plan corresponding to clean instant
- * @throws IOException
- */
- public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, byte[] details)
+ public static HoodieCleanerPlan getCleanerPlan(HoodieTableMetaClient
metaClient, InputStream in)
throws IOException {
CleanPlanMigrator cleanPlanMigrator = new CleanPlanMigrator(metaClient);
- HoodieCleanerPlan cleanerPlan =
TimelineMetadataUtils.deserializeAvroMetadata(details, HoodieCleanerPlan.class);
+ HoodieCleanerPlan cleanerPlan =
TimelineMetadataUtils.deserializeAvroMetadata(in, HoodieCleanerPlan.class);
Review Comment:
nit: use `readCleanerPlan`
--
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]
