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.

Sign up for GitHub

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

Jump to bottom

fix(swingset): delete unused snapshots #3505

Merged
merged 23 commits into from
Jul 29, 2021
Merged

fix(swingset): delete unused snapshots #3505

merged 23 commits into from
Jul 29, 2021

Conversation

dckc
Copy link
Member

@dckc dckc commented Jul 21, 2021

In addition to maintaining a mapping from vatID to snapshotID,
vatKeeper maintains a reverse mapping.

After commitCrank(), the kernel calls vatWarehouse.pruneSnapshots(),
which, if snapshotPruneInterval cranks have passed:

  1. calls kernelKeeper.getUnusedSnapshots(),
  2. tries to snapStore.delete() each of them, and
  3. reports the results using kernelKeeper.forgetUnusedSnapshots().

fixes #3374

includes progress on #3431

@dckc dckc requested review from warner, mhofman and FUDCo July 21, 2021 21:18
@dckc dckc force-pushed the 3374-prune-snapshots branch from 813f43c to d615b80 Compare July 22, 2021 21:03
@warner
Copy link
Member

warner commented Jul 22, 2021

Oh, we should make sure that terminated vats get removed from the users list.

@warner warner mentioned this pull request Jul 23, 2021
Closed
@dckc dckc force-pushed the 3374-prune-snapshots branch 3 times, most recently from d5be4a9 to 2b8a348 Compare July 27, 2021 21:06
@dckc dckc force-pushed the 3374-prune-snapshots branch from e23f6ca to d074c9e Compare July 28, 2021 00:47
@dckc dckc requested a review from michaelfig July 28, 2021 01:43
@warner warner self-requested a review July 29, 2021 17:52
warner
warner approved these changes Jul 29, 2021
View reviewed changes
Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One additional check in the unit test recommended, rest looks good.

dckc added 4 commits July 29, 2021 13:29
@dckc
refactor(swingset): factor out makeSnapStoreIO
446a39c 
refs #3431
@dckc
refactor(swingset): move c.shutdown to caller of runSteps
accec6a 
runSteps didn't create it; its caller still has a reference
@dckc
fix(swingset): delete unused snapshots
aaa84e3 
In addition to maintaining a mapping from vatID to snapshotID,
vatKeeper maintains a reverse mapping.

After `commitCrank()`, the kernel calls `vatWarehouse.pruneSnapshots()`,
which
 1. calls `kernelKeeper.getUnusedSnapshots()`,
 2. tries to `snapStore.delete()` each of them, and
 3. reports the results using `kernelKeeper.forgetUnusedSnapshots()`.

fixes #3374
@dckc
feat(swingset): configurable snapshotPruneInterval (=20)
5e71fc0 
Pruning snapshots involves an O(n) query on the DB, so
doing it on every crank seems expensive.

refs #3374
dckc added 19 commits July 29, 2021 13:29
@dckc
test(swingset): use tmp directories for snapStore in tests
8984761
@dckc
chore: move snapStore to swing-store-lmdb package
479a264
@dckc
feat(snapstore): prepare/commit deletes
7b0a4b0
@dckc
feat(swing-store-lmdb): provide snapStore with kvStore, streamStore
f1bc8f7
@dckc
chore(swingset): host commit() deletes snapshots
a5e23a2 
 - makeSnapStoreIO is in swing-store-lmdb now
 - vatKeeper.saveSnapshot() prepares deletes
   - removeFromSnapshot() returns consumers.length
   - no more vatWarehouse.pruneSnapshots
     - no more snapshotPruneInterval
 - test-warehouse: use commit()
 - test-reload-snapshot: update imports
@dckc
chore(cosmis-swingset): use consolidated snapStore API
9c48c70
@dckc
chore(solo): use consolidated snapStore API
f0493e5
@dckc
chore(swingset): free snapshots of terminated vats
8eddd61
@dckc
refactor(swingset): static type for kvStore in vatKeeper
6991266 
 - getRequired() asserts that get() does not return undefined
 - fix addHelpers() return type by declaring arg type
 - where kvStore.get() is ensured by getKeys() or has(),
   mark the alternative with assert.fail().
@dckc
fix(vat-warehouse): don't provideVatKeeper on terminated vat
561ec00
@dckc
docs(swingset): mark DB keys excluded from consensus
1578a5d
@dckc
chore(snapstore): assert hash has no path separators
3369f2e
@dckc
chore: prune kernelKeeper.forgetUnusedSnapshots
87ce0c1
@dckc
test: no more snapshotPruneInterval
6f34be9
@dckc
fix(snapstore): robust prepare / commit delete
e1f0150
@dckc
docs(swingstore): document snapStore commit constraint
0d5504b
@dckc
refactor: concise default to [] in vatKeeper.addToSnapshot
ce67061
@dckc
test: move xsnap/snapstore integration test to solo
21a7a91 
to avoid @agoric/xsnap in swing-store-lmdb devDependencies
@dckc
test(snapstore): re-save between prepare and commit delete
1b2050d 
plus one more /etc/passwd test
@dckc dckc force-pushed the 3374-prune-snapshots branch from c6e76e0 to 1b2050d Compare July 29, 2021 18:32
@dckc dckc enabled auto-merge (squash) July 29, 2021 18:46
@dckc dckc merged commit 317959d into master Jul 29, 2021
@dckc dckc deleted the 3374-prune-snapshots branch July 29, 2021 19:05
@dckc dckc mentioned this pull request Aug 14, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@warner warner warner approved these changes

@mhofman mhofman Awaiting requested review from mhofman

@FUDCo FUDCo Awaiting requested review from FUDCo

@michaelfig michaelfig Awaiting requested review from michaelfig

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

snapstore: unused snapshots are never deleted
4 participants
@dckc @warner @michaelfig @FUDCo