feat(outbox): implement outbox queue on transaction #548
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
zcabter
changed the title
Codecov ReportAttention: Patch coverage is
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
zcabter
force-pushed
the
ryan@outbox
branch
2 times, most recently
from
59bcf1d to
72460ad
Compare
zcabter
changed the title
zcabter
changed the title
zcabter
force-pushed
the
ryan@outbox
branch
2 times, most recently
from
ae53270 to
995a24b
Compare
johnyob
requested changes
Aug 28, 2024
crates/jstz_core/src/kv/outbox.rs
Outdated
| /// next flush. | ||
| pub fn flush( | ||
| &mut self, | ||
| host: &mut impl Runtime, |
There was a problem hiding this comment.
Suggested change
| host: &mut impl Runtime, | |
| rt: &mut impl Runtime, |
crates/jstz_core/src/kv/outbox.rs
Outdated
| let max = u16::MAX as u32; | ||
| let mut outbox_queue = OutboxQueue { | ||
| meta: OutboxQueueMeta { | ||
| queue_len: 120, |
There was a problem hiding this comment.
Incorrect setup? Initial queue length is 60
crates/jstz_core/src/kv/outbox.rs
Outdated
|
|
||
| outbox_queue.flush(&mut host, outbox_queue_snapshot); | ||
|
|
||
| assert_eq!(20, outbox_queue.queue_len()); |
There was a problem hiding this comment.
Incorrect calculation: 120 - 60 (flushed) - 40 (written) = 20
Should be: 60 - 60 (flushed) + 20 (queued) = 20
zcabter
force-pushed
the
ryan@outbox
branch
5 times, most recently
from
bdbd15d to
10d45f0
Compare
johnyob
reviewed
Aug 29, 2024
zcabter
force-pushed
the
ryan@outbox
branch
2 times, most recently
from
617b2ff to
e4ad642
Compare
feat(outbox): move outbox snapshots into outbox queue
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.
Context
This PR omplements the lower level transactional outbox queue api
Part of JSTZ-27
Description
Why
The kernel is only allowed to write 100 messages per level. Luckily, the kernel sdk to expose an OutboxQueue which decouples enqueuing outbox messages and writing to the outbox, allowing a much high upper limit of outbox messages to be persisted as long as they are flushed (ideally, every level). However, the
OutboxQueueis eager to write to persistent storage on message enqueue which makes it difficult to rollback and doesn't fit our transaction model.What
Instead, we define an outbox queue for each snapshot that adheres to the transaction rules. Furthermore, an api is added to transaction to enqueue messages to the latest snapshot and to flush messages (on commit).
Here are high level details:
Manually testing the PR
cargo test -p jstz_core kv