feat: add locking in cleanup #23
Merged
Conversation
This file contains 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
chris-asl
force-pushed
the
feat/add-locking-in-cleanup
branch
4 times, most recently
from
d1de85a to
649f6fc
Compare
GregBal
reviewed
Jun 17, 2024
core/src/main/kotlin/io/github/bluegroundltd/outbox/TransactionalOutboxImpl.kt
Outdated
Show resolved
Hide resolved
chris-asl
force-pushed
the
feat/add-locking-in-cleanup
branch
3 times, most recently
from
810de71 to
b9dea5a
Compare
ippokratoys
reviewed
Jun 17, 2024
core/src/main/kotlin/io/github/bluegroundltd/outbox/TransactionalOutboxImpl.kt
Show resolved
Hide resolved
| @@ -27,7 +27,7 @@ Transactional Outbox is published on `mavenCentral`. In order to use it just add | |||
|
|
|||
| ```gradle | |||
|
|
|||
| implementation("io.github.bluegroundltd:transactional-outbox-core:1.0.0") | |||
| implementation("io.github.bluegroundltd:transactional-outbox-core:2.0.0") | |||
There was a problem hiding this comment.
Should we also add some notes on how users should upgrade from 1.0.0 to 2.0.0?
Or we should add this to the release tag after the release?
There was a problem hiding this comment.
Hmm, that's a nice idea!
In this case though, the upgrade only requires the addition of the locks provider call and the rename of the current function. The TransactionalBuilder API is guiding the user via the fluent interface. I've also kinda updated the docs here.
Do you possibly mean adding a migration guide? Like
Upgrading from 1.x to 2.0.0
?
Despite the fact that the store implementation could simply issue a `DELETE FROM outbox_item WHERE delete_after < now` it could also, fetch the to be deleted items and then try to delete them, which when run concurrently (w/o locks) could end up with errors. Thus, to be safe, we're introducing locks in cleanup, as well. We could use the same locks provider we're using for monitor, but that would serialize the monitor and cleanup runs, which is not required. [VR-3403]
chris-asl
force-pushed
the
feat/add-locking-in-cleanup
branch
2 times, most recently
from
cc3d366 to
16980d8
Compare
[VR-3403]
A breaking change since we change the interface of `TransactionalOutboxBuilder`. [VR-3403]
chris-asl
force-pushed
the
feat/add-locking-in-cleanup
branch
from
16980d8 to
fba98cb
Compare
ippokratoys
approved these changes
Jun 19, 2024
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.
Continuation of the outbox items cleanup epic. As per our spike, we've introduced a separate locks provider for cleanup. Please refer to the commit message for the rationale behind this.
Resolves [VR-3403].