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: terminateTimeout options that decides when slow/stuck worker termination should be error #49

Closed
AriPerkkio opened this issue Mar 3, 2023 · 1 comment · Fixed by #50

Comments

@AriPerkkio
Copy link
Member

While looking into vitest-dev/vitest#2008 I noticed that Tinypool's workers can get stuck. The await worker.terminate() here does not resolve.

await this.worker.terminate()

By adding a timeout wrapper around that call it can easily be seen. Every time this error pops up the hanging-process reporter of Vitest shows a worker and the process won't stop.

async destroy(): Promise<void> {
  const timer = setTimeout(() => {
    console.error(`[TinyPool] Worker ${this.workerId} failed to terminate`)
  }, 10_000);
  await this.worker.terminate()
  clearTimeout(timer)

I'm preparing a feature where Tinypool would accept a terminateTimeout that would be used to decide whether a worker is unable to terminate. In these cases the pool.run() method would throw an error. Vitest could then catch these and inform users which test file is stuck:

try {
  await pool.run(data, { transferList: [workerPort], name })
}
catch (error) {
  // Proper logger would be used here of course
  process.stdout.write(`\nVitest caught error "${error.message}" while running files ${files.join(', ')}\n`)
}

I've been able to reproduce the process hanging case of vitest#2008 with trpc's repository (:wave: @sachinraja). In their case two test cases come up:

  • Vitest caught error "Failed to terminate worker" while running files /x/trpc/packages/tests/server/adapters/fetch.test.ts
  • Vitest caught error "Failed to terminate worker" while running files /x/trpc/packages/tests/server/interop/adapters/fetch.test.ts

As soon as these two test cases are excluded the process does not hang. I can loop their 600 test cases 10 times with success. When the test cases are included, the process hangs after 1-2 runs.

@Aslemammad, @sheremet-va any ideas what could be preventing these workers from terminating? I'm starting to think it's always something the users test cases trigger and that the root cause it not in Tinypool or Vitest at all.

And any thoughts about adding terminateTimeout flag to Tinypool? I have some-what working setup ready but it would need some refactoring.

@Aslemammad
Copy link
Member

I do not know the root cause of the halt, though I believe this new option can be helpful.
Send a PR if you want, and we'll work on it from there together.

cc @sheremet-va for an opinion.

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 a pull request may close this issue.

2 participants