fix(swing-store): replace getAllState/etc with a debug facet #6796
Conversation
|
This PR wants to land on trunk after the two #6742 PRs finish rearranging the snapstore, but to get CI to run, I'm configuring the PR target to the second snapstore PR's branch. I don't actually want to merge it into that branch; this is a separate effort. To get the types to pass lint, this makes some changes that should really be in #6742 (specifically the removal of the ephemeral/stub snapstore: now that snapshots are in the DB, even |
|
requesting a preliminary review from @FUDCo, also feel free to pull typedef updates or the no-more-ephemeral-snapstore changes from it |
FUDCo
force-pushed
the
6742-snapshot-with-metadata
branch
2 times, most recently
from
b843da9 to
0da5f4b
Compare
warner
force-pushed
the
move-dumpstreams
branch
from
e3ec1f1 to
4d5574b
Compare
warner
force-pushed
the
move-dumpstreams
branch
from
4d5574b to
0ff1df5
Compare
| if (filePath !== ':memory:') { | ||
| throw Error('on-disk DBs with WAL mode enabled do not serialize well'); | ||
| } |
There was a problem hiding this comment.
Hm, I'm unsure.. it's not like the user did something wrong, it's more like "I wish I could accomodate that, but I can't, sorry". assert.meekly(), as it were. I guess I'll leave it.
| return db.serialize(); | ||
| } | ||
|
|
||
| function getAllKVEntries() { |
There was a problem hiding this comment.
For symmetry, I'm wondering if getAllKVEntries should be called something like dumpKVEntries.
I'm also wondering if at this point we should split all the kvStore stuff into its own source file, parallel to snapStore.js and streamStore.js. (Not in this PR, but if you agree it makes sense then one of us should add an issue.)
There was a problem hiding this comment.
yes on both, I'll file an issue and I can do it after we land the getNext stuff (just to minimize merge conflicts)
There was a problem hiding this comment.
actually I'll do the dumpKVEntries here, it's trivial, but I'll defer the refactoring for later, the activityhash made that nontrivial
|
Hrmph, the CI failure looks to be a GC variance between original and replay under Node.js (but only under Node-18.3.0, not 16). And of course I can't reproduce it locally, even under linux and the same version of Node. I'm going to change it to only use XS (as the comment suggests). What this test is examining shouldn't depend upon the engine in use. |
Previously, `getAllState` and `setAllState` were swing-store helper
methods which copy (or set) all the state of a store at once, used
exclusively for testing. These tests would either want to inspect the
swing-store contents directly, or clone a swing-store for e.g. replay
testing.
This commit replaces both with a new `debug` facet (a sibling of
`kernelStorage` and `hostStorage`), which offers two
methods.
`debug.dump()` returns a JS Object with the store data in an
easy-to-examine format (`dump.kvEntries['key']=value`,
`dump.streams[vatID]=[..]`, and `dump.snapshots[vatID] = {endPos,
hash, compressedSnapshot }`.
For cloning, `debug.serialize()` returns a Buffer that has a raw copy
of the SQLite backing store. This can be used to make a new DB by
passing it as an option to `initSwingStore`:
```js
const serialized = swingstore1.debug.serialize();
const swingstore2 = initSwingStore(null, { serialized });
```
Note that both `.serialize()` and `{ serialized }` require an in-RAM
database, rather than an on-disk one.
It also cleans up the streamstore types exports a bit.
I'm seeing CI failures under Node-18 in this test, presumeably because we get GC variation between the first run and the replay. Super annoying, and not germane to what this test is supposed to be executing. The `test.serial` wrapper wasn't enough to fix it, so I've just switched the test to only use xs-worker. refs #3240
warner
force-pushed
the
move-dumpstreams
branch
from
0ff1df5 to
c6ecf28
Compare
warner
added
the
automerge:no-update
Previously,
getAllStateandsetAllStatewere swing-store helper methods which copy (or set) all the state of a store at once, used exclusively for testing. These tests would either want to inspect the swing-store contents directly, or clone a swing-store for e.g. replay testing.This commit replaces both with a new
debugfacet (a sibling ofkernelStorageandhostStorage), which offers two methods.debug.dump()returns a JS Object with the store data in an easy-to-examine format (dump.kvEntries['key']=value,dump.streams[vatID]=[..], anddump.snapshots[vatID] = {endPos, hash, compressedSnapshot }.For cloning,
debug.serialize()returns a Buffer that has a raw copy of the SQLite backing store. This can be used to make a new DB by passing it as an option toinitSwingStore:Note that both
.serialize()and{ serialized }require an in-RAM database, rather than an on-disk one.