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

Add a reapAllVats method to the controller #8634

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Dec 7, 2023

Closes #8626

@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet labels Dec 7, 2023
@FUDCo FUDCo requested a review from warner December 7, 2023 02:31
@FUDCo FUDCo self-assigned this Dec 7, 2023
@FUDCo FUDCo force-pushed the 8626-reap-all-vats branch from cceee44 to 0356e4b Compare December 8, 2023 08:35
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.

Sounds good.. approved pending the suggested changes.

packages/SwingSet/test/reap-all/bootstrap-reap-all.js Outdated Show resolved Hide resolved
packages/SwingSet/test/reap-all/test-reap-all.js Outdated Show resolved Hide resolved

const dumpBefore = c.dump();
t.is(dumpBefore.acceptanceQueue.length, 12);
t.is(dumpBefore.reapQueue.length, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Oh, neat, it didn't occur to me to use reapQueue for measuring this.. very clever.

Let's see, the kernel servicing code:

  function getNextMessageAndProcessor() {
    const acceptanceMessage = kernelKeeper.getNextAcceptanceQueueMsg();
    if (acceptanceMessage) {
      return {
        message: acceptanceMessage,
        processor: processAcceptanceMessage,
      };
    }
    const message =
      processGCActionSet(kernelKeeper) ||
      kernelKeeper.nextReapAction() ||
      kernelKeeper.getNextRunQueueMsg();
    return { message, processor: processDeliveryMessage };
  }

services the acceptanceQueue first (transferring items to the run-queue), then takes any GC action, then reapQueue, then finally runQueue. So when we use this feature for GC remediation, it's important to drain a controller.run() first (leaving all queues empty), then submit the c.reapAllVats(), then drain a second controller.run().

(if we submitted the reapAllVats() while there were still things in the acceptance or run queues, the reaps would happen first, then the real deliveries, which would leave GC stuff from the real deliveries still hanging out for an unknown period of time before the "organic" BOYDs finally happened)

@FUDCo FUDCo force-pushed the 8626-reap-all-vats branch from 0356e4b to 884bd01 Compare December 11, 2023 18:43
@FUDCo FUDCo added the automerge:rebase Automatically rebase updates, then merge label Dec 11, 2023
Copy link

@FUDCo - This PR appears to be stuck. It's had a merge label for > 24 hours

@FUDCo FUDCo force-pushed the 8626-reap-all-vats branch from 884bd01 to 6ba0af9 Compare December 13, 2023 19:22
@mergify mergify bot merged commit 4fb0418 into master Dec 13, 2023
67 checks passed
@mergify mergify bot deleted the 8626-reap-all-vats branch December 13, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add controller.reapAllVats()
2 participants