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 needDrain property #126

Closed
ronag opened this issue May 12, 2021 · 8 comments · Fixed by #368
Closed

Add needDrain property #126

ronag opened this issue May 12, 2021 · 8 comments · Fixed by #368
Labels
enhancement New feature or request

Comments

@ronag
Copy link
Collaborator

ronag commented May 12, 2021

The current way to determine whether a pool needs a drain event before queueing more tasks is a little awkward. Can we add a needDrain proprerty?

if (pool.needDrain) {
  await EE.once(pool, 'drain')
}
pool.runTask()
@ronag
Copy link
Collaborator Author

ronag commented May 12, 2021

When does 'drain' actually get emitted? Is it when the queue is empty or when the queue.size < queue.capacity?

@jasnell
Copy link
Collaborator

jasnell commented May 12, 2021

Currently when the queue is empty. The plan has been to go back and revisit that but haven't been able to get to it yet.

@ronag
Copy link
Collaborator Author

ronag commented May 13, 2021

That’s doesn’t sound optimal.

@jasnell
Copy link
Collaborator

jasnell commented May 15, 2021

Yeah, it's not the most optimal approach but I haven't really had the opportunity to go back and figure out a more optimal strategy.

@jasnell jasnell added the enhancement New feature or request label May 15, 2021
@metcoder95
Copy link
Member

Which should be the optimal approach?

@metcoder95
Copy link
Member

Hey @ronag, I'm planning to revisit this, do you have any suggestion for an optimal approach?

@ronag
Copy link
Collaborator Author

ronag commented Jun 19, 2023

Not really. But I do think #348 is a duplicate of this.

@metcoder95
Copy link
Member

I believe that we can have two action paths here:

  1. We can maybe start by adjusting the drain method to be triggered once as soon as the queue.size < queue.capacity, in that way we enable continuing work as soon as there is spare capacity to schedule more tasks (this will translate into a major, so we will need to point to next branch for this one)
  2. Adding the needsDraincan be used as soon as the maxQueue has been reached. That should cover the requirement to understand when the pool needs to be waited for being drained until schedule a new task

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