Deploy / Update Hook Enhancements and Bugfixes for Drush 9 #3006
Conversation
| } | ||
|
|
||
| // Create a temporary cache directory for this drush process only. | ||
| $cache_directory = sprintf('/mnt/tmp/%s.%s/drush_tmp_cache/%s', $site, $env, md5($uri)); |
There was a problem hiding this comment.
How is this functionally different from the existing code? It looks like this PR is assigning a cache directory based on site URI, whereas the existing site is assigning it based on db_role. Shouldn't that basically do the same thing?
There was a problem hiding this comment.
passing the directory as a cli prefix ensures that the child processes spawned from the current shell inherit the value. Exporting the value in the bash shell could still result in another process changing or overriding what should be a unique value. The db_role alone isnt enough to isolate the calls to cache rebuild in particular because on ACSF the live and update environments share a database, so env differentiates the two. It is more significant that that the cache directly is in /mnt as opposed to /tmp and that this directory is also created and deleted to avoid permissions issues and race conditions
There was a problem hiding this comment.
My understanding is that using export makes the variable available to all child / sub-processes. Ref: https://stackoverflow.com/questions/1158091/defining-a-variable-with-or-without-export
There was a problem hiding this comment.
the issue is that the value can then be modified by exec or other subprocesses of the blt and respective drush commands which does not prevent cache collisions on environments with multiple web servers. if you have suggestions on how to better solve this I am happy to refactor.
|
Please split this up into more atomic PRs so they're easier to review. |
|
@lcatlett @danepowell @ba66e77 I recently had to apply the .patch instance of this PR in an ACSF project to get sites installing and updating on PROD (the fix was recommended by Acquia support). Is there a plan to get this work merged into the main line of development? I'm concerned that the project is now supported by a patch that has no traction. |
|
thanks for following up from your Acquia support crit @lpeabody - you are spot on that these are critical fixes for updating to 8.6, site installs from config, multisite config split support, and deploy / updates to applications using multi-headed web servers (such as ACSF production). @ba66e77 and @danepowell what needs to be done to make this easier for you to review? I can of course put in multiple smaller PRs, but is there any other constructive feedback as to why this was not merged? |
| @@ -1,4 +1,4 @@ | |||
| #!/bin/bash | |||
| ##!/bin/bash | |||
There was a problem hiding this comment.
Extra hash here I had to comment out in order to get bash to run this.
|
@lcatlett @ba66e77 @danepowell Merge this PR please (after updating db-update.sh as indicated above). The above is extraordinarily nitpicky and it's preventing critical changes from making their way in. Also this has now happened to me TWICE (two separate projects), only manifesting itself once the code reached 01live. The first time wasn't a big deal because we hadn't launched any sites yet. This 2nd time however, 30+ sites went down for an extended period of time. This fixes a critical issue with with sites updating to 8.6 on ACSF. Please merge this PR now or actually provide some useful feedback - your customers are pissed and unhappy. PLEASE let me know if there's anything I can do to help. I'd rather not have to deal with this again. |
Fixes #2957 #3001 #3000 #3005
Changes proposed:
drush.ymltemplate with cache directory defaultTo test post-install and db-update locally or manually you would execute the same command outout by ACSF in the logs, substituting your site, environment, database role, and uri as needed:
CACHE_PREFIX='/mnt/tmp/sandbox.01live' \drush8 --root='/mnt/www/html/sandbox.01live/docroot' -l 'bash.sandbox.acsitefactory.com' scr --script-path='/mnt/www/html/sandbox.01live/factory-hooks/post-install' 'post-install.php'/mnt/www/html/sandbox.01live/factory-hooks/db-update/db-update.sh' 'sandbox' '01live' '[db_role]' 'bash.sandbox.acsitefactory.com'