Skip to content

[core] Fix snapshot lookup for duplicate commit timestamps - #9437

Open
ArnavBalyan wants to merge 1 commit into
apache:masterfrom
ArnavBalyan:arnavb/fix-snapshot-time
Open

[core] Fix snapshot lookup for duplicate commit timestamps#9437
ArnavBalyan wants to merge 1 commit into
apache:masterfrom
ArnavBalyan:arnavb/fix-snapshot-time

Conversation

@ArnavBalyan

Copy link
Copy Markdown
Member

Purpose

  • Multiple snapshots can have the same millisecond commit timestamp.
  • Timestamp lookup stops at an arbitrary snapshot, causing time travel and rollback operations to select the wrong snapshot when multiple snapshots share the same timestamp.
  • Continue searching equal timestamps to return the newest snapshot for earlier/equal lookup and the earliest snapshot for later/equal lookup.

Tests

  • UT

finalSnapshot = snapshot; // Found the exact match
break;
finalSnapshot = snapshot;
earliest = mid + 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh the upper bound before searching past an exact match

latest is only an ID captured before the search. A concurrent rollback can update the latest hint and delete that snapshot after latestSnapshotId() returns. With snapshots 0/1/2 sharing this timestamp, if rollback removes snapshot 2 after we capture it, the first probe hits snapshot 1; this new continuation then probes snapshot 2 and throws Snapshot file ... does not exist, although snapshot 1 is still the correct answer. The old break returned snapshot 1.

Please resolve the upper boundary through the existing live-snapshot path (for example, latestSnapshot() plus an early boundary return) and add a rollback-race regression test.

finalSnapshot = snapshot; // Found the exact match
break;
finalSnapshot = snapshot;
latest = mid - 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh the lower bound before searching past an exact match

earliest can become stale when expiration runs between earliestSnapshotId() and the binary search. With snapshots 0/1/2 sharing this timestamp, expiration can delete snapshot 0 after the ID is read; the search first hits snapshot 1, then this new continuation probes snapshot 0 and fails even though snapshot 1 is the earliest surviving match. The old break returned snapshot 1.

Please obtain the lower bound through earliestSnapshot(latest), which already retries concurrent earliest deletion, return it directly when it satisfies the query, and cover this race in the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants