Revert "Remove respawn after 50% memory exhaustion during batch proce…

[attrib]

…ssing. (#3909)"

This reverts #3909

Like @MarkCarver commented in the pull request

Removing this is not a BC "solution" for code that actually relies on this functionality (which is hard to get around in Drush due to the way batches are handled).

We currently have two scenarios where we depend on this.

1. search api reindex/index - Search API loads all entity in a batch process to index them, resulting in reaching the memory limit because of Drupal static caches. (https://www.drupal.org/project/search_api/issues/3036504)
2. batches in update processes - Its very common (happens also in core, e.g. taxonomy in 8.5) that a update is creating a batch because a lot of entities are updated. Same issue as above loading a lot of entities will fill Drupals static caches and resulting in reaching the memory limit.

I'm not sure if something like this should be removed, even if better ways for handling above cases are implemented (see https://www.drupal.org/project/drupal/issues/2577417). As the basic idea of using batches was to circumvent issues with memory limit (in cli) and max execution times (in browser). Removing this means I can't use batches anymore if I know I do something very memory consuming, meaning I need to reinvent something new for this case.

This code is broken. Since migrate already handles low memory, this code no longer serves its original purpose.

Maybe we could set the limit to 60% to not interfere with migrate batch?

[attrib]

Added new commit which fixes an error as it seems some more stuff changed in the meanwhile an only reverting that one commit didn't worked anymore.

Also changed the memory limit to 60% as proposed.

[weitzman]

Added new commit which fixes an error as it seems some more stuff changed in the meanwhile an only reverting that one commit didn't worked anymore.

I'm not aware of batch code changing. What error did you see?

[weitzman]

OK, I'll agree that this needs code needs to come back to Drush. Any chance you can figure out whats wrong in the issues #2762, #3117. Ideally this code comes back with any needed fixes.

[attrib]

@weitzman I'm not sure how it worked before in 9.5 (but it did..), but in _drush_batch_command there is no else at the end:

  if (_drush_batch_worker()) {
    return _drush_batch_finished();
  }

So if the drush didn't finished (like in the case memory limit reached) it didn't returned anything, which now results in the following error (when only reverting that one commit)

> In AbstractListData.php line 14:
>
>   Passed variable is not an array or object

Now I return ['drush_batch_process_finished' => FALSE] in this case, so _drush_backend_batch_process continues to run.

[attrib]

Could it be that #3117 is fixed with my latest commit and that some better error handling (AbstractListData, w/e) while calling new processes just made it clearer where the issue is? The issue there was created with 8.x and till than a lot of stuff changed, where you maybe have a better idea if some changes in the past could have changed the error from nothing and it stops to this validation error?

[attrib]

Can't reproduce #2762 - see comment there.

[weitzman]

OK, thanks for following. Merged.