Skip to content

Remember the generated stream_id across batches of events being appeneded.#289

Merged
jwilger merged 3 commits intocommanded:masterfrom
bitfield-co:283-eventstoreappend_to_stream-is-not-able-to-append-more-than-1000-events-to-a-new-stream
Oct 21, 2024
Merged

Remember the generated stream_id across batches of events being appeneded.#289
jwilger merged 3 commits intocommanded:masterfrom
bitfield-co:283-eventstoreappend_to_stream-is-not-able-to-append-more-than-1000-events-to-a-new-stream

Conversation

@drteeth
Copy link
Contributor

@drteeth drteeth commented Sep 19, 2024

Addresses #283

By returning and remembering stream_id created in the first batch, then subsequent batches no longer fail with a duplicate stream_uuid constraint error.

This has been a long standing issue as mentioned in the issue. It was hidden by excluding :slow tests by default. This PR also includes slow tests by default. The test run goes from about 30s to 60s on my machine.

I think we can close #285 in favour of this one.

Big thanks to @thomasdziedzic for finding this one.

@drteeth drteeth requested review from cdegroot and jwilger September 19, 2024 15:22
So that subsequent batches update instead of trying to insert and fail
@drteeth drteeth force-pushed the 283-eventstoreappend_to_stream-is-not-able-to-append-more-than-1000-events-to-a-new-stream branch from 39f429b to 0956c4c Compare September 19, 2024 15:25
{:ok, %Postgrex.Result{num_rows: 0}} -> {:error, :not_found}
{:ok, %Postgrex.Result{}} -> :ok
{:error, error} -> handle_error(error)
{:ok, %Postgrex.Result{num_rows: 0}} ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This branch can no longer be hit, and whatever case it was guarding against will change. There is no test that was covering this.

@cdegroot
Copy link
Contributor

Looks a bit better than #283 so let's merge this and close that one.

@jwilger jwilger merged commit 54429c3 into commanded:master Oct 21, 2024
@drteeth drteeth deleted the 283-eventstoreappend_to_stream-is-not-able-to-append-more-than-1000-events-to-a-new-stream branch October 22, 2024 15:34
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.

3 participants