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

fix(swing-store): move getAllState/setAllState onto a debug object #6722

Closed
wants to merge 1 commit into from

Conversation

warner
Copy link
Member

@warner warner commented Dec 23, 2022

getAllState and setAllState are swing-store helper functions which copy (or set) all the state of a store at once, used exclusively for testing.

Previously, they were exported by swing-store, and applied to a kernelStorage object, but that depended upon a
streamStore.dumpStreams method that we'd rather not expose to normal users.

This commit adds a new debug object, next to kernelStorage and hostStorage, which holds getAllState, setAllState, and dumpStreams. The kernelStorage.streamStore object no longer has a dumpStreams method.

This also updates all the swingset unit tests which were using the helper functions.

@warner warner added the SwingSet package: SwingSet label Dec 23, 2022
@warner warner requested a review from FUDCo December 23, 2022 22:18
@warner warner self-assigned this Dec 23, 2022
@warner warner force-pushed the move-dumpstreams branch 3 times, most recently from 64695b3 to d9b1fd4 Compare January 4, 2023 19:25
`getAllState` and `setAllState` are swing-store helper functions which
copy (or set) all the state of a store at once, used exclusively for
testing.

Previously, they were exported by swing-store, and applied to a
`kernelStorage` object, but that depended upon a
`streamStore.dumpStreams` method that we'd rather not expose to normal
users.

This commit adds a new `debug` object, next to `kernelStorage` and
`hostStorage`, which holds `getAllState`, `setAllState`, and
`dumpStreams`. The `kernelStorage.streamStore` object no longer has a
`dumpStreams` method.

This also updates all the swingset unit tests which were using the
helper functions.
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

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

This looks good, though merging with the swing-store changes already in flight might be a pain, which raises a question of whether snapStore-then-debug-hooks or debug-hooks-then-snapStore will be the lesser merge effort. I'm going to withhold approving until we answer that question, so that unexpected automation doesn't do something prematurely, but the changes here themselves seem just fine.

Related to all that, I'm wondering are we going to want or need something analogous to dumpStreams for the snapStore?

@warner
Copy link
Member Author

warner commented Jan 14, 2023

I'll rewrite this on top of #6781 before proceeding. @FUDCo pointed out that what we really want is a programmatic form of /usr/bin/sqlite3 swingstore.sqlite .dump, since this API doesn't need to be editable or deterministic (or fast, although .dump is pretty fast). I found some pointers on using the built-in sqlite_schema table to accomplish this: this table includes a row for every table/index/etc that exists, and contains the SQL commands used to created the item in the first place. With a bit of fiddling, this can be used to reconstruct the .dump CLI functionality. So the output will be an array of strings, each a separate SQL command, and setAllState might be implemented with a simple db.exec(lines.join('\n')) (although maybe it wants to be a ss=createFromDump(lines) function instead).

@warner
Copy link
Member Author

warner commented Jan 14, 2023

Actually, for our purposes, we probably don't even need the dumped data to be a string. SQLite has a "serialize" API, which better-sqlite3 exposes as blob = db.serialize(), which just returns the raw in-memory DB contents. The corresponding SQLite [unserialize](https://sqlite.org/c3ref/deserialize.html) API is exposed a constructor argument:

const buffer = db.serialize();
db.close();
db = new Database(buffer);

which we might then have swingstore expose as createFromSerialized(buffer), a sibling of initSwingStoreopenSwingStore.

That would be sufficient for the "clone a swingstore" requirements of the unit tests that use this.

@warner
Copy link
Member Author

warner commented Jan 15, 2023

closing in favor of version that sits on top of #6781 and #6755

@warner warner closed this Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants