amogh-jahagirdar commented on code in PR #16293:
URL: https://github.com/apache/iceberg/pull/16293#discussion_r3778351578
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -683,6 +685,31 @@ protected ManifestReader<DeleteFile>
newDeleteManifestReader(ManifestFile manife
return ManifestFiles.readDeleteManifest(manifest, ops.io(),
ops.current().specsById());
}
+ @VisibleForTesting
+ void setClock(Clock newClock) {
+ this.clock = newClock;
+ }
+
+ /**
+ * Generates the snapshot timestamp in milliseconds.
+ *
+ * <p>For format version 4 and above, this implements the Lamport clock
algorithm to guarantee
+ * monotonically increasing snapshot timestamps. For older format versions,
this returns the
+ * current wall clock time.
+ *
+ * @param parentSnapshot the parent snapshot on the target branch, or null
if there is no parent
+ * @return the snapshot timestamp in milliseconds
+ */
+ private long snapshotTimestampMillis(Snapshot parentSnapshot) {
Review Comment:
I think I somewhat agreee with this. I think the enforcement validation
should change but I think we should make sure that comparison is done with
lastUpdatedMillis reflecting the actually assigned snapshotTimestampMillis. In
essence if parent snapshot was committed with 12:00:00.000, and the current
clock is running slow and is at 11:59:57 , and the new snapshot is set to
12:00:01 based on the current logic, we should make sure this comparison is
actually done against 12:00:01
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
