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

No deletion of older swing-store artifacts when creating new spans #9387

Closed
Tracked by #9174
mhofman opened this issue May 20, 2024 · 0 comments · Fixed by #10032
Closed
Tracked by #9174

No deletion of older swing-store artifacts when creating new spans #9387

mhofman opened this issue May 20, 2024 · 0 comments · Fixed by #10032
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers swing-store SwingSet package: SwingSet

Comments

@mhofman
Copy link
Member

mhofman commented May 20, 2024

What is the Problem Being Solved?

When switching retention config (#9386) from "prune nothing" to "prune everything", a new span may cause a lot of old span artifacts to be found eligible for deletion. Unless we stop storing these artifacts in the SQLite DB (issue TBD), this may be a costly operation (see #8928 for a similar problem when terminating a vat).

Description of the Design

Only delete the previous span artifact when creating a new span or incarnation, and leave pruning of any older spans to an external mechanism (like state-sync / export+import).

Security Considerations

None

Scaling Considerations

Maintain constant time span creation

Test Plan

Some unit test of swing-store

Upgrade Considerations

Host side of the swingset kernel stack, not upgrade sensitive, but deployed as part of a chain software upgrade.

@mhofman mhofman added enhancement New feature or request swing-store labels May 20, 2024
@mhofman mhofman added good first issue Good for newcomers SwingSet package: SwingSet labels Jul 12, 2024
@mergify mergify bot closed this as completed in #10032 Sep 6, 2024
mergify bot added a commit that referenced this issue Sep 6, 2024
Ref #9174
Fixes #9387
Fixes #9386

TODO:
- [ ] #9389

## Description
Adds consensus-independent `vat-snapshot-retention` ("debug" vs. "operational") and `vat-transcript-retention` ("archival" vs. "operational" vs. "default") cosmos-sdk swingset configuration (values chosen to correspond with [`artifactMode`](https://github.com/Agoric/agoric-sdk/blob/master/packages/swing-store/docs/data-export.md#optional--historical-data)) for propagation in AG_COSMOS_INIT. The former defaults to "operational" and the latter defaults to "default", which infers a value from cosmos-sdk `pruning` to allow simple configuration of archiving nodes.

It also updates the semantics of TranscriptStore `keepTranscripts: false` configuration to remove items from only the previously-current span rather than from all previous spans when rolling over (to avoid expensive database churn). Removal of older items can be accomplished by reloading from an export that does not include them.

### Security Considerations
I don't think this changes any relevant security posture.

### Scaling Considerations
This will reduce the SQLite disk usage for any node that is not explicitly configured to retain snapshots and/or transcripts. The latter in particular is expected to have significant benefits for mainnet (as noted in #9174, about 116 GB ÷ 147 GB ≈ 79% of the database on 2024-03-29 was vat transcript items).

### Documentation Considerations
The new fields are documented in our default TOML template, and captured in a JSDoc type on the JavaScript side.

### Testing Considerations
This PR extends coverage TranscriptStore to include `keepTranscripts` true vs. false, but I don't see a good way to cover Go→JS propagation other than manually (which I have done). It should be possible to add testing for the use and validation of `resolvedConfig` in AG_COSMOS_INIT handling, but IMO that is best saved for after completion of split-brain (to avoid issues with same-process Go–JS entanglement).

### Upgrade Considerations
This is all kernel code that can be used at any node restart (i.e., because the configuration is consensus-independent, it doesn't even need to wait for a chain software upgrade). But we should mention the new cosmos-sdk configuration in release notes, because it won't be added to existing app.toml files already in use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers swing-store SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants