yihua commented on code in PR #10945:
URL: https://github.com/apache/hudi/pull/10945#discussion_r1548795405
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1367,10 +1362,8 @@ protected void compactIfNecessary(BaseHoodieWriteClient
writeClient, String late
LOG.info("Compaction is scheduled for timestamp " +
compactionInstantTime);
writeClient.compact(compactionInstantTime);
} else if (metadataWriteConfig.isLogCompactionEnabled()) {
- // Schedule and execute log compaction with suffixes based on the same
instant time. This ensures that any future
- // delta commits synced over will not have an instant time lesser than
the last completed instant on the
- // metadata table.
- final String logCompactionInstantTime =
HoodieTableMetadataUtil.createLogCompactionTimestamp(latestDeltacommitTime);
+ // Schedule and execute log compaction with new instant time.
+ final String logCompactionInstantTime =
metadataMetaClient.createNewInstantTime(false);
Review Comment:
To remind ourselves, for MDT, log compaction is no longer necessary if full
compaction on MDT can be scheduled and executed without any restriction.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1375,33 +1375,15 @@ public static Set<String>
getValidInstantTimestamps(HoodieTableMetaClient dataMe
/**
* Checks if the Instant is a delta commit and has a valid suffix for
operations on MDT.
*
+ * @param datasetPendingInstants The dataset pending instants
* @param instant {@code HoodieInstant} to check.
* @return {@code true} if the instant is valid.
*/
- public static boolean isValidInstant(HoodieInstant instant) {
- // Should be a deltacommit
- if (!instant.getAction().equals(HoodieTimeline.DELTA_COMMIT_ACTION)) {
- return false;
- }
-
- // Check correct length. The timestamp should have a suffix over the
timeline's timestamp format.
- final String instantTime = instant.getTimestamp();
- if (!(instantTime.length() == MILLIS_INSTANT_ID_LENGTH +
OperationSuffix.METADATA_INDEXER.getSuffix().length())) {
- return false;
Review Comment:
Should this be kept?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1613,21 +1595,6 @@ private static int
getFileIdLengthWithoutFileIndex(String fileId) {
return fileId.endsWith("-0") ? fileId.length() - 2 : fileId.length();
}
- /**
- * Create the timestamp for a clean operation on the metadata table.
- */
- public static String createCleanTimestamp(String timestamp) {
- return timestamp + OperationSuffix.CLEAN.getSuffix();
- }
-
- public static String createRollbackTimestamp(String timestamp) {
- return timestamp + OperationSuffix.ROLLBACK.getSuffix();
- }
-
- public static String createRestoreTimestamp(String timestamp) {
- return timestamp + OperationSuffix.RESTORE.getSuffix();
- }
Review Comment:
Since the suffices are no longer used, should those suffix enums (e.g.,
`OperationSuffix.RESTORE`) be removed?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1375,33 +1375,15 @@ public static Set<String>
getValidInstantTimestamps(HoodieTableMetaClient dataMe
/**
* Checks if the Instant is a delta commit and has a valid suffix for
operations on MDT.
*
+ * @param datasetPendingInstants The dataset pending instants
* @param instant {@code HoodieInstant} to check.
* @return {@code true} if the instant is valid.
*/
- public static boolean isValidInstant(HoodieInstant instant) {
- // Should be a deltacommit
- if (!instant.getAction().equals(HoodieTimeline.DELTA_COMMIT_ACTION)) {
- return false;
- }
-
- // Check correct length. The timestamp should have a suffix over the
timeline's timestamp format.
- final String instantTime = instant.getTimestamp();
- if (!(instantTime.length() == MILLIS_INSTANT_ID_LENGTH +
OperationSuffix.METADATA_INDEXER.getSuffix().length())) {
- return false;
- }
-
- // Is this a fixed operations suffix
- final String suffix = instantTime.substring(instantTime.length() - 3);
- if (OperationSuffix.isValidSuffix(suffix)) {
- return true;
- }
-
- // Is this a index init suffix?
- if (suffix.compareTo(String.format("%03d",
PARTITION_INITIALIZATION_TIME_SUFFIX)) >= 0) {
- return true;
- }
Review Comment:
What is this? Should this be kept?
--
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]
