nsivabalan commented on code in PR #7740:
URL: https://github.com/apache/hudi/pull/7740#discussion_r1085740528
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1040,7 +1043,13 @@ protected void compactIfNecessary(BaseHoodieWriteClient
writeClient, String inst
// Trigger 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 compactionInstantTime = latestDeltaCommitTimeInMetadataTable
+ METADATA_COMPACTION_TIME_SUFFIX;
+ if (latestDeltaCommitTimeInMetadataTable.length() >
MILLIS_INSTANT_TIMESTAMP_FORMAT_LENGTH) {
+
checkState(latestDeltaCommitTimeInMetadataTable.endsWith(METADATA_TABLE_INIT_TIME_SUFFIX),
Review Comment:
I would prefer to keep it simple. if latest delta commit refers to an
instantiation dc, we can ignore triggering compaction only.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1119,7 +1128,8 @@ private void initialCommit(String createInstantTime,
List<MetadataPartitionType>
LOG.info("Committing " + partitions.size() + " partitions and " +
totalDataFilesCount + " files to metadata");
- commit(createInstantTime, partitionToRecordsMap, false);
+ commit(createInstantTime.equals(SOLO_COMMIT_TIMESTAMP) ? createInstantTime
: createInstantTime + METADATA_TABLE_INIT_TIME_SUFFIX,
Review Comment:
can we write a functional test.
start hudi table w/ default. after 2 commits, enable col stats. after 1
commit succeeds, confirm metadata table timeline is as expected. we have a
delta commit for every commit in data table. and we have additional dc w/ "000"
suffix. we can even check the commit metadata for this dc to ensure it has data
only for col_stats partition and not for "files" partition.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1040,7 +1043,13 @@ protected void compactIfNecessary(BaseHoodieWriteClient
writeClient, String inst
// Trigger 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 compactionInstantTime = latestDeltaCommitTimeInMetadataTable
+ METADATA_COMPACTION_TIME_SUFFIX;
+ if (latestDeltaCommitTimeInMetadataTable.length() >
MILLIS_INSTANT_TIMESTAMP_FORMAT_LENGTH) {
+
checkState(latestDeltaCommitTimeInMetadataTable.endsWith(METADATA_TABLE_INIT_TIME_SUFFIX),
Review Comment:
@codope : any existing table should work w/o any issues. but does not hurt
to test it out. @xushiyan : can you test out this scenario once to confirm we
are good.
--
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]
