create new swingset-worker-xsnap-v1 package #7101
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
warner
force-pushed
the
6596-worker-xsnap-v1
branch
from
e5251ef to
4063a31
Compare
warner
force-pushed
the
6596-worker-xsnap-v1
branch
from
4063a31 to
cdc0401
Compare
Datadog ReportBranch report: ✅ |
warner
force-pushed
the
6596-worker-xsnap-v1
branch
from
cdc0401 to
511f5d5
Compare
warner
force-pushed
the
6596-worker-xsnap-v1
branch
2 times, most recently
from
470c569 to
48051e5
Compare
|
This is finally ready for review. Best reviewed one commit at a time: the core function gets refactored and moved around in like half the commits. |
warner
force-pushed
the
6596-worker-xsnap-v1
branch
from
48051e5 to
0731fbd
Compare
These types define the messages (deliveries and syscalls) that can pass between a swingset kernel and one of its workers. By moving them to swingset-liveslots, they can be imported by the upcoming "swingset-xsnap-supervisor" package. Strictly speaking, these are not defined by (or limited to) liveslots: we can imagine alternate (non-liveslots) vat runtimes which still speak the same language of deliveries and syscalls. For example we might define a less object-capability -ish runtime whose userspace code makes direct vatstore changes, in more of an ORM style, which lacks internal confinement but might be faster/cheaper. Or we could use a language other than JavaScript. In the long run, we probably need to define a separate "swingset-vat-api-v1" package, which defines the kinds of deliveries and syscalls that can be exchanged between the kernel and a vat worker. The kernel-side translators will emit these, the worker management code will transport them, and they will be consumed by liveslots/etc within the worker. If we add a new kind of delivery in the future, we could define "swingset-vat-api-v2" with the additions, the kernel could use both -v1 and -v2, and the translator would refrain from sending the extra -v2 messages to an older worker.
This extracts the XS supervisor bundle out of packages/SwingSet and into a new package whose NPM name is `@agoric/swingset-xsnap-supervisor`, and lives (for now) in packages/swingset-xsnap-supervisor . The bundle is created explicitly, by running `yarn build`. As with `xsnap-lockdown`, the intention is that each published version has a stable bundle, with specific (hashable) contents. Importing clients can rely upon this stability to e.g. have deterministic xsnap heap snapshot contents even if the versions of other parts of the system have changed. The new README.md describes the API, the stability goals, and how we achieve them. Fairly soon, we'll need manage the version number of this package more explicitly, either by moving it to a different git repository, or changing our use of `lerna publish` to avoid spurious changes. But for now we can leave it in place. refs #6596
Our replay-transcript.js tool has some affordances for forcing a rebuild of the lockdown and supervisor bundles. Now that the bundles are coming from separate packages, the affordances have changed. I'm not sure if the new code is as useful as the original.
Running tsc in the new swingset-xsnap-supervisor package caused this unrelated line in packages/store to start complaining. Removing the ts-expect-error caused it to fail tsc when run in packages/store. Thanks to @mfig for a fix that appeased both cases.
Minor refactoring of `makeStartXSnap` to inline as much as possible
This moves the "which bundles should we use?" responsibility into
startXSnap, described by a new 'workerVersion' argument. The only
version supported for now is 'xsnap-v1'.
The bundles themselves are loaded from their source
packages ('@agoric/xsnap-lockdown' and
'@agoric/swingset-xsnap-supervisor'), rather than being pulled from
the kvStore. We rely upon those packages supplying stable bundles
despite kernel-package changes.
Unit tests and replay tools can use `options.overrideBundles=[..]` to
replace the usual bundles with alternates: either no bundles, or
custom lockdown/supervisor bundles.
This moves the xsnap-worker-creation code out of SwingSet and into a new package, whose NPM name is `@agoric/swingset-worker-xsnap-v1`, and lives (for now) in packages/swingset-worker-xsnap-v1 . This new package encapsulates: * the choice of `xsnap` executable, hence the XS engine version inside * the choice of `@agoric/xsnap-lockdown`, hence the Endo/SES version * the choice of `@agoric/swingset-xsnap-supervisor`, hence liveslots We roughly expect there will be exactly one version of `@agoric/swingset-worker-xsnap-v1` published. Any major changes to XS/xsnap/SES/liveslots will be published to a new `@agoric/swingset-worker-xsnap-v2` package, and a single kernel can depend upon both -v1 and -v2 at the same time, to support older vats (started with -v1) as well as newer vats (started with, or upgraded to, -v2). Minor (and sufficiently backwards-compatible) changes of the components may be possible. If so, we can publish new versions of -v1, and deployed systems can perform orchestrated (within-consensus) upgrades from e.g. -v1@1.0 to -v1@1.1 . We still need to think carefully about potential differences in behavior between the first-run deliveries (using 1.0) and the post-upgrade replayed deliveries (using 1.1). SwingSet now has a primary dependency on the new `@agoric/swingset-worker-xsnap-v1` package, and only `devDependencies` on the individual components (`@agoric/swingset-xsnap-supervisor` and `@agoric/xsnap-lockdown`). Hopefully these dev-dependencies will go away once the dust settles. refs #6596
Previously, the SwingSet-side `manager-subprocess-xsnap.js` asked the `xsnap` package for the integers corresponding to the three meter-based error exits: E_TOO_MUCH_COMPUTATION, E_STACK_OVERFLOW, and E_NOT_ENOUGH_MEMORY. It needs these to distinguish an in-consensus vat termination from a random I-don't-know-why-so-panic-the-kernel worker exit. However, that required SwingSet to have a direct dependency on `@agoric/xsnap`, whereas we really want the xsnap version to be entirely encapsulated by the new swingset-worker-xsnap-v1 package. Imagine how this would work when we introduce a -v2 package, which e.g. uses a new xsnap that has changed the exit code values. This commit adds a new `trapMeteringFailure()` method to the workers created by `swingset-worker-xsnap-v1`. This method inspects the error object (including the exit code), and either converts it into a termination-reason string (for the three recognized meter-based exits), or throws an error (for anything else). The manager can call this worker method instead of needing to know the exact exit codes, delegating knowledge of the details to the specific worker version. This allows SwingSet to lose the direct dependency upon @agoric/xsnap, relegating it to `devDependencies`.
warner
force-pushed
the
6596-worker-xsnap-v1
branch
from
0731fbd to
2a7780a
Compare
warner
force-pushed
the
6596-supervisor
branch
2 times, most recently
from
9fbfb20 to
8139f14
Compare
|
In our meeting today, our new approach for #6596 does not use a separate worker package, so I'm abandoning this PR in favor of the plan described in #6596 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the main
startXSnapfunctionality out to a new package named@agoric/swingset-worker-xsnap-v1, and replacing the packages/SwingSet code with a stub that switches between available versions via a newworkerVersionargument (the only currently accepted version isxsnap-v1).refs #6596