Process queue via batch API

Have you posted the information on hook_queue_info() yet? I'd love to be able to batch my queues straight from this interface.

I'm also interested in this... any documentation yet?

Making this active, pending documentation...

Currently the 'Batch process' button only handles queues that have a definition in this module's hook_queue_info().

However, if a queue is defined in core's hook_cron_queue_info(), then a worker callback is defined for it, and therefore it's possible to process it in a batch operation too.

Here's a patch for that.

Updated patch -- last one would lead to batch process going on for ever if the queue had items claimed by another process.

I got all this implemented today and things were working but it didn't seem to be processing via Batch API properly as no matter how many items we processed the batch would saying processing 0 of 1 until the end.

To fix this some coworkers and I went a different route and also tried to simplify the batch processing options inside hook_queue_info().

We kept the existing functionality but added the ability to have an array key of "callback" on the batch key within hook_queue_info(). Then in queue_ui.pages.inc I see if that exists and call the user function:

          // If user just wants a callback then pass to callback function.
          if (isset($batch['callback'])) {
            $max = $queue->numberOfItems();
            for ($i = 0; $i < $max; $i++) {
              $operations[] = array('queue_ui_batch_process_operation', array($batch['callback'], $queue));
            }
            unset($batch['callback']);
          }

Basically all we are doing here is adding an operation for each item in the queue at this point. This allows the batch API to process Item x of n (5 of 100) for instance based on how many items were in the queue at the start.

Inside queue_ui_batch_process_operation we claim and Item and call the callback function.

function queue_ui_batch_process_operation($callback, $queue, &$context) {
  $item = $queue->claimItem();

  // No items left to process so finish the batch.
  if (empty($item)) {
    $context['finished'] = 1;
    return FALSE;
  }
  if (!function_exists($callback)) {
    drupal_set_message(t('Callback for batch process !callback not found.', array('!callback' => $callback)), 'error');
    return FALSE;
  }
  $callback($item, $context);
  $queue->deleteItem($item);
}

If we run out of items it errors out gracefully and deletes items as it passes through this allows a big change in the callback function. Rather than dealing with the $context['sandbox'] and working through items as shown in queue_ui_batch_test we do a much simpler step through in queue_ui_batch_simple_test:

function queue_ui_batch_simple_test($item, &$context) {
  // Perform processing operation here if this were REAL.
  $context['message'] = t('Processing !value', array('!value' => '[value_goes_here]'));
}

And we set a message if we want. This streamlines the code a bunch for the batch processing but still keeps the old code working as well.

If this should be a separate issue let me know but it is based upon the patch already applied in this ticket.

> no matter how many items we processed the batch would saying processing 0 of 1 until the end.

That's because $queue->numberOfItems() is not a reliable count.

I could modify my patch to at least show how many items have been processed so far -- we just can't precisely say out of how many.

            $max = $queue->numberOfItems();
            for ($i = 0; $i < $max; $i++) {
              $operations[] = array('queue_ui_batch_process_operation', array($batch['callback'], $queue));
            }

That is going to fail on one point -- numberOfItems() is unreliable.

And also, putting potentially hundreds (or thousands!!!) of operations into a batch strikes me as a really bad idea. I don't even know if the batch API will have enough space to handle that in its storage table. Operations are meant to be slim.

And the whole point of having a queue is that the queue holds the data. Here you are treating the batch as a queue all over again!

Hi, joachim. "Operations are meant to be slim" - agreed! In the patch suggested by kbasarab (disclaimer: also authored by me), each operation is *very* slim: it only processes one queue item per operation. Batches (not individual operations) are meant to be very large: their data is stored in the "batch" table and is only limited by the memory and timeout settings of your server when creating the batch.

I think you may misunderstand the idea behind batch processing. Having statically defined operations is very limiting: in order to achieve any "batch processing" (where execution is spread over multiple requests to prevent memory and timeout errors) with only *one* operation, you have to be fairly expert with manipulating the $context array provided by BatchAPI to trick it into continuing to execute your same operation over and over.

The *correct* approach for using BatchAPI is to break each individual task into separate operations. Now, you could do this replacing the "operations" array in the queue_ui info hook with a user-defined callback that defines the batch operations, or you could accept kbasarab's patch which is much more elegant and handles much of the overhead automatically.

To sum up, processing an entire queue in one operation is not batch processing: without some expert manipulations of the $context array, all your execution will take place across one request, and will almost certainly run up against timeout and memory errors. The basic premise of batch is to spread your execution across multiple operations, which would need to be defined by the user or handled automatically as kbasarab has proposed.

Thanks,
Derek

PS - regarding the "Processing x out of y" message: in the original queue_ui code (before kbasarab's patch), the message is not defined by $queue->numberOfItems()... it is simply a reflection of the number of operations in the batch. Where the number of operations is defined statically in the user's info hook implementation.

> To sum up, processing an entire queue in one operation is not batch processing: without some expert manipulations of the $context array, all your execution will take place across one request, and will almost certainly run up against timeout and memory errors.

My patch guards against that -- it processes as many as it can in the batch operation, then hands back to Batch API.

Having one batch operation which runs multiple times by setting $context to say it is not finished is exactly how Batch API should be used.

Think of importing a CSV file with a batch operation: you have the one operation which you place once in the batch, and it repeats until it's done. Your approach is equivalent to placing a copy of the batch operation in the batch array for each line of the CSV!

I think it's weird to have the list of items a) in the queue and b) in the list of operations passed to batch API. It's a straight duplication.

Here's the updated version of my patch with the progress message.

Ok I think we jumping all over the board a bit right now.

Recaps:
Original Patch:
Pros

• Removes duplication of queue list vs batch list
• Prevents PHP timeouts with hardcoded 20 in user for loop to retrigger batch

Cons

• Requires end user to add a function with $context['sandbox'] and altering $context array.
• Could confuse user that operations are only happening on one request instead of multiple due to the 0 of 1 (We tested this to ensure it won't timeout)

New additions
Pros

• Simplifies end user implementation. All they need to worry about is a processor callback for the item.
• Easier to tell that multiple items are processing during batch.
• Still keeps all functionality of original patch intact, simply adds a simpler solution

Cons

• Duplicates list of queue into a batch
• Doesn't allow as much manipulation of $context array for advanced usage

Another option could be to abstract some of the advanced $context and sandbox stuff out to the queue_ui module instead of in the custom user function and keep one operation instead of multiples.

As a subtask to this we will want to document out how to implement these. For us to implement it required reading the code to see what was happening and how to setup our functions.

In summary I think there is room for two options: An advanced one that allows for altering the $context and performing batch as you wish (for advanced users) and a simpler option for users that just want to have a callback function to process data.

I'm sorry I don't quite understand the last comment.

Neither of these seems to be about my patch.

Hiya, joachim.

Re:

Having one batch operation which runs multiple times by setting $context to say it is not finished is exactly how Batch API should be used.

I disagree entirely. Think about bulk deleting content from the admin/content page. Or bulk deleting users from the Users Administration page. Or any Views Bulk Operation actions. They take each individual item you've selected and pass them one-by-one (that is to say, in separate "operations") to the appropriate handler (node_delete, user_delete, etc.).

When you want to process different types of items in the same batch run (such as processing two different queues in the same batch run), you make separate calls to batch_set() before exiting the form submit handler. When you do this, the user gets a nice "Processing x out of y" for each item in the first queue, and then the bar resets when it automatically starts processing the second queue, "Processing 0 out of Z [items in the second queue]."

When using this method, I would suggest including the Queue name in the $context['message'] variable. Since we're almost certainly going to pass the $context variable to the user callback function, the user could override the message if they wish.

Re:

Think of importing a CSV file with a batch operation: you have the one operation which you place once in the batch, and it repeats until it's done. Your approach is equivalent to placing a copy of the batch operation in the batch array for each line of the CSV!

I can see your point about importing a CSV file this way. Doing it all in one operation is a valid method, but I don't think it's best method for processing items in a system queue, as it places an unnecessary burden on the user. The method suggested by kbasarab and I makes it much simpler for the user to implement: their callback only has to take care of processing one queue item and doesn't have to worry about keeping track of max/finished/etc.

Also, I can see your point about the ugliness of including the $queue object in each operation call. (I'm not sure if BatchAPI stores the whole object and it's data or just a reference). There are two ways around this, if you like: (1) pass only the Queue ID instead of the whole queue object to our queue_ui handler, or (2) In the form submit, retrieve and delete each item in the queue, passing only the retrieved item into each operation call.

Anywho, those are my 2c, for what they're worth. Sorry for hijacking your patch =p

> I disagree entirely. Think about bulk deleting content from the admin/content page. Or bulk deleting users from the Users Administration page. Or any Views Bulk Operation actions. They take each individual item you've selected and pass them one-by-one (that is to say, in separate "operations") to the appropriate handler (node_delete, user_delete, etc.).

I'm afraid you're wrong there:

function node_mass_update($nodes, $updates) {
  // We use batch processing to prevent timeout when updating a large number
  // of nodes.
  if (count($nodes) > 10) {
    $batch = array(
      'operations' => array(
        array('_node_mass_update_batch_process', array($nodes, $updates))
      ),

Single batch operation, which is given a list.

> as it places an unnecessary burden on the user. The method suggested by kbasarab and I makes it much simpler for the user to implement: their callback only has to take care of processing one queue item and doesn't have to worry about keeping track of max/finished/etc.

I think you've misunderstood what I am proposing.

Nobody needs to write a batch API operation callback. My patch provides that.

You write a **queue worker callback**. That processes **one single queue item**.

Been playing around with an early version of the patch ... this will be a great addition! I'm looking forward to it.

Thanks everyone for comments and patches on this issue.

I've not yet had a chance to test fully so I've only reviewed the code in the various patches so far, but I think the patch in #13 is looking like the way to go.

My take on the Batch API single operation vs multiple operations debate is that what we're actually trying to do here is create a batch of batches (because a user could have ticked multiple queues on the Queue UI screen to be processed by Batch). This would result in multiple operations being defined - each with a different queue name. In this scenario the "processed x of y" is the number of queues that have been processed, not the number of items on those queues.

Within each queue batch (mixing metaphors a bit here...) we're processing the queue for as long as is defined in the hook_queue_info or hook_cron_queue_info - being consistent with how queues are processed by cron by processing as many as possible in that time.

On that note, I think the action on #13 is to confirm tests that selecting multiple queues to be processed by batch doesn't break by setting $context[#finished] to 1 within an individual operation [I will try and do this myself later, but would be great if someone could assist and upload some test evidence to this issue]

> selecting multiple queues to be processed by batch doesn't break by setting $context[#finished] to 1 within an individual operation

Ah I think you have a point there.

Setting $context['finished'] = 1; tells the batch system that the current operation is complete.

That means the current operation in the list of operations passed to batch_set().

Which means that this is wrong:

+          $batch = array(
+            'operations' => array(
+              array('queue_ui_cron_queue_batch_process', array($name, $cron_queues[$name])),
+            ),
+          );

because we have only one operation for all the queues.

What we should have instead is:

+          $batch = array(
+            'operations' => array(
+              array('queue_ui_cron_queue_batch_process', array($queue_name_1, $cron_queues[$queue_name_1])),
+              array('queue_ui_cron_queue_batch_process', array($queue_name_2, $cron_queues[$queue_name_2])),
+            ),
+          );

(but obviously built up with a loop, this is just to demonstrate)

No, wait, I've got that all wrong :(

The code in full, after the patch is applied, is:

      foreach ($queues as $name) {
        if (isset($defined_queues[$name])) {
          $batch = $defined_queues[$name]['batch'];
          // Add queue as argument to operations by resetting the operations array.
          $operations = array();
          $queue = DrupalQueue::get($name);
          foreach ($batch['operations'] as $operation) {
            // First element is the batch process callback, second is the argument.
            $operations[] = array($operation[0], array_merge(array($queue), $operation[1]));
          }
          $batch['operations'] = $operations;
          // Set.
          batch_set($batch);

          continue;
        }

        if (isset($cron_queues[$name])) {
          $batch = array(
            'operations' => array(
              array('queue_ui_cron_queue_batch_process', array($name, $cron_queues[$name])),
            ),
          );
          batch_set($batch);

          continue;
        }
      }

So each queue gets its own batch_set().

> selecting multiple queues to be processed by batch doesn't break by setting $context[#finished] to 1 within an individual operation

Given that a batch operation callback forgets its value of $context['finished'] **within the same batch operation** we can safely say that in a new batch it'll be fine.

It would be really great to see this patch committed. It makes this module tremendously useful for things like updating API module's data.

It seems like you are a bit stuck over here... Maybe let someone independent review this? Mr Queue API for core is Msonnabaum (at least for D8). Maybe ping him? I tested Joachim patch which works fine (doesn't mean there is something wrong with the other one)

I'm going to eat my words. @joachim your approach is lovely. I'm uploading a new patch and interdiff for a slight change that adds a try/catch to make it more inline with how cron processes the queue items (if the processor throws an error, catch it and leave the item in the queue).

EDIT: Just realized maybe that should be a drupal_set_message() instead of (or in addition to) the watchdog(), since this is user-facing Batch API and not cron.

I haven't actually implemented the processor with this patch; I'll be doing that shortly. Anything else needed before RTBC?

:)

Catching exceptions thrown by a queue worker was IIRC a recent addition to core. Good call :)

drupal_set_message() would be a good idea though, as it's a user-facing system, as you say, so setting back to needs work.

Is it just me, or is there an issue with the watchdog() call in core handling of cron-processed queue items? Seems like watchdog('cron', $e) will result in

Recoverable fatal error: Object of class stdClass could not be converted to string.

New interdiff and patch attached.

Looking at https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_cr... , it's watchdog_exception('cron', $e).

+++ b/queue_ui.module
@@ -305,8 +305,16 @@ function queue_ui_cron_queue_batch_process($queue_name, $cron_queue_info, &$cont
+      watchdog_exception('queue_ui', $e->getMessage(), NULL, WATCHDOG_ERROR);

The parameters for watchdog_exception() are incorrect: https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/watchd...

Nearly there! :)

Doh, that's why the function signature didn't look right. I thought it was watchdog() and not watchdog_exception(). New patch and interdiff attached.

Thanks!

Patch looks good to me, though as parts of that are mine I probably shouldn't set to RTBC.

Thanks for the work on this, from a quick look it all looks good. I've got some time set aside on Friday to test this and get it merged in.

I tested against 7.x-2.0-rc1, works great. Only comment, progress bar does not update or show progress as queue processed.

> progress bar does not update or show progress as queue processed.

IIRC that's because of the way Batch API shows progress -- the progress bar updates for each operation callback, no matter how runs it does.

Works (if you have enough memory ;-))!

The reason that the processing doesnt show progress on slow jobs is because the queue_ui_cron_queue_batch_process function is a copy of the cron_queue_batch which processes X number of items per run, technically by time allowed.
So the status is only updated when this function returns, but it only returns once the time has run out or the number of items to process is 0.
The way the patch was written, and the batch processing works, all the items in the queue will be processed (unlike the cron version which only processes as many items as it can in the time allowed).
So to show progress "live" the function should be run 1 item per run so that the progress bar is updated relatively quickly.

By simply adding a break to end of the while loop in that function we can force 1 item per function call, and thus the progress bar is now updated per item processed.

I also updated the message that is put out in the function to say how many items total:

$context['message'] = t('Processed !count of !total items in queue @name.', array(
'!count' => $context['sandbox']['progress'],
'@name' => $queue_name,
'!total' => $context['sandbox']['max']
));

I've installed latest dev and patched with above, when I click batch process the screen just refreshed quickly and nothing happens.

Cheers Dan

The patch still applies cleanly to the latest dev and still works for me. I would put the "message"-Issue in a separate task and would +1 a commit of #31.

@danharper, maybe the patch did not apply correctly in your case.

Tested #31 against latest dev "7.x-2.0-rc1+3-dev" successfully.

Bump.

This has been in RTBC for almost 2 years. Any chance on feedback and moving back to NW or just commiting?

I needed to use this module and only with this patch can I run the batch path.

See image.

+1 on moving forward.

Any idea when this will be released?

I believe is fair to mention that there is the Queue UI Improved module that also provides the batch process with few extra improvements.

Warning:
It's not possible to use queue_ui in conjunction with queue_ui_imp, because it will cause module's name space collisions issue.

Why?
At that point in time I've just added one of the patches was added in here and a few features right on queue_ui source code and then added my proposal for a new Queue UI release version (not that new right now) that is still without reply.

@See (https://www.drupal.org/project/queue_ui/issues/2295929).

I hope this might help someone.

Pushed to 7.x-2.x
Thanks