updatedb should clear cache

committed.

In the current drush version, cache clear is removed again. Was there any reason for that? It seems it was removed in rev1.57.

The problem is that e.g. schema needs to have its cache updated after there was an updated performed. When updating with the Drupal UI, it does so. When updating with drush, it does not clear the cache. So e.g. new tables or table fields won't be recognized in drupal_write_record() or other drupal_get_schema()-related functions.

This might be only one problem of not clearing the cache.

Please see also #806190: drush updb removes themes under /profiles from the system table for another possible effect of not clearing the cache.

Put back that code. Looks to me like it was inadvertently removed.

Not quite there yet :)

As a result of putting that back in, Drush is now clearing caches twice sometimes. Patch at #893562: Caches are flushed too many times when running the updatedb command in Drupal 7

In Drush 4 and 5, this code has been removed again.

This time it was intentional (see code below), but it still has nasty side effects as described above. Modules definitely rely on this cache clearing sometimes; e.g. if your update function enables another module, that module's menu items won't be available until the cache is cleared.

So I think the "Should be unnecessary on D7" code comment below is wrong, and we need to figure out how to add this back.

***

Current code snippet from drush_core_updatedb():

  if (drush_drupal_major_version() <= 6) {
    // Clear all caches. We just performed major surgery.
    drush_drupal_cache_clear_all();
  }
  else {
    // Should be unnecessary on D7.
    // On D7 site-upgrade, this cache_clear was leading to:
    // Call to undefined function field_read_fields() in field_sql_storage.install line 17
  }

  drush_log(dt('Finished performing updates.'), 'ok');
}

Confirmed this is still a bug in the current verison. A module that adds a blank update function in order to have the menu rebuilt (for example if things in the module's hook_menu() are moved around), then after running drush updatedb, those menu pages now refer to invalid callbacks or locations, because the menu cache hasn't been cleared. This is super frustrating as an end user, and as a module developer writing updates.

I'm not sure if the inline code comment is still a concern (how old is that, maybe it's something that's already been fixed in core)? This problem will get worse if Drush doesn't clear the PHP storage on updatedb in Drupal 8.

What version of Drupal are you using?

Typical Drush requests *can't* clear PHPStorage. See #1899842: MTimeProtected Directory Grief

Sure, my example was not great for how this will be compounded in the future. I'm having this problem right now on a Drupal 7 site using both 8.x-6.x and 7.x-5.x, both latest from Git. The problem lies in the batch finished callback in commands/core/drupal/update.inc:

function drush_update_finished($success, $results, $operations) {
  // Nothing to do here. All caches already cleared. Kept as documentation of 'finished' callback.
}

In reality, there have not actually been any caches cleared. When we run update.php normally, the finished callback (update_finished() runs drupal_flush_all_caches(). This is what we're missing here.

Well, it seems that I/we am having trouble understanding cache clear flow on various versions of Drupal. In #7 Rothstein says that we double clear after that change it seems that we sometimes never clear. We want to clear exactly once. Patches welcome.

Sure. So caching was rightly removed from the finished callback in #893562: Caches are flushed too many times when running the updatedb command in Drupal 7 on Aug, 2010. But then on Dec of 2010, the remaining cache clear for Drupal 7 and above was removed with http://drupalcode.org/project/drush.git/commit/5125ce469320de3a3aea91957..., which leaves zero invocations of cache clearing for D7+ when you run updatedb. Patch incoming.

Patch against 8.x-6.x.

The error in the removed comments in #15 seem to refer to #894556: drush updatedb does something different to update.php (causes d6 -> d7 upgrade to fail). That's puzzling though since when this cache clear is called, the update should be done running. It's likely a core issue that is the root of the problem *while* the updates are running (#896698: Circular dependency references between field and field_sql_storage). I don't see any proof as to Drush being the cause, which is why I feel comfortable making the change proposed in #15.

@Greg - would it be easy for you to test site-upgrade with this patch applied? Just want to be sure we don't regress and cause a fatal in field api.

Yeah, I'm going to say this is working. I did most of an upgrade of a simple D6 site I have, and got stuck on the updatedb for admin_menu due to a postgres bug in its upgrade path. However, the update of core and most other modules was successful, so I think the issue from the comment is no longer a problem.

Committed to 5.x and 6.x