RussellSpitzer commented on code in PR #16293:
URL: https://github.com/apache/iceberg/pull/16293#discussion_r3606400846


##########
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 feel a little weird that we have the algorithm here, but the validation in 
the commit still allows for backwards drift. I would feel a little bit better 
if we changed the enforcement validation there as well if the format version > 
4. 



-- 
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]

Reply via email to