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

feat(SwingSet): evictAfterSnapshot option for periodic eviction of all vats #5929

Closed
wants to merge 3 commits into from

Conversation

dckc
Copy link
Member

@dckc dckc commented Aug 10, 2022

Add a switch in case "turn it off and turn it on again" helps manage vat worker RAM usage.

refs: #5910

Documentation Considerations

$EVICT_AFTER_SNAPSHOT environment variable is documented.

Testing Considerations

unit tests updated

passes a smoke test:

agoric-sdk/packages/cosmic-swingset$ EVICT_AFTER_SNAPSHOT=1 make scenario2-run-chain
...
2022-08-17T02:15:44.347Z SwingSet: vat: v1: agoricNamesAdmin settled; remaining: []
XS snapshot written to /home/connolly/projects/agoric-sdk/packages/cosmic-swingset/t1/n0/data/ag-cosmos-chain-state/xs-snapshots/ba667a6f910f04ec834edfb9f71b480f94b8f2fad3fe40bd0a27a4292a9d6f5d.gz : 2664962 bytes compressed, 14694111 raw
2022-08-17T02:15:45.181Z SwingSet: kernel: periodic eviction v1

@dckc dckc force-pushed the dc-restart-workers branch from b52b35b to 0aff12c Compare August 10, 2022 23:01
@dckc dckc marked this pull request as ready for review August 10, 2022 23:05
@dckc
Copy link
Member Author

dckc commented Aug 12, 2022

@arirubinstein @warner maybe we should land this after all, until we have a better idea?

@dckc dckc requested review from arirubinstein and warner August 12, 2022 19:42
@warner
Copy link
Member

warner commented Aug 12, 2022

@arirubinstein @warner maybe we should land this after all, until we have a better idea?

Let's look at the slogfile timing and get a sense for how long it takes to evict the worker (and how long it takes to load it back in again later).. that should help us make a decision.

@warner
Copy link
Member

warner commented Aug 16, 2022

Let's add runtimeOptions.warehousePolicy.evictAfterSnapshot, which launch-chain.js will add to the third argument of its makeSwingsetController() call, to enable this eviction only on the chain. Swingset will have it disabled by default. Let's add a copy of test-controller.js 's static vats are unmetered on XS to exercise setting this to true, which should cause it to observe t.deepEqual(c.dump().log, ['buildRootObject called', 'bootstrap called', 'buildRootObject called', 'bootstrap called']). We could probably strip out some other parts of the test but if it's sensitive to the thing that we're testing, maybe we don't bother. It's probably worth a note in the new test that we expect the normal actions of the test (whatever basedir-controller-2 's config is provoking) to result in enough deliveries to trigger exactly one snapshot writing, followed by at least one delivery, so that we start a worker twice.

Although .. hang on, the second worker startup should be happening from the snapshot, so why is it invoking vatPowers.testLog('buildRootObject called') more than once? We might need to investigate that, to make sure the test is doing what we expect.

@dckc dckc changed the title test(swingset): EXPERIMENTAL: periodic eviction of all vats feat(SwingSet): evictAfterSnapshot option for periodic eviction of all vats Aug 17, 2022
@dckc dckc requested a review from michaelfig August 17, 2022 02:36
dckc added 3 commits August 16, 2022 21:37
This lets us check whether "turn it off and turn it on again"
affects performance trends.
 - test 'static vats are unmetered on XS' with both true and false
@dckc dckc force-pushed the dc-restart-workers branch from accc610 to 214b86f Compare August 17, 2022 02:37
@dckc
Copy link
Member Author

dckc commented Aug 17, 2022

... hang on, ... so why ...

I don't have a good mental model; I hope you can figure that out, @warner .
@FUDCo maybe you know?

@dckc
Copy link
Member Author

dckc commented Aug 17, 2022

overtaken by #5987

@dckc dckc closed this Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants