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

Feature: task level isolation #62

Closed
AriPerkkio opened this issue Jun 29, 2023 · 3 comments · Fixed by #63
Closed

Feature: task level isolation #62

AriPerkkio opened this issue Jun 29, 2023 · 3 comments · Fixed by #63
Labels
enhancement New feature or request

Comments

@AriPerkkio
Copy link
Member

AriPerkkio commented Jun 29, 2023

Vitest will need "task level" worker isolation for following features and fixes:

Naming things is hard, but by "task level isolation" I mean a situation where pool's isolateWorkers can be set as false, but then pool itself can be instructed to use fresh new workers for tasks on demand. There are situations where we may not need isolation at all, but based on dynamic variables on runtime we'll suddenly need to recycle workers for next tasks.

import { Tinypool } from "tinypool";

const pool = new Tinypool({
  // Workers are not isolated at "pool level"
  isolateWorkers: false,
  ...
});

// Task takes "environment: 'jsdom' | 'node'" as argument
const options = { environment: "jsdom" };

await pool.run("taskName", options) // workerId #1
await pool.run("taskName", options) // workerId #1
await pool.run("taskName", options) // workerId #1

// Change task options. Next tasks need to be run in new workers even though "pool level" isolation is disabled
options.environment = "node";

// Indicate pool to use new worker for next tasks. Could be something like:
// Imperative method "flushWorkers":
await pool.flushWorkers();
// Or new argument in the pool.run() method, e.g. "runInNewWorker":
options.runInNewWorker = true

// Eventually new worker should be used for next tasks and workerId should change
await pool.run("taskName", options) // workerId #2
await pool.run("taskName", options) // workerId #2
await pool.run("taskName", options) // workerId #2

Or maybe a isolationKey which would be used to see if given tasks can share the worker:

options.environment = "jsdom"; // Option for task
options.isolationKey = "some-key-1"; // Tinypool's option
await pool.run("taskName", options) // workerId #1
await pool.run("taskName", options) // workerId #1
await pool.run("taskName", options) // workerId #1

options.environment = "node"; // Option for task
// Worker ID is same as isolationKey did not change:
await pool.run("taskName", options) // workerId #1

options.isolationKey = "some-key-2"; // Tinypool's option
await pool.run("taskName", options) // workerId #2        <---
await pool.run("taskName", options) // workerId #2        <---
await pool.run("taskName", options) // workerId #2        <---

Some pitfalls to consider:

  • If there are on-going tasks, how would pool.flushWorkers instruct those ones to terminate once they have resolved?
  • When options.environment changes we only want to recycle the existing threads and not the new ones. Here the runInNewWorker would keep recycling all new threads which would be slow.
  • isolationKey might be easiest to implement and handle

Let's start by looking into pool.flushWorkers().

@sheremet-va
Copy link
Member

I envisioned something like pool.flushWorkers option:

await runIsolated()
for (const env of envs) {
  await pool.flushWorkers()
  await runSingleThread()
}

If we are looking into isolationKey, maybe it would be something like this:

options.isolationKey = 'all'
await runIsolated()
for (const env of envs) {
  options.isolationKey = env
  await runSingleThread()
}

@Aslemammad
Copy link
Member

or a filter function argument, that analyzes the arguments passed to the run method and returns a key (any string), and based on that string, starts grouping workers.

filter: (options) => {
  if (options.env === 'node') {
   return 'node' // workers with env=node will be in their own isolation
  } else {
   return 'else' // others will be in another isolation.
  }
} 

@AriPerkkio
Copy link
Member Author

filter: (options) => {
  if (options.env === 'node') {
   return 'node' // workers with env=node will be in their own isolation
  } else {
   return 'else' // others will be in another isolation.
  }
} 

This is also good idea but I think it has some design pitfalls as well. Mostly caused by the fact that workers are created before options are known, and held in idle state in the pool. The isolationKey API has similar issues.

I'll start by looking into pool.flushWorkers() method. This API would be most clear to end-users (developers) since they have to call it explicitly. Re-creating workers can be expensive, so a filter function could cause performance issues if developers did not understand its effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants