TengHuo commented on code in PR #6000:
URL: https://github.com/apache/hudi/pull/6000#discussion_r951084633
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java:
##########
@@ -80,6 +80,15 @@ public class HoodieActiveTimeline extends
HoodieDefaultTimeline {
/**
* Parse the timestamp of an Instant and return a {@code Date}.
+ * Throw ParseException if timestamp not valid format as
+ * {@link
org.apache.hudi.common.table.timeline.HoodieInstantTimeGenerator#SECS_INSTANT_TIMESTAMP_FORMAT}.
+ * This method will mute ParseException if gaven these timestamp and return
a corresponding Date
+ * {@link
org.apache.hudi.common.table.timeline.HoodieTimeline#INIT_INSTANT_TS},
Review Comment:
yeah
TBH, I just realised we should fix this issue in the code
[`metrics.updateCommitMetrics(HoodieActiveTimeline.parseDateFromInstantTime(instantTime).getTime(),
....)`](https://github.com/apache/hudi/blob/98f01666c740244a23f883ed4a754448fbea1d5b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java#L314)
When it updates a metrics value, it should ignore timestamp like
"00000000000000" or "00000000000001" etc because it won't cause any bad result
except the missing metrics value. The method `parseDateFromInstantTime` should
always throw `ParseException` when there is an illegal timestamp
@danny0405 what do you think? it can fix the issue we mentioned in #3774 as
well?
--
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]
