crop_update_8001 never completes

+++ b/crop.install
@@ -43,7 +43,8 @@ function crop_update_8001(&$sandbox) {
+  $sandbox['#finished'] = 1;

This won't work correctly in case of batch updates as it will mark batch finished after first iteration (even if not true).

More info, this should be a relatively simple fix.

On one of our sites, this update never completed because it kept getting a "divide by zero" error on line 48 of crop.install. This was because we haven't defined any presets/used crop yet. There needs to be a check for if there are more than 0 results to update, and exit if not.

Hii all, I ve seen that !! At begining my code are well for me @see https://www.drupal.org/node/2651994#comment-10773474

I Think We should perhaps a version of this while + foreach any car worked well and it could properly finish this .
I think in retrospect that the idea of ​​batch abusive think. Crop API is not yet stable and the number of fairly limited use for now , this fix was useful only for sites with orphan crop entities and this was the case before the onset of my developement with crop_file_delete ( ) . This update was therefore justified only for sites with crops before the onset of the delete () therefore my KNOWLEDGE few production sites ( except my projects ) .

What do you think @slashrsm ?

( I promote to major because we don't have an infinite updb for crop and we need to solve it today ).

For reason exposed before, I think this is the real and more clean patch to solve that problematic.

Sites with a lot of crops might experience timeouts with #7.

In which cases this happens? What about this solution?

+++ b/crop.install
@@ -24,26 +24,30 @@ function crop_update_8001(&$sandbox) {
+  if (empty($crops)) {
+    $sandbox['#finished'] = 1;
   }

This can't be work because $crops are never empty if we have already few valid crops (attached with file).

1. +++ b/crop.install
   @@ -24,26 +24,30 @@ function crop_update_8001(&$sandbox) {
   +      if (empty($files->execute())) {
   

   The real problem is due to that. You need to transform that into !empty($files->execute()) to unsure we HAVE orphans crops and we can delete it.


EDIT : My mistake ! Ok is good for that is good to (empty($files->execute())) because we delete crop if and only if the crop not have file linked.

2. +++ b/crop.install
   @@ -24,26 +24,30 @@ function crop_update_8001(&$sandbox) {
   +      $sandbox['current']++;
   

   That is a real voodo for me ! sandbox['current'] is same number of item_per_batch ! he is not incremented... i ve test to delete the first line $sandbox['current'] = 0 but no changes ...

This can't be work because $crops are never empty if we have already few valid crops (attached with file).

Script is loading all crops and checks file after that. Query should return empty result when all crops have been checked.

That is a real voodo for me ! sandbox['current'] is same number of item_per_batch ! he is not incremented... i ve test to delete the first line $sandbox['current'] = 0 but no changes ...

I do not understand. "current" is counting items that we processed. "items_per_batch" is number of items we will process in one batch. They are related when we want to calculate whether batch is done or not, but they represent two completely different things.

I can explain again:

If you var_dump $sandbox variables we have already sames number for "current"

For me i have

Total => string(4) "8750"
Current => int(100)
Performing crop_update_8001

In each updb ... it's not normal.

And for the query

  $crops = \Drupal::entityQuery('crop')
    ->sort('cid', 'ASC')
    ->range($sandbox['current'], $items_per_batch)
    ->execute();

$sandbox['current'] are 100 forever and not increment normally is that the problem for me :/ I ve test with you patch too and not works correctly the update_n is call again and again ...

This can't be work because $crops are never empty if we have already few valid crops (attached with file).

You're right I made a mistake when i ve write my message.

That can't be "need review" because in my cases not works :(

Edit: If i switch $items_per_batch to 4000 by example $current value is 4000 forever and we try to divide 8750 / 4000 on each updb and we can't finish batch because $sandbox['max'] / $sandbox['current'] is never 1 to finish process. In conclusion the problem is why $sandbox['current'] === $items_per_batch in each itteration. When we fire updb 3 x we need to have 300 not 100 in each updb.

I checked this again. I think that the main problem lies in wrong calculation of #finished. Updated patch attached.

Yata !!! It's ok now :) The batch mode works you rocks @slashrsm !! I ve test that patch with preprod site and crops are correctly clean in first updb and if i re fire updb i ve "No database updates required" :)

Commited ;) Thank a lot all.