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

Do keys generated by bull expire? #261

Closed
kentnek opened this issue Mar 2, 2016 · 28 comments · Fixed by #1331
Closed

Do keys generated by bull expire? #261

kentnek opened this issue Mar 2, 2016 · 28 comments · Fixed by #1331
Milestone

Comments

@kentnek
Copy link

kentnek commented Mar 2, 2016

Hi guys,

I'm using Bull in a personal project. After running for a while, I notice bull generated a lot of keys and Redis starts hogging lots of RAM. Will these keys expire?

Thanks!

@nibblesnbits
Copy link

At the moment there are no keys that require any sort of expiration. It is true that Bull will quickly accumulate keys in redis if you don't regularly clean jobs. Are you using Queue##clean at all?

@manast
Copy link
Member

manast commented Mar 14, 2016

It may be worth to implement an expire option for completed (and failed jobs), so the user is free to configure if he wants automatic key expiration after a certain optional time, or no expiration at all.

@nibblesnbits
Copy link

That shouldn't be too difficult. I'll get back into it and see if I can do this cleanly.

@kentnek
Copy link
Author

kentnek commented Mar 18, 2016

Nice!

@manast manast added this to the 1.1 milestone Mar 24, 2016
@manast manast modified the milestones: v3.0.0, 1.1 Mar 13, 2017
@manast manast modified the milestones: v3.1.0, v3.0.0 May 17, 2017
@CCharlieLi
Copy link

Any update here? I saw no expire option in the latest reference, and staled keys in Redis are still eating my RAM even with Queue##clean.

@manast
Copy link
Member

manast commented Dec 27, 2017

@CCharlieLi why don't you use the option removeOnComplete ? https://github.com/OptimalBits/bull/blob/master/REFERENCE.md#queueadd

@CCharlieLi
Copy link

Thanks for reminding @manast , I guess it's the same as queue.clean(0) (please correct me if I m wrong). I also wonder if there's a way to remove keys left by previous queue instances which were already shutdown. Or do I have to use script to delete them from Redis?

@manast
Copy link
Member

manast commented Dec 27, 2017

@CCharlieLi you should be able to instantiate the old queue and then call clean on it.

@carlhopf
Copy link

carlhopf commented Apr 12, 2018

It would be very useful to be able to set TTL, for example in an environment where Nodes go up and down all the time, with unique queue names for inter-server communication.

F.ex.: two AWS spot instances communicate with each other, each instance uses a unique named queue based local instance id. both instances get shut down at the same time by aws. remaining data on redis never expires, unless manually keeping track of instances (or queue) names for cleaning up.

(edit for better example)

@manast manast modified the milestones: v3.1.0, v3.5.0 Aug 18, 2018
@joe-at-startupmedia
Copy link

In my use case removeIOnComplete isn't an ideal option because I would like to keep track within a given time interval which records have completed. For example: I remove all completed queues after 2 weeks because by then I have seen and acknowledged they were completed. It brings peace of mind to know queues are being completed and not just purging them immediately after they complete.

@nidhhoggr
Copy link

nidhhoggr commented Sep 21, 2018

See my implementation here:
nidhhoggr@486eabb

Several Issues

  1. The injection of queue settings into the constructor job. Currently Job doesn't inherit settings from Queue in this manner. Job sets this.expiresIn from queue.settings.
  2. The order of arguments for lib/commands/moveToFinished-6.lua was shifted up one before the arguments from the 9 position used to avoid the next roundtrip. I though it was necessary to keep the argument next to the relative arguments for the particular job so offset 7 (before the even data) made more sense to me.
  3. The keys are expired but not removed from the respective sorted sets in redis (e.g. bull:[job_name]:completed) . One way to do this is to add a redis event listener for expired keys and remove the keys in the event handler. Another way is to write a maintenance script to flush the expired keys. The cons of the first approach is that the key is not technically expired and purged from redis until it is attempted to be accessed per documentation below:

https://redis.io/topics/notifications#timing-of-expired-events

@nidhhoggr
Copy link

Implementation to expire expired keys from a job redis key:
nidhhoggr@1f34aa0

@ritter
Copy link

ritter commented Sep 21, 2018

@nidhhoggr glancing at your implementation I have immediate concerns about jobId being expected and cast to be number/integer, my understanding is that jobId can also be a string.

@ritter
Copy link

ritter commented Sep 21, 2018

Also, this code is subscribing to every key matching ":expired", and probably should only be matching those set under bull's namespace, like: "bull:*:expired", and even then probably only for a specific queue?

Disregard that, I misread it.

@nidhhoggr
Copy link

@ritter Disregard what? These are both valid concerns, I was unaware there was a possibility of jobId as string with non-numeric keys. The issue with having a string as a key means that the jobId must be confirmed as an actual job entry of type hash, because bull has other keys with the convention bull:${queueName}:[string] that it uses to store completed, failed, active, stalled-check etc. So something needs to be devised to assert the jobId is an actual job before calling the removeJob.lua script with attempts to remove the entries by the jobId. regarding only watching for bull:*:expired that is a pretty simple fix:

          client.psubscribe(_this.toKey('expired'), function(err, result) {

The event handler also needs error checking as well. In conclusion, my implementation isn't PR ready but feel free to provide suggestions and/or take from it what you will to submit your own PR.

@nidhhoggr
Copy link

nidhhoggr commented Sep 21, 2018

Actually, regarding the expired keys events listener my example above would not work because the name of the keyspace event looks like this:

"pmessage","__key*__:*","__keyevent@0__:expired","bull:updateCauseTotalSavings:stalled-check"

When I attempted to listen for

client.subscribe("__keyevent@0__:expired") 

I wasn't getting anything so I changed it to the following with success.

client.psubscribe("*:expired") 

@jsardev
Copy link

jsardev commented May 27, 2019

This feature is really missing. For now, the only option to automate cleaning up the queue is to:

  • have a ioredis instance which updates the entries on completed and/or failed adding an EXPIRE
  • have a repeatable job which runs every X and cleans up the jobs.

This is a tedious task. I'd suggest adding an option to .add or extending removeOnComplete/removeOnFail to accept an object like: { expireIn: 30000 } instead of a boolean.

@manast
Copy link
Member

manast commented May 28, 2019

@sarneeh I will try to improve this in the following days.

@jsardev
Copy link

jsardev commented May 29, 2019

@manast But don't feel like you have to. It's open source, if I'd need it very much I'd do it myself. It's just a suggestion if you'd like to improve it somehow 😃 Your work already saves a dozen of hours creating such a solution (bull) manually 😃

@manast
Copy link
Member

manast commented May 29, 2019

I need it myself in some project :).

@manast
Copy link
Member

manast commented Jun 1, 2019

It is not really expiring all the keys, this would be a complete different story, we may implement it in the future for ephemeral queues.

@zzuhan
Copy link

zzuhan commented Dec 31, 2020

https://github.com/OptimalBits/bull/blob/develop/REFERENCE.md#queueadd

this removeOnComplete solved my problem,Mentioned by manast

realtimeCheckTaskQueue.add({}, {
    // keep the latest 1000jobs 
    removeOnComplete: 1000
})

@nmargaritis
Copy link

nmargaritis commented Jan 14, 2021

It would be very useful to be able to set TTL, for example in an environment where Nodes go up and down all the time, with unique queue names for inter-server communication.

F.ex.: two AWS spot instances communicate with each other, each instance uses a unique named queue based local instance id. both instances get shut down at the same time by aws. remaining data on redis never expires, unless manually keeping track of instances (or queue) names for cleaning up.

(edit for better example)

I also believe that this is a legit usecase. It happened to us as well to be using bull so that we avoid storing jobs locally when running on AWS nano, where memory is limited anyhow. Jobs are only for internal instance use, so when the application is scaling up and down by AWS, and as the queue name is given based on the instance, there are entries that remain forever on redis, not linked with anything.

I believe that an expires option makes sense to be there. However, it might not be the most optimal solution for the cleanup either.

We already use removeOnCompleted. However,, there might still be some jobs left on redis in this usecase.

@throrin19
Copy link

throrin19 commented Mar 9, 2021

In my case, I use Bull to execute CronJob agent.

In some cases, I may have too many agents being created for the number of tasks Bull can handle simultaneously. In that case, I would like to see the jobs that have been in the queue for too long simply deleted. Normally you can do this easily with Redis by playing the expire() function. Why not allow it with a JobOpts option? :

realtimeCheckTaskQueue.add({}, {
    // automatically remove job after 1 minute
    expire : 600000
})

Edit :

To avoid my problem, I made this code in my express APP :

const cleanInterval = setInterval(async () => {
    await Promise.all([
        realtimeCheckTaskQueue.clean(5 * 60 * 1000, 'wait'),
        realtimeCheckTaskQueue.clean(5 * 60 * 1000, 'delayed'),
    ]);
}, 60000);

Every minutes, I call Bull to clean wait and delayed messages oldest than 5 minutes

@manast
Copy link
Member

manast commented Mar 9, 2021

@throrin19 what is basically what would be needed by bull internally since there is no way to automatically expire the jobs being in different sets and lists.

@georgeroman
Copy link

https://github.com/OptimalBits/bull/blob/develop/REFERENCE.md#queueadd

this removeOnComplete solved my problem,Mentioned by manast

realtimeCheckTaskQueue.add({}, {
    // keep the latest 1000jobs 
    removeOnComplete: 1000
})

I got it working as well via this method. However, when adding the option on the queue itself it doesn't work. What I did was specify removeOnComplete: 1000 directly in defaultJobOptions when initializing the queue instead of having it defined in the creation of every job. Did anyone else have this issue?

@thanghv97
Copy link

https://github.com/OptimalBits/bull/blob/develop/REFERENCE.md#queueadd
this removeOnComplete solved my problem,Mentioned by manast

realtimeCheckTaskQueue.add({}, {
    // keep the latest 1000jobs 
    removeOnComplete: 1000
})

I got it working as well via this method. However, when adding the option on the queue itself it doesn't work. What I did was specify removeOnComplete: 1000 directly in defaultJobOptions when initializing the queue instead of having it defined in the creation of every job. Did anyone else have this issue?

I also got this issue

@sabinthomas
Copy link

See further discussion and the actual fix on #2265 . removeOnComplete was implemented in that MR with actual TTL/time based expire options.

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

Successfully merging a pull request may close this issue.