Repository navigation
Route a QuickWithServerTimestamps StartStream through mt_quick_append_events (#5454) - #5456
Merged
jeremydmiller merged 1 commit intoSep 21, 2026
Conversation
…_events (JasperFx#5454) An append to an existing stream goes through mt_quick_append_events, which returns the sequence numbers it drew; QuickAppendEventsOperationBase.PostprocessAsync copies them onto the events "only to aid in tombstone processing", so a rollback is tombstoned and the high-water detector sees no gap. A StartStream took the per-event InsertStream + QuickAppendEventWithVersion route instead, where seq_id is an inlined nextval() that nothing reads back: the events keep Sequence 0, and TryCreateTombstoneBatch skips exactly those. A failure later in the same batch — a losing UpdateRevision, a duplicate document INSERT — therefore left sequence numbers that nothing fills. This routes such a Start through the same function, which is what UseTenantPartitionedEvents already does for every shape, so the existing tombstone path covers it. The function checks the version before any nextval(), so a colliding id still advances no sequence; MT003 on a Start is mapped back to ExistingStreamIdCollisionException for the non-partitioned path, leaving the partitioned divergence that StartStream_duplicate_id_within_tenant_collides pins on purpose. The per-event route stays for AppendMode.Quick (the function stamps now() in that mode and takes no timestamps array, which override_timestamp_on_start(mode: Quick) pins), UseArchivedStreamPartitioning and EnableStrictStreamIdentityEnforcement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller
added a commit
that referenced
this pull request
Sep 21, 2026
Follow-up to #5456. For a brand-new stream the function inserted the mt_streams row with version 0 and then let the unconditional trailing UPDATE set the final version -- two heap tuples and two WAL records per stream creation, in one transaction, where nothing outside it can observe the intermediate 0. The insert now carries the final version and the UPDATE is skipped for a new stream. This got measurably more expensive with #5456, which routes every non-partitioned QuickWithServerTimestamps StartStream -- the V9 default append mode -- through this function. Measured on PostgreSQL 17.10: a 1-event StartStream went 471us (old per-event route) -> 511us (#5456) -> 480us with this change. The tenant-partitioned path and FetchForWriting against a missing stream get the same saving.
This was referenced Sep 22, 2026
Closed
This was referenced Oct 1, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5454. Taking you up on "if you think you can do better, please feel free to try" — and you're right that it isn't common usage: exactly one door in our application reaches it.
What it is. An append to an existing stream already goes through
mt_quick_append_events, which returns the sequence numbers it drew, andQuickAppendEventsOperationBase.PostprocessAsynccopies them onto the events "only to aid in tombstone processing" — so a rollback gets tombstoned and the high-water detector sees no gap. AStartStreamtook the per-eventInsertStream+QuickAppendEventWithVersionroute, whereseq_idis an inlinednextval()that nothing reads back. Those events keepSequence0, andTryCreateTombstoneBatchskips exactly those, so a failure later in the same batch — a losingUpdateRevision, a duplicate document INSERT — leaves numbers nothing fills.This routes such a Start through the same function, which is what
UseTenantPartitionedEventsalready does for every shape. Your suggestion to "retry the order of commands" is what it amounts to: the function's version check runs before anynextval().Deliberately narrow. The per-event route stays for three cases, each with the reason at the branch:
AppendMode.Quick— the generated function stamps(now() at time zone 'utc')in that mode and takes no timestamps array, so routing a Start through it would drop a caller'sIEvent.Timestamp, whichoverride_timestamp_on_start(mode: Quick)pins.UseArchivedStreamPartitioning— the dedicated INSERT routes the row tomt_streams_default, which is what makes reuse of an archived id legal.EnableStrictStreamIdentityEnforcement— its CTE writesmt_streams_identity.What I did not change. No SQL, no function signature, no public API, no existing test. The new MT003 →
ExistingStreamIdCollisionExceptionmapping is non-partitioned only, soStartStream_duplicate_id_within_tenant_collideskeeps pinning the partitioned path'sEventStreamUnexpectedMaxEventIdException— the divergence your comment asks to keep for intentional review.Tests. New
Bug_5454_start_stream_sequence_gap.cs: the gap (QWST), the collision contract (both quick modes), and versions from 1 on a successful start. Revert the two source files and the gap test fails — 0 tombstones,last_valuetwo ahead ofmax(seq_id).Local runs, .NET 10, on PostgreSQL 18.6 and 17.9:
EventSourcingTests: 2,272 passed, 20 skipped, 5 failed — allProjections.testing_projections.*, which fail identically on unpatched master here.TenantPartitionedEventsTests: 251 passed, 1 failed —projected_document_table_is_NOT_partitioned_by_default, also failing on unpatched master here.Dcb.Bug_5268_hstore_tag_index_can_be_built_out_of_bandfailed in one of four full runs (CREATE INDEX CONCURRENTLY, DDL execution failed) and passed in isolation, in the other three runs, and in every run on 18.6. It exercises archived-stream partitioning, which this patch leaves on the old route.Two rulings I left to you rather than making them in a PR:
Quickkeeps the gap, because two tests currently pin opposite behaviour for the same mode depending on the route:override_timestamp_on_start(mode: Quick)says a caller's timestamp is honoured;timestamp_source_per_append_mode.Quick_mode_uses_server_now_and_ignores_caller_timestampsays it is ignored under partitioning. Say which side wins and I'll follow up — drop the clause (server wins) or give the function an optionaltimestampsarray withcoalesce(caller wins, and partitionedQuickgains back-dating).StartStreamexception across the two configurations. One line now; left alone because your test says to.Happy to add a docs note on the
Quick/QuickWithServerTimestampstimestamp difference — it isn't obvious from the names. No urgency at our end; we handle this application-side already.