codope commented on code in PR #8915:
URL: https://github.com/apache/hudi/pull/8915#discussion_r1225067003
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1361,7 +1363,13 @@ public static Set<String>
getValidInstantTimestamps(HoodieTableMetaClient dataMe
});
// SOLO_COMMIT_TIMESTAMP is used during bootstrap so it is a valid
timestamp
- validInstantTimestamps.add(createIndexInitTimestamp(SOLO_COMMIT_TIMESTAMP,
PARTITION_INITIALIZATION_TIME_SUFFIX));
+ List<String> metadataInitializationTimestamps =
metadataMetaClient.getActiveTimeline()
Review Comment:
@danny0405 @nsivabalan Agree with your points but should a util method be
aware of different cases? Let's say tomorrow for a new MDT partition, the
semantics change and it writes a data block with the initializing commit
itself, then the author/reviewer needs to come back to util method and fix it.
This is going to be harder to maintain. IMO, the better way to handle such
cases is by keeping the util method dumb, and do any case handling at the call
site or have assertions for invariants such as data block can never have
initializing commit time. Wdyt?
Btw, the change is backward compatible as it checks for
`startsWith(SOLO_COMMIT_TIMESTAMP)`. Also, there is no reloading of the
timeline but to avoid going through active timeline again, i can merge the
filter with the stream operation at lines 1349-1350.
--
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]
