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

[Support v4] Fix unqueue first then publish #87

Merged
merged 3 commits into from
Nov 3, 2021

Conversation

jokesterfr
Copy link
Contributor

@jokesterfr jokesterfr commented Nov 2, 2021

We wait for the publisher to fully push the events before flushing the queue. If concurrent .commit() calls are performed this could lead to IDEMPOTENT WRITE errors on the eventstore side, leading to unsent events.

This PR aims at flushing the events before sending them to the publisher, drastically reducing error probability.

@jokesterfr jokesterfr self-assigned this Nov 2, 2021
@jokesterfr jokesterfr added the bug Something isn't working label Nov 2, 2021
@jokesterfr
Copy link
Contributor Author

Could this be merged to a v4 support branch?

@jokesterfr jokesterfr changed the title Fix unqueue first then publish [Support v4] Fix unqueue first then publish Nov 2, 2021
@jokesterfr
Copy link
Contributor Author

jokesterfr commented Nov 2, 2021

IMO a good implementation would commit events without any manual commit() call, just setting rules about refresh frequency and/or events count per batch.

On the implementation side this would be a simple setInterval() calling the publishers on a regular basis if uncommitted events remain.

@jokesterfr jokesterfr changed the base branch from master to support/4.x November 3, 2021 11:00
@jokesterfr
Copy link
Contributor Author

We also should deprecate the auto commit feature sooner or later.

@jokesterfr jokesterfr merged commit 7b211fa into support/4.x Nov 3, 2021
@jokesterfr jokesterfr deleted the fix/concurrent-commit-v4.x branch November 3, 2021 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants