refactor: CapsuleStore with staged writes #19449
Merged
+421
−116
Conversation
This file contains hidden or 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
mverzilli
added
ci-no-squash
ci-no-fail-fast
mverzilli
force-pushed
the
martin/capsule-store-with-staged-writes
branch
from
e705402 to
e770876
Compare
nventuro
reviewed
Jan 9, 2026
nventuro
approved these changes
Jan 12, 2026
Contributor
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
I have some more comments, but it's already looking great! Address them and merge as you see fit. Thanks!
| * Deletes a capsule on the stage of a job. Note the capsule will still | ||
| * exist in storage until the job is committed. | ||
| */ | ||
| #deleteOnStage(jobId: string, dbSlotKey: string) { |
Contributor
There was a problem hiding this comment.
Is is 'stage' or 'staged'?
Contributor
Author
There was a problem hiding this comment.
There are staged capsules which mean they are on stage 🤡. Naming in this whole series of PR has not been my forte tbh. I re-think how I should call this.
Comment on lines
+86
to
+91
| const dataBuffer = await this.#capsules.getAsync(dbSlotKey); | ||
| if (!dataBuffer) { | ||
| return null; | ||
| } | ||
|
|
||
| return dataBuffer; |
Contributor
There was a problem hiding this comment.
Suggested change
| const dataBuffer = await this.#capsules.getAsync(dbSlotKey); | |
| if (!dataBuffer) { | |
| return null; | |
| } | |
| return dataBuffer; | |
| return await this.#capsules.getAsync(dbSlotKey) ?? null; |
| */ | ||
| async #getFromStage(jobId: string, dbSlotKey: string): Promise<Buffer | null | undefined> { | ||
| const jobStagedCapsules = this.#getJobStagedCapsules(jobId); | ||
| let staged: Buffer | null | undefined = jobStagedCapsules.get(dbSlotKey); |
Contributor
There was a problem hiding this comment.
Why do we need explicit types here?
Comment on lines
+63
to
+67
| if (staged === undefined) { | ||
| // If we don't have a staged version of this dbSlotKey, first we check if there's one in DB | ||
| staged = await this.#loadCapsuleFromDb(dbSlotKey); | ||
| } | ||
| return staged; |
Contributor
There was a problem hiding this comment.
I'd do
Suggested change
| if (staged === undefined) { | |
| // If we don't have a staged version of this dbSlotKey, first we check if there's one in DB | |
| staged = await this.#loadCapsuleFromDb(dbSlotKey); | |
| } | |
| return staged; | |
| if (staged !== undefined) { | |
| return staged; | |
| } else { | |
| // If we don't have a staged version of this dbSlotKey, first we check if there's one in DB | |
| return this.#loadCapsuleFromDb(dbSlotKey); | |
| } |
but maybe that's just me. It feels weird to do staged = db when we don't actually make the db value become staged.
| async deleteCapsule(contractAddress: AztecAddress, slot: Fr): Promise<void> { | ||
| await this.#capsules.delete(dbSlotToKey(contractAddress, slot)); | ||
| deleteCapsule(contractAddress: AztecAddress, slot: Fr, jobId: string) { | ||
| // When we commit this, we will interpret null as a deletion, so we'll propagate the delete to the KV store |
Contributor
There was a problem hiding this comment.
Suggested change
| // When we commit this, we will interpret null as a deletion, so we'll propagate the delete to the KV store |
I don't think needs needs clarifying at the callsite
| * exist in storage until the job is committed. | ||
| */ | ||
| #deleteOnStage(jobId: string, dbSlotKey: string) { | ||
| this.#getJobStagedCapsules(jobId).set(dbSlotKey, null); |
Contributor
There was a problem hiding this comment.
Suggested change
| this.#getJobStagedCapsules(jobId).set(dbSlotKey, null); | |
| // A staged null value indicates deletion, and will result in the KV store entry being deleted on commit | |
| this.#getJobStagedCapsules(jobId).set(dbSlotKey, null); |
Comment on lines
+191
to
+194
| // This transactional context gives us "copy atomicity": | ||
| // there shouldn't be concurrent writes to what's being copied here. | ||
| // Equally important: this in practice is expected to perform thousands of DB operations | ||
| // and not using a transaction here would heavily impact performance. |
Contributor
There was a problem hiding this comment.
Why not use the full 120 line length?
Contributor
Author
There was a problem hiding this comment.
Because there are places where it hurts my eyes to introduce a line break. Is this a case of conflicting OCD's? :P
Contributor
Author
|
Thanks for the review! I'll merge as is and address comments on the next one in line, so I don't miss the integration train any longer |
mverzilli
added
ci-squash-and-merge
and removed
ci-no-fail-fast
Second part of the series started with #19445. This makes the CapsuleStore work based on staged writes. With this, capsules aren't written to persistent storage until PXE decides to commit the job.
AztecBot
force-pushed
the
martin/capsule-store-with-staged-writes
branch
from
1de3908 to
b30c796
Compare
Collaborator
Flakey Tests🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
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.
Second part of the series started with #19445.
This makes the CapsuleStore work based on staged writes. With this, capsules aren't written to persistent storage until PXE decides to commit the job.