Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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 jwilger and cdegroot 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
2 checks passed
@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