xushiyan commented on a change in pull request #4024:
URL: https://github.com/apache/hudi/pull/4024#discussion_r752823452
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java
##########
@@ -33,14 +35,27 @@
*/
public class HoodieInstantTimeGenerator {
// Format of the timestamp used for an Instant
- private static final String INSTANT_TIMESTAMP_FORMAT = "yyyyMMddHHmmss";
- private static final int INSTANT_TIMESTAMP_FORMAT_LENGTH =
INSTANT_TIMESTAMP_FORMAT.length();
+ public static final String SECS_INSTANT_TIMESTAMP_FORMAT = "yyyyMMddHHmmss";
+ public static final int SECS_INSTANT_ID_LENGTH =
SECS_INSTANT_TIMESTAMP_FORMAT.length();
+ public static final String MILLIS_INSTANT_TIMESTAMP_FORMAT =
"yyyyMMddHHmmssSSS";
+ public static final int MILLIS_INSTANT_ID_LENGTH =
MILLIS_INSTANT_TIMESTAMP_FORMAT.length();
+ public static final int MILLIS_INSTANT_TIMESTAMP_FORMAT_LENGTH =
MILLIS_INSTANT_TIMESTAMP_FORMAT.length();
// Formatter to generate Instant timestamps
- private static DateTimeFormatter INSTANT_TIME_FORMATTER =
DateTimeFormatter.ofPattern(INSTANT_TIMESTAMP_FORMAT);
+ // Unfortunately millisecond format is not parsable as is
https://bugs.openjdk.java.net/browse/JDK-8031085. hence have to do appendValue()
+ private static DateTimeFormatter MILLIS_INSTANT_TIME_FORMATTER = new
DateTimeFormatterBuilder().appendPattern(SECS_INSTANT_TIMESTAMP_FORMAT)
+ .appendValue(ChronoField.MILLI_OF_SECOND, 3).toFormatter();
+ private static final String MILLIS_GRANULARITY_DATE_FORMAT = "yyyy-MM-dd
HH:mm:ss:SSS";
Review comment:
usually sec and millis were separated by `.` instead of `:`
`yyyy-MM-dd HH:mm:ss.SSS` is a more common format
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -431,6 +431,12 @@
.sinceVersion("0.10.0")
.withDocumentation("File Id Prefix provider class, that implements
`org.apache.hudi.fileid.FileIdPrefixProvider`");
+ public static final ConfigProperty<Boolean>
ENABLE_MILLIS_COMMIT_TIMESTAMP_FORMAT = ConfigProperty
+ .key("hoodie.enable.millisecs.commit.timestamp.format")
+ .defaultValue(true)
+ .sinceVersion("0.10.0")
+ .withDocumentation("Enables millisecs granularity for commit times.
Falls back to secs level granularity if disabled.");
Review comment:
still need this?
##########
File path:
hudi-common/src/test/java/org/apache/hudi/common/table/timeline/TestHoodieActiveTimeline.java
##########
@@ -476,16 +476,37 @@ public void testCreateNewInstantTime() throws Exception {
@Test
public void testMetadataCompactionInstantDateParsing() throws ParseException
{
// default second granularity instant ID
- String secondGranularityInstant = "20210101120101";
- Date defaultSecsGranularityDate =
HoodieActiveTimeline.parseInstantTime(secondGranularityInstant);
+ String secondGranularityInstant = "20210101120101123";
+ Date defaultSecsGranularityDate =
HoodieActiveTimeline.parseDateFromInstantTime(secondGranularityInstant);
Review comment:
```suggestion
String msGranularityInstant = "20210101120101123";
Date defaultMsGranularityDate =
HoodieActiveTimeline.parseDateFromInstantTime(msGranularityInstant);
```
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java
##########
@@ -52,36 +67,65 @@ public static String createNewInstantTime(long
milliseconds) {
String newCommitTime;
do {
Date d = new Date(System.currentTimeMillis() + milliseconds);
- newCommitTime =
INSTANT_TIME_FORMATTER.format(convertDateToTemporalAccessor(d));
+ newCommitTime =
MILLIS_INSTANT_TIME_FORMATTER.format(convertDateToTemporalAccessor(d));
} while (HoodieTimeline.compareTimestamps(newCommitTime,
HoodieActiveTimeline.LESSER_THAN_OR_EQUALS, oldVal));
return newCommitTime;
});
}
- public static Date parseInstantTime(String timestamp) throws ParseException {
+ public static Date parseDateFromInstantTime(String timestamp) throws
ParseException {
try {
- LocalDateTime dt = LocalDateTime.parse(timestamp,
INSTANT_TIME_FORMATTER);
+ // Enables backwards compatibility with non-millisecond granularity
instants
+ String timestampInMillis = timestamp;
+ if (isSecondGranularity(timestamp)) {
+ // Add milliseconds to the instant in order to parse successfully
+ timestampInMillis = timestamp + DEFAULT_MILLIS_EXT;
+ } else if (timestamp.length() > MILLIS_INSTANT_TIMESTAMP_FORMAT_LENGTH) {
+ // compaction and cleaning in metadata has special format. handling it
by trimming extra chars and treating it with secs granularity
Review comment:
```suggestion
// compaction and cleaning in metadata has special format. handling
it by trimming extra chars and treating it with millis granularity
```
--
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]
