Skip to content

tests: Extend to run into OCC timestamp failure - #38341

Open
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:pr-tests-occ
Open

tests: Extend to run into OCC timestamp failure#38341
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:pr-tests-occ

Conversation

@def-

@def- def- commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #38322 so currently based on top of it. Otherwise all tests would be failing in CI

Test run: https://buildkite.com/materialize/nightly/builds/18125 & https://buildkite.com/materialize/nightly/builds/18126
2nd test run after rebase: https://buildkite.com/materialize/nightly/builds/18150

Feel free to rebase and merge this in my absence, only the last commit is required. Nightly was green, modulo current GitHub shenanigans.

@def-
def- requested a review from aljoscha August 19, 2026 09:54
@def-
def- requested review from a team as code owners August 19, 2026 09:54
@def-
def- force-pushed the pr-tests-occ branch 2 times, most recently from e816afb to d4a9ebd Compare August 19, 2026 10:09
@def-

def- commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

QA LLM Review

1. MEDIUM -- test_occ_zero_row_write_linearization can no longer fail

test/cluster/mzcompose.py:4915

Under the new timestamp rule the UPDATE's write attempt always queues behind the parked
committer, so it cannot reach its zero-row conclusion until after the winning DELETE has
been applied to the oracle. after == 0 is then guaranteed by the winner's own
apply_write, not by the zero-row response being linearized, and the workflow would pass
unchanged if ensure_read_linearized were dropped from the NoRowsMatched arm.

Details

Walk the window with W = the winner's write timestamp:

  • The winner's DELETE is a read-dependent read-then-write on an autocommit connection
    (Composition.sql_connection sets autocommit = True), so it goes through
    commit_timestamped, which never calls oracle.write_ts(). While it sleeps at
    group_commit_before_apply_write (src/adapter/src/coord/appends.rs:626), the oracle's
    write timestamp is still W - 1.
  • The UPDATE therefore picks target = peek_write_ts + 1 = W
    (src/adapter/src/frontend_read_then_write.rs:1518). The delete's diffs sit at exactly
    W, so fold_below(W) leaves them pending and the payload is the non-empty snapshot:
    the loop submits. This is what the new docstring describes, and it is what removes
    conflicts > 0 from the disqualifier at line 4909.
  • handle_attempt_write forwards to group_committer_tx
    (src/adapter/src/coord/read_then_write.rs:192) and the committer is a single task
    blocked in the winner's sleep. The request cannot be answered until the sleep ends, at
    which point apply_write(W) has already run and the oracle's read timestamp is W.
  • Only then does the committer report TimestampPassed { next_eligible: W + 1 }, the loop
    folds the delete in, and the payload consolidates to empty.

A strict-serializable read at read timestamp W includes the delete committed at W, so
after = count(session, key) is 0 whether or not the loop waited for W + 1.

Before this change the UPDATE concluded zero rows at the observed frontier without
submitting anything, while the winner was still parked and the oracle was at W - 1; a
read there still saw the row, which is what made the assertion bite. The Rust test
test_zero_row_write_does_not_wait_for_keepalive only measures the nudge, so this
workflow is still the sole coverage for the linearization, as its own docstring says.

To restore it the UPDATE has to reach the zero-row conclusion without needing the
committer. That happens when its first target already clears W, i.e. when the winner is
a write that allocates from the oracle before appending (write_to_txns) rather than a
timestamped OCC write, since then peek_write_ts == W during the park and the loop folds
the delete in on its first attempt. Alternatively, assert on a witness that the winner's
own apply_write cannot satisfy, for example that the oracle's read timestamp observed
right after the UPDATE returns is above W.

Follow-up to MaterializeInc#38322,
so currently based on top of it
@def-

def- commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

QA LLM Review

Not related to the remaining changes

@def-

def- commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

@aljoscha This is now ready for review since your PR merged.

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.

1 participant