-
Notifications
You must be signed in to change notification settings - Fork 208
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
feat(swing-store): budget-limited deletion of snapshot and transcripts #9478
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
Security ConsiderationsNone. Swingstore exports during slow deletion continue to be coherent (export-data records remain until deleted, artifacts stop right away), unit tests demonstrate the importability of these exports. There were no changes to the import-side code, including the hash validation pathways. Scaling ConsiderationsThis change enables better scaling, by allowing the kernel to delete terminated vats slowly, and avoid overloading validator nodes with tons of DB or IAVL work in a single block. Documentation ConsiderationsThis PR introduces extensive new swing-store documentation, including coverage of the new deletion-budget feature. Testing ConsiderationsThe upcoming swingset changes that take advantage of this API should have their own unit tests, which exercise slow deletion. We'll also need to perform integration/performance tests with real mainnet chain data, and make sure we're satisfied with our choices for deletion rate / budget. We must choose values that delete the old price-feed vats in a timely fashion (weeks or months), and does not overload validators with the constant "background" cleanup work. Upgrade ConsiderationsNone: the old (unlimited) API continues to work the same as before. It only changes behavior if you provide a new positional |
3200c52
to
a698f7b
Compare
e2f75f4
to
472dd22
Compare
a698f7b
to
2e3641a
Compare
472dd22
to
b43f4da
Compare
2e3641a
to
fdecbe5
Compare
0126e7d
to
85c31ab
Compare
fdecbe5
to
796a2d3
Compare
796a2d3
to
2611cb9
Compare
0868981
to
e7be6b3
Compare
c05be76
to
70300ac
Compare
e7be6b3
to
1914088
Compare
70300ac
to
e79c29c
Compare
1914088
to
1ba5547
Compare
1ba5547
to
afe1c29
Compare
e79c29c
to
691b43a
Compare
691b43a
to
ede07c7
Compare
afe1c29
to
49ba122
Compare
ede07c7
to
a1b9efe
Compare
49ba122
to
77c46d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just doc changes for this first-round review; next I'll look at the code. But by the way, 👍👍 for making the former so thorough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once again, great testing. No blocking concerns.
@@ -39,7 +39,7 @@ import { buffer } from './util.js'; | |||
* loadSnapshot: (vatID: string) => AsyncIterableIterator<Uint8Array>, | |||
* saveSnapshot: (vatID: string, snapPos: number, snapshotStream: AsyncIterable<Uint8Array>) => Promise<SnapshotResult>, | |||
* deleteAllUnusedSnapshots: () => void, | |||
* deleteVatSnapshots: (vatID: string) => void, | |||
* deleteVatSnapshots: (vatID: string, budget?: number) => { done: boolean, cleanups: number }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: rename "cleanups" for clarity/comprehensibility.
* deleteVatSnapshots: (vatID: string, budget?: number) => { done: boolean, cleanups: number }, | |
* deleteVatSnapshots: (vatID: string, budget?: number) => { done: boolean, count: number }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into it.. that'll require matching in the kernel, in the next PR in this series
c862b45
to
227dafa
Compare
2fef729
to
1f7943e
Compare
227dafa
to
486dfbb
Compare
1f7943e
to
6547c83
Compare
486dfbb
to
f6787e8
Compare
Both `snapStore.deleteVatSnapshots()` and `transcriptStore.deleteVatTranscripts()` now take a numeric `budget=` argument, which will limit the number of snapshots or transcript spans deleted in each call. Both return a `{ done, cleanups }` record so the caller knows when to stop calling. This enables the slow deletion of large vats (lots of transcript spans or snapshots), a small number of items at a time. Recommended budget is 5, which (given SwingSet's `snapInterval=200` default) will cause the deletion of 1000 rows from the `transcriptItems` table each call, which shouldn't take more than 100ms. Without this, the kernel's attempt to slowly delete a terminated vat would succeed in slowly draining the kvStore, but would trigger a gigantic SQL transaction at the end, as it deleted every transcript item in the vat's history. The worst-case example I found would be the mainnet chain's v43-walletFactory, which (as of apr-2024) has 8.2M transcript items in 40k spans. A fast machine takes two seconds just to count all the items, and deletion took 22 *minutes*, with a `swingstore.wal` file that peaked at 27 GiB. This would cause an enormous chain stall at some surprising point in time weeks or months after the vat was first terminated. In addition, both the transcript spans and the snapshot records are shadowed into IAVL (via `export-data`) for integrity, and deleting 40k+40k=80k IAVL records in a single block might cause some significant churn too. The kernel should call `transcriptStore.stopUsingTranscript()` and `snapStore.stopUsingLastSnapshot()` as soon as the vat is terminated, to make exports smaller right away (by omitting all transcript/snapshot artifacts for the given vat, even before those DB rows or their export-data records have been deleted). New swing-store documentation was added. refs #8928 Co-authored-by: Richard Gibson <[email protected]>
f6787e8
to
c43bf63
Compare
Both
snapStore.deleteVatSnapshots()
andtranscriptStore.deleteVatTranscripts()
now take a numericbudget=
argument, which will limit the number of snapshots or transcript spans deleted in each call. Both return a{ done, cleanups }
record so the caller knows when to stop calling.This enables the slow deletion of large vats (lots of transcript spans or snapshots), a small number of items at a time. Recommended budget is 5, which (given SwingSet's
snapInterval=200
default) will cause the deletion of 1000 rows from thetranscriptItems
table each call, which shouldn't take more than 100ms.Without this, the kernel's attempt to slowly delete a terminated vat would succeed in slowly draining the kvStore, but would trigger a gigantic SQL transaction at the end, as it deleted every transcript item in the vat's history. The worst-case example I found would be the mainnet chain's v43-walletFactory, which (as of apr-2024) has 8.2M transcript items in 40k spans. A fast machine takes two seconds just to count all the items, and deletion took 22 minutes, with a
swingstore.wal
file that peaked at 27 GiB. This would cause an enormous chain stall at some surprising point in time weeks or months after the vat was first terminated. In addition, both the transcript spans and the snapshot records are shadowed into IAVL (viaexport-data
) for integrity, and deleting 40k+40k=80k IAVL records in a single block might cause some significant churn too.The kernel should call
transcriptStore.stopUsingTranscript()
andsnapStore.stopUsingLastSnapshot()
as soon as the vat is terminated, to make exports smaller right away (by omitting all transcript/snapshot artifacts for the given vat, even before those DB rows or their export-data records have been deleted).New swing-store documentation was added.
refs #8928