voonhous commented on code in PR #18965:
URL: https://github.com/apache/hudi/pull/18965#discussion_r3407644286
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java:
##########
@@ -649,14 +650,39 @@ public static Stream<HoodieRecord>
createPartitionStatsRecords(String partitionP
*/
public static HoodieRecord<HoodieMetadataPayload>
createRecordIndexUpdate(String recordKey, String partition,
String fileId, String instantTime, int fileIdEncoding) {
+ return createRecordIndexUpdate(recordKey, partition, fileId,
parseRecordIndexInstantTime(instantTime), fileIdEncoding);
+ }
- HoodieKey key = new HoodieKey(recordKey,
MetadataPartitionType.RECORD_INDEX.getPartitionPath());
- long instantTimeMillis = -1;
+ /**
+ * Parses an instant time into the epoch millis stored in record index
entries.
+ * <p>
+ * Callers creating record index updates for many records of the same commit
should parse the
+ * instant time once with this method and use the millis-based
+ * {@link #createRecordIndexUpdate(String, String, String, long, int)}
overload per record.
+ */
+ public static long parseRecordIndexInstantTime(String instantTime) {
try {
- instantTimeMillis =
TimelineUtils.parseDateFromInstantTime(instantTime).getTime();
+ return TimelineUtils.parseDateFromInstantTime(instantTime).getTime();
} catch (Exception e) {
throw new HoodieMetadataException("Failed to create metadata payload for
record index. Instant time parsing for " + instantTime + " failed ", e);
}
+ }
+
+ /**
+ * Create and return a {@code HoodieMetadataPayload} to insert or update an
entry for the record index.
+ * <p>
+ * Same as {@link #createRecordIndexUpdate(String, String, String, String,
int)} but takes the
+ * instant time already parsed to epoch millis, so per-commit callers parse
it only once.
+ *
+ * @param recordKey Key of the record
+ * @param partition Name of the partition which contains the record
+ * @param fileId fileId which contains the record
+ * @param instantTimeMillis epoch millis of the instant when the record was
added
Review Comment:
Done - expanded the millis-overload Javadoc to document both: (1)
`instantTimeMillis` must be obtained from `parseRecordIndexInstantTime(String)`
(which delegates to `TimelineUtils.parseDateFromInstantTime`) and not
hand-constructed, so the stored value matches what the String overload writes;
(2) the instant-time string is interpreted in the JVM default time zone
(`ZoneId.systemDefault()`) and the result is epoch milliseconds (since
1970-01-01T00:00:00Z), matching the implementation. Also annotated `@param
instantTimeMillis` to point at `parseRecordIndexInstantTime`.
--
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]
