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

Adding The unsaturated callback to the queue #868 #869

Closed
wants to merge 2 commits into from

Conversation

ecasilla
Copy link
Contributor

I needed this function to allow be to create a simple backoff notification for the producers of data on to a queue.

@@ -879,6 +882,8 @@
concurrency: concurrency,
payload: payload,
saturated: noop,
unsaturated:noop,
buffer: concurrency / 4,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the point of the buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the difference between the saturation point and a buffer point you want in order to say the queue is unsaturated ...

Here I'm saying once the queue has been saturated i want it to be at 25% - concurrency in order to say the queue is unsaturated..

Example
concurrency = 10 //workers
saturated = 10 //items in queue
unsaturated = 7.5 //items in queue or less..

@aearly
Copy link
Collaborator

aearly commented Oct 26, 2015

If you can rebase from master, I'll merge this.

@lucaswxp
Copy link

+1

@aearly aearly mentioned this pull request Jan 7, 2016
@ecasilla
Copy link
Contributor Author

ecasilla commented Jan 8, 2016

ok will do

@ecasilla
Copy link
Contributor Author

@aearly updated the branch

@lucaswxp
Copy link

Will this be merged into master?

2016-01-19 15:14 GMT-03:00 Ernie Casilla [email protected]:

@aearly https://github.com/aearly updated the branch


Reply to this email directly or view it on GitHub
#869 (comment).

@ecasilla
Copy link
Contributor Author

yes i assume it will is there another branch i should be making the PR into?

@aearly
Copy link
Collaborator

aearly commented Jan 20, 2016

We're currently going through a modularization phase (#996) so it would be more useful if you could merge it into that branch (modularization). You'll see there are lots of changes, so you'll likely have to re-do some of the work.

Sorry, things changed a lot since my last comment.

@ecasilla ecasilla closed this Jan 21, 2016
@lucaswxp
Copy link

This was closed without the PR?

@aearly
Copy link
Collaborator

aearly commented Mar 23, 2016

It was completed in another PR.

@lucaswxp
Copy link

This is already available on master or should I checkout another branch?

@aearly
Copy link
Collaborator

aearly commented Mar 23, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants