Core: ensure snapshot timestamp is monotonically increasing for V4 tables by stevenzwu · Pull Request #16293 · apache/iceberg

stevenzwu · 2026-05-12T00:09:38Z

Summary

For format version 4 and above, this change makes SnapshotProducer produce snapshot timestamps that are strictly greater than the parent snapshot's timestamp-ms, using a Lamport-clock style fast-forward when the wall clock has drifted backward.

Adds TableMetadata.MIN_FORMAT_VERSION_MONOTONIC_TIMESTAMPS = 4.
In SnapshotProducer.commit(), replaces System.currentTimeMillis() with snapshotTimestampMillis(parentSnapshot). The new helper returns clock.millis() for V1\u2013V3 (and for V4 root snapshots with no parent), and max(clock.millis(), parentSnapshot.timestampMillis() + 1) for V4+ with a parent on the target branch. This preserves wall-clock timestamps in the steady state and only fast-forwards by the minimum amount needed when the clock has drifted backward.
Adds an @VisibleForTesting setClock(Clock) hook so the Lamport behavior can be exercised deterministically.

This is a behavior change for V4 only. V1\u2013V3 commits continue to use the wall clock unchanged.

Why

The format-v4 row-timestamp feature exposes each manifest entry's commit_timestamp_ms as the _last_updated_timestamp_ms metadata column. Time-travel and "latest update" queries on that column become incoherent if a backward jump in the writer's wall clock produces snapshots whose timestamp-ms does not strictly increase with sequence number. Enforcing monotonicity at write time keeps the per-row "last updated" timestamp consistent with snapshot order.

The change is also defensive against clock skew between writers in distributed environments: a stale clock that briefly reports a past time no longer rewrites history.

Test plan

./gradlew :iceberg-core:test --tests org.apache.iceberg.TestSnapshotProducer (29 tests, 6 skipped for pre-V4 by assumeThat, 0 failures)
./gradlew :iceberg-core:spotlessCheck
CI

New tests in TestSnapshotProducer:

testSnapshotTimestampsAreMonotonicallyIncreasing \u2014 three consecutive V4 appends produce strictly increasing timestamp-ms.
testV4LamportClockFastForwardsDriftedClock \u2014 with the producer's Clock pinned 10s in the past, the second V4 snapshot's timestamp-ms equals firstTs + 1, confirming the fast-forward branch is exercised.

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

github-actions · 2026-06-19T00:57:07Z

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

… tables. Made-with: Cursor Model: claude-4.6-opus-high-thinking Co-authored-by: Cursor <cursoragent@cursor.com>

…arget branch Pins that SnapshotProducer.snapshotTimestampMillis uses the parent on the target branch (via SnapshotUtil.latestSnapshot(base, targetBranch)) rather than the table's globally newest snapshot. The test commits to a separate branch with a clock pinned far in the future, then commits to main with a clock pinned just slightly past main's head. Main's new timestamp must equal the pinned wall-clock value (no fast-forward) and remain well below the sibling branch's head timestamp. Without this test, a regression that read base.currentSnapshot() (or any max-over-branches) for the parent would silently pass the existing single-branch monotonicity tests. Made-with: Cursor Model: claude-4.7-opus

huaxingao · 2026-07-17T20:24:38Z

LGTM. Please land this together with / after the spec PR #16294 so v4 enforcement doesn't ship ahead of the ratified spec text.

RussellSpitzer · 2026-07-17T21:59:13Z

+   * @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) {


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.

Specifically this

if (last != null) { Preconditions.checkArgument( // commits can happen concurrently from different machines. // A tolerance helps us avoid failure for small clock skew lastUpdatedMillis - last.timestampMillis() >= -ONE_MINUTE, "Invalid update timestamp %s: before last snapshot log entry at %s", lastUpdatedMillis, last.timestampMillis()); }

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

@RussellSpitzer The quoted snippet compares TableMetadata.lastUpdatedMillis (wall clock) to the snapshot log, not parent→child snapshot timestamp-ms. v4 now validates parent→child in addSnapshot. I dropped the check you quoted rather than tightening it: snapshot-log entries for new snapshots carry Lamport timestamp-ms and are no longer comparable to last-updated-ms.

amogh-jahagirdar · 2026-08-13T18:48:41Z

+    long now = clock.millis();
+    if (base.formatVersion() >= TableMetadata.MIN_FORMAT_VERSION_MONOTONIC_TIMESTAMPS
+        && parentSnapshot != null) {
+      return Math.max(now, parentSnapshot.timestampMillis() + 1);


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?

Hmm do you think we should do this monotonic production for older formats? or just change the validation?

I think it's both. I'm less opinionated on the monotinc production for older formats, it just seems to eliminate the portion of the if condition and is still spec compliant, and produces more "expected" timestamps etc. I think validaiton needs to be updated to make sure that any v4 snapshots are monotonic. So both I think, one makes sure that we produce reaosnable values in the Java implementation, and the other is a read-side validation (though since the comparison is done relative to the new v4 snapshot which we guarnateed that its monotonic, maybe it doesn't matter here either).

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?

I am good with this. The reference implementation can change to monotonic timestamp for older format versions. we just don't validate it.

Done in the latest commit: Java always stamps max(now, parent+1). Spec validation remains v4-only in addSnapshot.

amogh-jahagirdar · 2026-08-13T19:27:53Z

      executor.shutdownNow();
    }
  }
+


Maybe some transaction tests which produce multipel snapshots?

Added transactionCommitsProduceMonotonicTimestamps and transactionRetryFastForwardsTimestampsPastConflictingSnapshot (retry after a conflicting snapshot whose timestamp is ahead of the txn's stale clock).

Always fast-forward snapshot timestamp-ms past the parent so Java writers stay monotonic going forward. Validate parent-to-child timestamps only when adding v4 snapshots, and stop comparing last-updated-ms to snapshot-log times that can now be logically ahead of wall clock. Generated-by: Cursor Grok 4.6 Co-authored-by: Cursor <cursoragent@cursor.com>

v4 addSnapshot now requires timestamp-ms strictly after the parent, so these fixtures cannot reuse 0 for both snapshots. Generated-by: Cursor Grok 4.6 Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu · 2026-09-28T22:53:45Z

@@ -378,15 +379,6 @@ public String toString() {
      }
      last = logEntry;
    }


Dropped the lastUpdatedMillis vs last snapshot-log ONE_MINUTE check that used to sit here.

These are different clocks: TableMetadata.lastUpdatedMillis is physical wall clock, while snapshot timestamp-ms is a monotonic Lamport clock (max(now, parent+1)) copied into the snapshot log for new snapshots. A Lamport fast-forward can put the snapshot log arbitrarily ahead of wall clock, so comparing them would reject a valid commit.

The metadata-log checks remain unchanged; both sides are wall-clock lastUpdatedMillis (L383-L402).

stevenzwu · 2026-09-28T22:53:52Z

+   * @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) {


@RussellSpitzer The quoted snippet compares TableMetadata.lastUpdatedMillis (wall clock) to the snapshot log, not parent→child snapshot timestamp-ms. v4 now validates parent→child in addSnapshot. I dropped the check you quoted rather than tightening it: snapshot-log entries for new snapshots carry Lamport timestamp-ms and are no longer comparable to last-updated-ms.

stevenzwu · 2026-09-28T22:53:53Z

+    long now = clock.millis();
+    if (base.formatVersion() >= TableMetadata.MIN_FORMAT_VERSION_MONOTONIC_TIMESTAMPS
+        && parentSnapshot != null) {
+      return Math.max(now, parentSnapshot.timestampMillis() + 1);


Done in the latest commit: Java always stamps max(now, parent+1). Spec validation remains v4-only in addSnapshot.

stevenzwu · 2026-09-28T22:53:53Z

      executor.shutdownNow();
    }
  }
+


Added transactionCommitsProduceMonotonicTimestamps and transactionRetryFastForwardsTimestampsPastConflictingSnapshot (retry after a conflicting snapshot whose timestamp is ahead of the txn's stale clock).

RussellSpitzer · 2026-10-07T18:27:57Z

  }
+
+  @Test
+  public void addSnapshotRejectsNonMonotonicTimestampForV4() {


I would use the "ALL_VERSIONS" test helper with "Assume Version > 4" rather than hardcoding the 4 in here

Used TestHelpers.V4_AND_ABOVE instead of assumeThat.

RussellSpitzer · 2026-10-07T18:28:12Z

+  }
+
+  @Test
+  public void snapshotTimestampMayBeAheadOfLastUpdatedMillis() {


All versions with Version < 4

This behavior is not limited to versions below 4, so the test runs on ALL_VERSIONS. It is renamed to snapshotTimestampMayBeAheadOfTableMetadataLastUpdatedMillis to be more clear on the scenario

RussellSpitzer · 2026-10-07T18:28:44Z

+  }
+
+  @Test
+  public void addSnapshotAllowsNonMonotonicTimestampBeforeV4() {


All versions with Versions <4

Parameterized on TestHelpers.V3_AND_BELOW.

v1 still uses sequence number 0, and v3 sets row-lineage fields, since those versions do not share one snapshot shape.

RussellSpitzer · 2026-10-07T18:29:07Z

+  public void addSnapshotAllowsNonMonotonicTimestampBeforeV4() {
+    TableMetadata base =
+        TableMetadata.newTableMetadata(
+            TEST_SCHEMA, PartitionSpec.unpartitioned(), "location", ImmutableMap.of());


Probably need to explicitly pass through version here

format-version is now passed in the properties map for each version in V3_AND_BELOW.

Parameterize metadata checks by version lists and cover transaction commits and retry clocks more realistically. Generated-by: Cursor Grok 4.7 Co-authored-by: Cursor <cursoragent@cursor.com>

RussellSpitzer · 2026-10-07T21:13:30Z

Merged. Thanks @stevenzwu for the pr and @huaxingao + @danielcweeks for reviewing.

Since this is a good idea and isn't actually tied to the spec change I think it's fine to merge now before the vote is finished although that looks like it's going through easily as well :)

stevenzwu · 2026-10-07T21:47:42Z

thanks @RussellSpitzer @amogh-jahagirdar @huaxingao for the review

amogh-jahagirdar

Late +1 from me just a minor comment on the error message. THank you @stevenzwu !

amogh-jahagirdar · 2026-10-08T14:10:01Z

+        if (parent != null) {
+          ValidationException.check(
+              snapshot.timestampMillis() > parent.timestampMillis(),
+              "Invalid snapshot timestamp %s: not after parent snapshot %s at %s",


Sorry for the delayed review, maybe in a follow on we can include the snapshot ID in the error message, maybe something like

"Snapshot %s must have a later timestamp than parent snapshot %s: %s <= %s"

parent snapshot id was already included in the error msg. the conflicting committing/child snapshot id wasn't really useful, as it wasn't committed. we can't inspect it from table history.

…bles (apache#16293)

github-actions Bot added the core label May 12, 2026

stevenzwu mentioned this pull request May 12, 2026

Spec: require monotonically increasing snapshot timestamp-ms in v4 #16294

Merged

1 of 2 tasks

amogh-jahagirdar self-requested a review May 12, 2026 15:42

github-actions Bot added the stale label Jun 12, 2026

github-actions Bot closed this Jun 19, 2026

stevenzwu reopened this Jun 30, 2026

stevenzwu added not-stale and removed stale labels Jun 30, 2026

stevenzwu and others added 2 commits July 1, 2026 00:00

Core: make sure snapshot timestamp is monotonically increasing for V4…

29970b9

… tables. Made-with: Cursor Model: claude-4.6-opus-high-thinking Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu force-pushed the snapshot-timestamp-monotonic branch from 8c58bd2 to 9d3744e Compare July 1, 2026 07:01

danielcweeks self-requested a review July 1, 2026 20:55

RussellSpitzer added the Iceberg V4 Iceberg Table Format Version 4 label Jul 16, 2026

huaxingao approved these changes Jul 17, 2026

View reviewed changes

RussellSpitzer reviewed Jul 17, 2026

View reviewed changes

stevenzwu mentioned this pull request Aug 12, 2026

Monotonic snapshot timestamp and expose a new metadata column _last_updated_timestamp_ms #17619

Open

1 of 6 tasks

amogh-jahagirdar reviewed Aug 13, 2026

View reviewed changes

stevenzwu and others added 2 commits September 28, 2026 15:41

Core: give v4 row-lineage test snapshots increasing timestamps

c454906

v4 addSnapshot now requires timestamp-ms strictly after the parent, so these fixtures cannot reuse 0 for both snapshots. Generated-by: Cursor Grok 4.6 Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu commented Sep 28, 2026

View reviewed changes

RussellSpitzer reviewed Oct 7, 2026

View reviewed changes

github-actions Bot added the API label Oct 7, 2026

stevenzwu force-pushed the snapshot-timestamp-monotonic branch from db401a2 to 809f149 Compare October 7, 2026 19:45

Core: align monotonic timestamp tests with format versions

c12134e

Parameterize metadata checks by version lists and cover transaction commits and retry clocks more realistically. Generated-by: Cursor Grok 4.7 Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu force-pushed the snapshot-timestamp-monotonic branch from 809f149 to c12134e Compare October 7, 2026 20:15

RussellSpitzer approved these changes Oct 7, 2026

View reviewed changes

RussellSpitzer merged commit 42034ff into apache:main Oct 7, 2026
41 checks passed

stevenzwu deleted the snapshot-timestamp-monotonic branch October 7, 2026 21:46

amogh-jahagirdar approved these changes Oct 8, 2026

View reviewed changes

danielcweeks pushed a commit to danielcweeks/iceberg that referenced this pull request Oct 19, 2026

Core: ensure snapshot timestamp is monotonically increasing for V4 ta…

2a5544d

…bles (apache#16293)

Conversation

stevenzwu commented May 12, 2026

Summary

Why

Test plan

Related

Uh oh!

github-actions Bot commented Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

huaxingao commented Jul 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer commented Oct 7, 2026

Uh oh!

Uh oh!

stevenzwu commented Oct 7, 2026

Uh oh!

amogh-jahagirdar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenzwu Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

stevenzwu Oct 8, 2026 •

edited

Loading