amogh-jahagirdar commented on code in PR #16293:
URL: https://github.com/apache/iceberg/pull/16293#discussion_r3778200655
##########
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) {
+ long now = clock.millis();
+ if (base.formatVersion() >=
TableMetadata.MIN_FORMAT_VERSION_MONOTONIC_TIMESTAMPS
+ && parentSnapshot != null) {
+ return Math.max(now, parentSnapshot.timestampMillis() + 1);
Review Comment:
Any reason we can't just produce the monotonic timestamp for older format
versions as well? Sure, it's not a spec level guarantee for those and we
shouldn't validate on read that it's monotonic for older format versions, but
from an implementation perspective I think we can just do it going forward?
--
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]
