drush should re-boot Drupal after the update_N's and before the post updates

[hchonov]

Currently drush is executing the update_N's and the post updates in a single batch and in between is rebuilding the cache in a separate process, which is causing the static cache of the checksum service to be out-of-sync in the post updates. This is leading to all kind of bugs and there is a currently an issue on drupal.org - https://www.drupal.org/node/2851529.

[hchonov]

test_drupal_reboot_before_post_updates.patch.txt

Here is a test patch for Drupal 8 and the hash created in the both methods should be different - that should be enough verification that the problem is solved.

[weitzman]

The cache clear in question was added in #1603. Perhaps @jhedstrom can help with this issue as I'm busy on client work ATM.

[hchonov]

I've just took a look at how Drupal is running the updates and drupal is clearing the cache between the update_N's and the post updates - but there is one difference between Drupal and Drush executing the updates - Drupal clears the caches in the same process, Drush starts a new one - so we should probably only clear the caches in the same process?

[tstoeckler]

Just chatted with @hchonov, I think he is correct. Drush forks a process for the batch processing but then seems to process all batch operations in a single process (unless some memory limit is exceeded, but let's disregard that for now). The batch operations in this case are drush_update_do_one() and core's update_invoke_post_update() which both use $function() so they are executed in the same process. However, the batch operation in between those (i.e. in between normal updates and post updates) is drush_drupal_cache_clear_all() which does a drush_invoke_process('@self', 'cache-rebuild') so is executed in a separate process.

So it seems the correct fix is to replace the 'drush_drupal_cache_clear_all' batch operation with 'drupal_flush_all_caches' just like core is doing so that it happens in the same request.

Additionally there seems to be another problem that in fact drupal_flush_all_caches() is not sufficient because it merely invalidates the container but it does not rebuild it. That works for core's update.php as there the post updates will be performed in a new HTTP request and thus receive a new container. However, in Drush - as all of this will be happening in the same process - the container will be invalidated but never actually rebuilt.

To fix that @hchonov wants to open a new issue in core to make drupal_rebuild() (which internally calls drupal_flush_all_caches()) actually rebuild the container and update core's update.php to use that instead of drupal_flush_all_caches(). So we could do the same here. Alternatively we could somehow make sure ourselves in Drush that the container is actually being rebuilt.

[tstoeckler]

Oops, made had a number twist in the issue reference, that's why it doesn't show up here, but ...

Made a pull request for the above: #2620

Went with a separate method for now so that this works for all 8.* version regardless of whether/when the Drupal core problem described is fixed. (BTW, see https://www.drupal.org/node/2853152 for that)

[tstoeckler]

WIth PR mentioned above I get different hashes running drush updb with @hchonov's patch from above applied. Before they were identical. So it seems to work as expected.

[tstoeckler]

Oh, I should note that I had to have I should note that I had to have #2619 applied as well in order for the post updates to run at all.

[weitzman]

Fixed in PRs