Skip to content

storage-controller: respond to table appends before txn-wal apply#37443

Merged
def- merged 2 commits into
MaterializeInc:mainfrom
def-:txn-wal-deferred-apply
Jul 13, 2026
Merged

storage-controller: respond to table appends before txn-wal apply#37443
def- merged 2 commits into
MaterializeInc:mainfrom
def-:txn-wal-deferred-apply

Conversation

@def-

@def- def- commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

The commit to the txns shard already makes a table write durable and linearized, so send the append response then and run the idempotent apply and compaction afterwards, off the caller's critical path. Reads of the just-written shards block until the apply happens, and a crash before applying is covered by the next append's apply_le.

NAME                                | TYPE            |      THIS       |      OTHER      |  UNIT  | THRESHOLD  |  Regression?  | 'THIS' is
--------------------------------------------------------------------------------------------------------------------------------------------------------
MultiTableTransactionCommit         | wallclock       |           0.038 |           0.098 |   s    |    10%     |      no       | better:  2.6 times faster

(with 1000 tables)

The commit to the txns shard already makes a table write durable and
linearized, so send the append response then and run the idempotent
apply and compaction afterwards, off the caller's critical path. Reads
of the just-written shards block until the apply happens, and a crash
before applying is covered by the next append's apply_le.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@def- def- force-pushed the txn-wal-deferred-apply branch from 63be862 to 9d14786 Compare July 5, 2026 11:42
@def- def- requested a review from DAlperin July 5, 2026 13:39
@def- def- marked this pull request as ready for review July 5, 2026 13:39
@def- def- requested a review from a team as a code owner July 5, 2026 13:39
@def-

def- commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

ready for review

@DAlperin DAlperin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. One thought is that this changes the semantics of the append_table_duration_seconds. We lose the coverage of actually applying. Probably fine, but might be worth a new metric.

@def- def- force-pushed the txn-wal-deferred-apply branch from 709d9d8 to ccb9bde Compare July 10, 2026 07:57
@def- def- requested a review from a team as a code owner July 10, 2026 07:57
@def- def- requested a review from DAlperin July 10, 2026 07:57
@def-

def- commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@DAlperin I added a commit with new metric. I'm not sure if this is too heavy-duty though. What's your opinion?

@def- def- merged commit 6c590b0 into MaterializeInc:main Jul 13, 2026
121 checks passed
@def- def- deleted the txn-wal-deferred-apply branch July 13, 2026 07:55
@DAlperin

Copy link
Copy Markdown
Member

@def- (sorry forgot to hit enter) I think it's fine. Thank you!

def- added a commit that referenced this pull request Jul 14, 2026
…ply (#37443) (#37631)

This reverts commit 6c590b0.

I tried to fix performance regression the change introduced in other
benchmarks in #37627,
but failed.

This feels safer. It's too bad we lose the ~2.5x improvement the PR had
though.

Nightly perf. run shows us that we do lose some performance from
reverting: https://buildkite.com/materialize/nightly/builds/17318
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