nsivabalan commented on a change in pull request #3824:
URL: https://github.com/apache/hudi/pull/3824#discussion_r732399635
##########
File path: hudi-flink/src/main/java/org/apache/hudi/util/StreamerUtil.java
##########
@@ -423,8 +423,8 @@ public static String medianInstantTime(String highVal,
String lowVal) {
*/
public static long instantTimeDiffSeconds(String newInstantTime, String
oldInstantTime) {
try {
- long newTimestamp =
HoodieActiveTimeline.COMMIT_FORMATTER.parse(newInstantTime).getTime();
- long oldTimestamp =
HoodieActiveTimeline.COMMIT_FORMATTER.parse(oldInstantTime).getTime();
+ long newTimestamp =
HoodieActiveTimeline.parseDateFromInstantTime(newInstantTime).getTime();
Review comment:
looks like this method is finding diff in secs. Do we wanna fix L 428 ?
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java
##########
@@ -59,7 +60,16 @@
*/
public class HoodieActiveTimeline extends HoodieDefaultTimeline {
- public static final SimpleDateFormat COMMIT_FORMATTER = new
SimpleDateFormat("yyyyMMddHHmmss");
+ private static final String COMMIT_FORMAT = "yyyyMMddHHmmssSSS";
+ private static final int INSTANT_ID_LENGTH = COMMIT_FORMAT.length();
Review comment:
if this is used only locally, can we name this MILLIS_INSTANT_ID_LENGTH?
whenever I bump into the usages of this, I get confused whether this is
referring to seconds or ms.
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java
##########
@@ -74,6 +84,43 @@
protected HoodieTableMetaClient metaClient;
private static AtomicReference<String> lastInstantTime = new
AtomicReference<>(String.valueOf(Integer.MIN_VALUE));
+ /**
+ * Parses the given instant ID to return a date instance.
+ * @param instant The instant ID
+ * @return A date
+ * @throws ParseException If the instant ID is malformed
+ */
+ public static Date parseDateFromInstantTime(String instant) throws
ParseException {
+ // Enables backwards compatibility with non-millisecond granularity
instants
+ if (isMillisecondGranularity(instant)) {
+ return COMMIT_FORMATTER.parse(instant);
+ } else {
+ // Add milliseconds to the instant in order to parse successfully
+ return COMMIT_FORMATTER.parse(instant + "000");
Review comment:
shouldn't we be suffixing DEFAULT_MILLIS_EXT instead of 000
--
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: commits-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
