[Bug]: RateLimitError() not working

[wootwoot1234]

Version

4.11

Platform

NodeJS

What happened?

The rate limiter doesn't move jobs to waiting when the Worker.RateLimitError() error is thrown. Even though the RateLimitError() has been thrown, the jobs move from delayed to active and continues to run jobs.

How to reproduce.

import { Queue, Worker } from "bullmq";
import IORedis from "ioredis";

const connection = new IORedis({
  host: process.env.REDIS_HOST,
  port: Number(process.env.REDIS_PORT),
  password: process.env.REDIS_PASS,
});

const queueName = 'rate-limit-test';
const queue = new Queue(queueName, {connection});

await queue.drain();
await queue.addBulk(
    Array.from({
        length: 2,
    }).map((_, idx) => ({
        name: "test-job",
        data: {
            idx
        },
        opts: {
            delay: idx * 5_000
        }
    }))
);

const worker = new Worker(
    queueName,
    async (job) => {
        console.log(job.data);
        await worker.rateLimit(60 * 1_000);
        throw Worker.RateLimitError();
    },
    // `concurrency` seems to be an issue if set it a bigger value such as 10 in this case
    // then 1st active job remains in active list and continue to retry even if the queue is being rate limited
    // and once the delayed job is time to run, it gets moved to active before getting moved back to waiting list
    // if set = 1, it works as expected
    // which is all active jobs should be moved to waiting list as the queue is rate limited
    { concurrency: 10, connection }
);

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[wootwoot1234]

Another easy way to see the issue is to only add 1 job and look at the logs, you will see that it continues to run the job and throw the Worker. RateLimitError() even though there is a 60-second rate limit set.

[wootwoot1234]

We created a complete repo here https://github.com/tmhao2005/bullmq-rate-limit

[roggervalf]

hey @wootwoot1234 from your example I'm seeing that limiter.max option is missing in the worker config as we check if that value is present in order to do a rate limit validation for the next job keys. Seems like in our docs https://docs.bullmq.io/guide/rate-limiting#manual-rate-limit we were missing to add that option as in our test cases https://github.com/taskforcesh/bullmq/blob/master/tests/test_rate_limiter.ts#L360-L363

[wootwoot1234]

Thank you. I will try that.

…

On Fri, Sep 22, 2023 at 10:10 PM Rogger Valverde ***@***.***> wrote: hey @wootwoot1234 <https://github.com/wootwoot1234> from your example I'm seeing that limiter.max option is missing in the worker config as we check if that value is present in order to do a rate limit validation for the next job keys. Seems like in our docs https://docs.bullmq.io/guide/rate-limiting#manual-rate-limit we were missing to add that option as in our test cases https://github.com/taskforcesh/bullmq/blob/master/tests/test_rate_limiter.ts#L360-L363 — Reply to this email directly, view it on GitHub <#2193 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADGOGLQ5HDB3IQ5EW2Z2QLX3ZVNJANCNFSM6AAAAAA5DNBOPE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wootwoot1234]

I've updated my code and the manual rate limiting is working but when the queue is manually rate-limited and delayed jobs should run, the jobs are not moved from delayed to waiting. Instead, they stay in delayed, and then when the manual rate limit is finished, they all go to active. I expect them to go to waiting while the queue is rate-limited. Is this the expected behavior?

[manast]

I think it is expected that they go from delayed directly to active. We improved how delayed jobs are handled, so now they do not need to go back to wait and then move to active, they go directly. But this should not affect the rate limiting, is it doing so?

[wootwoot1234]

The rate limiting seems to be working correctly now. I would prefer that they go to `waiting` because I have checks in place that notify me if there are too many jobs `waiting`. That way I'm notified if something is wrong with the server, etc.

…

On Mon, Sep 25, 2023 at 2:25 PM Manuel Astudillo ***@***.***> wrote: I think it is expected that they go from delayed directly to active. We improved how delayed jobs are handled, so now they do not need to go back to wait and then move to active, they go directly. But this should not affect the rate limiting, is it doing so? — Reply to this email directly, view it on GitHub <#2193 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADGOGOS4I3BJDRIUDJLPOTX4HZDDANCNFSM6AAAAAA5DNBOPE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[roggervalf]

hey @wootwoot1234, we are actually moving delayed jobs to wait, this is happening in the moveToActive script, but when the queue is rate limited, workers are not going to execute moveToActive, they are going to be in an idle state, then when the queue is not rate limited, workers will call moveToActive, this script promote delayed jobs (move them into wait state), but in the same script later we also move from wait to active, we only move 1 job to active, so the other promoted delayed jobs should be in wait or in prioritized state (if they have priority > 0)

[wootwoot1234]

Does that mean that if we manually rate limit a queue for 10 seconds, after 10 seconds, all the jobs that would have run are moved to waiting, then active but if we throw another 10 second manual rate limit, we will see a bunch of jobs waiting? In other words, if the queue continues to be manually rate limited the waiting jobs will continue to build, 10 seconds at a time?

…

On Mon, Sep 25, 2023 at 6:59 PM Rogger Valverde ***@***.***> wrote: hey @wootwoot1234 <https://github.com/wootwoot1234>, we are actually moving delayed jobs to wait, this is happening in the moveToActive script, but when the queue is rate limited, workers are not going to execute moveToActive, they are going to be in an idle state, then when the queue is not rate limited, workers will call moveToActive, this script promote delayed jobs (move them into wait state), but in the same script later we also move from wait to active, we only move 1 job to active, so the other promoted delayed jobs should be in wait or in prioritized state (if they have priority > 0) — Reply to this email directly, view it on GitHub <#2193 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADGOGJY7VXN74OVRNWXDTTX4IZJ5ANCNFSM6AAAAAA5DNBOPE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[roggervalf]

hey @wootwoot1234, yes, delayed jobs will be moved to wait after those 10 seconds, this is happening because we will use those 10 seconds to postpone any job execution https://github.com/taskforcesh/bullmq/blob/master/src/classes/worker.ts#L486, after that time we will call moveToActive command that it's the one that moves delayed jobs to wait