Old processed items are never deleted from the advancedqueue table, leading to potential memory and DB size issues over time.

Additionally, if Advanced Queue fails while processing an item (crash, PHP process killed, etc), the item will be left stuck in "processing" state forever.

Comments

ekes’s picture

I was wondering about this with #1998326: Add VBO support to change status. I have a working copy now that includes a ADVANCEDQUEUE_STATUS_DELETED the items themselves can then be periodically (set age) purged.

Does that sound like a way forward? I'll tidy the patch and post it then.

gbaudoin’s picture

StatusFileSize
new2.43 KB

Pruning the advancedqueue table on cron, with settings form in admin/configuration/system

gbaudoin’s picture

StatusFileSize
new2.47 KB

@ekes : Yes, the ADVANCEDQUEUE_STATUS_DELETED status would make more sense !

Re-roll with latest 1.x

mdupont’s picture

Assigned: gbaudoin » Unassigned
Issue summary: View changes
StatusFileSize
new2.82 KB

Re-rolled against latest version, taking into account that there is now a hook_cron() implementation. I've used the former patch on an older advancedqueue version and it worked well.

mdupont’s picture

Assigned: Unassigned » mdupont
Category: Bug report » Feature request
StatusFileSize
new4.61 KB

I improved the patch and added a new feature: in addition to allow for removing older successfully processed items, it can also release items that are in "processing" status and expired for a long time.

The use case is that if the PHP process crashes or is killed while processing an item, the item will stay in "processing" state forever and will never be picked up by another AdvancedQueue runner. Therefore I added a simple function that runs at cron time and releases "ghost" items stuck in "processing" state for a long time. That way these items will be back to the default state and will be able to be processed again.

mdupont’s picture

Issue summary: View changes
mdupont’s picture

StatusFileSize
new4.77 KB

I added a small change to purge old items in status ADVANCEDQUEUE_STATUS_FAILURE as well.

mdupont’s picture

Issue summary: View changes
freescholar’s picture

Hope this database issue is solved soon, we have a site that has a 10mb table and the site has hardly been used yet... Thanks for working on it!

deggertsen’s picture

My advancedqueue table had 5,936,010 rows and was 1.4 GiB. Definitely need this fix....

2 lines add whitespace errors with #7. However, that's a minor issue. Other than that the patch appears to work as expected. Thanks!

travisc’s picture

Hmm, I'm gonna try this patch, my table is 8 GB from 244911 rows

travisc’s picture

Some really nice improvements as far as UI goes, but i'm not certain that the settings are being respected, for instance

100 Number of successfully completed jobs to keep in the database.
Never Time to wait before an expired item is considered stale.

and i see 1200 rows in the advanced queue

rszrama’s picture

Testing this now. One minor nitpick is that the settings form suggests a number of successfully completed items to retain in the queue, but in practice the function will delete successful and failed items. Perhaps there should be a separate setting for failed items, as I'm more likely to want to retain failures for later examination than successes.

I'd also remove "configuration" from the menu item name. I may throw a patch in here that tweaks the interface text a bit.

rszrama’s picture

StatusFileSize
new5.08 KB

Patch attached moves the form builder function to an admin.inc, tweaks the menu item as I suggested, and changes the settings form to remove the description / fieldset (unnecessary) and indicate that any completed item (i.e. success or failure, for now) will be cleaned up.

artusamak’s picture

StatusFileSize
new4.33 KB

Just adding the code to apply the behavior to the drush command too. It would be sad to miss the fun there.

jweirather’s picture

If I only apply patch #15 to dev, I get a WSOD, can't find file advancedqueue.admin.inc

If I first apply #14 (which creates the file), then #15 won't apply clean.

I was under the impression that #15 would include/supercede #14?

Am I missing something?

THanks!

artusamak’s picture

StatusFileSize
new5.63 KB

You're totaly right!
Here is the proper fix!

jweirather’s picture

Thanks @Artusamak for the quick turnaround!

The patch in #17 applied clean to a fresh copy of dev.

Although I don't yet fully understand how the queue works, or how this patch prunes completed/expired items, it does appear to be working: where I used to have hundreds of thousands of "processed" queue items, those have been reduced to ~1000, which appears to correlate with my AQ settings.

I do have a couple of questions if anybody feels like helping out:

1) Are the settings on the admin page supposed to be minimums, maximums, or something else?

2) What is the trigger/process that invokes the code in this patch and causes queue items to be pruned from the table? In other words, is this a continuous process that runs as a part of advanced queue, or is it run as a separate cron job, only from CLI, etc?

Thanks again everybody for all your help!

artusamak’s picture

To answer your questions:

1/ It's a maximum of items to keep in the database.
2/ It's triggered by cron and if you want, there is a drush command to executed the queue processing in a background process. It can also be triggered when this command runs.

jweirather’s picture

@Artusamak: Thank you for the updates.

Sorry to be a pest, but to execute this cleanup from drush, is there a special command or option that I need to use, or does it just run automatically if you call advancedqueue from drush?

In other words, if I am already running advanced queue, every 15 minutes, like this:
drush @site.env advancedqueue --all --timeout=900
(via acquia cron)

Do I need to do anything else to enable the cleanup from this patch? Or is it already included in the normal advanced queue processing? I looked at the code in advancedqueue.drush.inc, and it appears to clean up the table before executing the queue, but I wanted to verify before I assumed.

Thank you again for your help!

artusamak’s picture

No extra command is required for the clean-up to take place. The command that you were running before will now take care of the clean-up (that was the purpose of my update to the previous patch actually).

If the patch worked for you i suggest that you update the issue status to Reviewed & tested by the community.

jweirather’s picture

Status: Needs review » Reviewed & tested by the community

+1 RTBC for the patch in #17

Thanks for the prompt responses, the patch, and the information. Much appreciated.

jweirather’s picture

I have a separate, and likely unrelated issue going on here: https://www.drupal.org/node/2692395, but I wanted to suggest that this patch (or a future patch) could also prune off unprocessed items more than "X time" old.

In my case, I had some items hanging around that simply were not processing and were several months old, and definitely should have already been processed. I had to manually remove those rows from the database in order to clear them up, and they may have been causing other issues with the queue.

I'm wondering if it might be helpful/good housekeeping to add a setting to config that would automatically clear up any unprocessed items that are older than 1 day/1 week/1 month/etc?

My understanding is if the queue is running properly, under a normal workload, queue items should never age more than a few minutes. Please let me know if this is not the case.

Also, while we're making a wish list, a "prune items now" button on the config page might also be helpful.

I'm happy to create this as a separate issue if this thread thinks that's best.

Thanks!

katharine_gates’s picture

Hi will this patch be put into next version of dev?

Kazanir’s picture

This looks great. The only change I made was to move the settings piece out to the cleanup_table wrapper function -- the inner functions are now pure functions which act on whatever parameter they are given.

  • Kazanir committed cc154a2 on 7.x-1.x authored by mdupont
    Issue #2054555 by mdupont, Artusamak, gbaudoin, rszrama, Kazanir:...
Kazanir’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

hgoto’s picture