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 a configurable delay if there are no jobs in the queue #52

Open
patricker opened this issue Feb 4, 2021 · 2 comments
Open

Add a configurable delay if there are no jobs in the queue #52

patricker opened this issue Feb 4, 2021 · 2 comments

Comments

@patricker
Copy link
Contributor

patricker commented Feb 4, 2021

With just a few task clients it's no big deal, but with a very large number of TaskClient's running, the CPU hit on Redis of all the clients checking for jobs continuously with no delay, because the queue is empty, is quite noticeable. What are your thoughts if I added a configurable Thread.sleep(nodatasleep) if info==null?

I started up a ridiculous number of TaskClients (~1800 instances spread across 18 VM's) as a test, and saw Redis start consuming 50% of the CPU :D

       private async Task ExecuteQueuedTask()
        {
            var (json, info) = await TaskQueue.SafeDequeue();
            if (info != null)
            {
                LogTaskStarted(info);

                try
                {
                    var now = DateTime.Now;
                    
                    await info.ExecuteTask();
                    
                    var completionSeconds = (DateTime.Now - now).TotalSeconds;
                    LogTaskFinished(info, completionSeconds);
                }
                catch (Exception e)
                {
                    LogTaskException(info, e);
                }
            } else { // some kind of sleep code would go here I guess 
            }
        }
@brthor
Copy link
Owner

brthor commented Feb 5, 2021

I'm thinking a general, configurable polling delay in the TaskRunnerThread rather than the ExecuteQueuedTask provides for a more consistent behavior and should also solve the problem.

Polling in a while true loop without any delay is overkill even for a small number of TaskListeners anyways.

A default delay of 100ms seems about right.

I realize that it still has the issue of polling a lot when there is nothing in the queue, but this is necessary to retain the responsiveness of the system.

In the long term taking advantage of Redis' pub/sub features could be the solution.

@patricker
Copy link
Contributor Author

Looking at the code, I see you had a variable for a PollDelay that wasn't being used. So maybe something like this: #53?

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

No branches or pull requests

2 participants