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

async.queue and concurrency #747

Closed
fprivitera opened this issue Apr 17, 2015 · 6 comments
Closed

async.queue and concurrency #747

fprivitera opened this issue Apr 17, 2015 · 6 comments

Comments

@fprivitera
Copy link

if I change onthefly the q.concurrency (of var q=async.queue(... ) ) there is no change happening.

are there any other workarounds?

thanks

@aearly
Copy link
Collaborator

aearly commented Apr 18, 2015

Is this related to #512 ? As far as I know, there's no clean way to change the concurrency on the fly.

@fprivitera
Copy link
Author

no,
it is different : If I create a queue q with concurrency 5 and later I
change to q.concurrency=10, q.running() is still <=5.

On Sat, Apr 18, 2015 at 9:23 PM, Alexander Early notifications@github.com
wrote:

Is this related to #512 #512 ? As
far as I know, there's no clean way to change the concurrency on the fly.


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


*Filippo Privitera *| Head of Technology

Milan | Rome | New York | Shanghai

Beintoo Spa - Corso di Porta Romana, 68 - 20122 Milano - Italy - Office
(+39) 02.97.687.959

This email is reserved exclusively for sending and receiving messages
inherent working activities, and is not intended nor authorized for
personal use. Therefore, any outgoing messages or incoming response
messages will be treated as company messages and will be subject to the
corporate IT policy and may possibly to be read by persons other than by
the subscriber of the box. Confidential information may be contained in
this message. If you are not the address indicated in this message, please
do not copy or deliver this message to anyone. In such case, you should
notify the sender immediately and delete the original message.

@fprivitera
Copy link
Author

why we don't add a function on async.queue like this?

setConcurrency: function(c) {
        q.concurrency=c
        if (!q.paused && workers < q.concurrency && q.tasks.length) {
                var end=q.concurrency-workers
                    for (var i =0;i<end;i++) {
                        q.process()
                    }
                }
            }

@justincy
Copy link
Contributor

justincy commented Jun 1, 2015

@fprivitera is half right. The concurrency is only updated when new items are pushed onto the queue. That behavior can be fixed without adding the proposed setConcurrency function.

justincy added a commit to justincy/async that referenced this issue Jun 1, 2015
@justincy
Copy link
Contributor

justincy commented Jun 1, 2015

The concurrency is only updated when new items are pushed onto the queue.

I need to revise that statement to say "The concurrency can only be updated on the same event loop in which an item was added to the queue. And it can only be decreased, not increased."

I just added a failing test case. There was previously a test case for changing concurrency but it didn't correctly test situations where the concurrency can't be changed.

@aearly
Copy link
Collaborator

aearly commented Jun 2, 2015

I think @justincy 's fix is good enough for now. If you want the queue to immediately take into account the new concurrency, you can manually call q.pause(); q.resume() to add more workers after settings the concurrency..

@aearly aearly closed this as completed Jun 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants