I'm using drush sql-sync in a few places and get this warning:

"WARNING: Using temporary files to store and transfer sql-dump. It is recommended that you specify --source-dump and --target-dump options on the command line, or set '%dump' or '%dump-dir' in the path-aliases section of your site alias records. This facilitates fast file transfer via rsync."

There are a few automated jobs that may end up running at the same time and I'm worried that the files used in transfer are getting clobbered and/or that one job is dropping a table that the other job just created. It's not important to us to get the rsync benefits, but I wonder if I should use something else.

It's not convenient for me to use directories for these because then I'd have to create one directory per jenkins job which could be a lot of work to do on both sides of the transfer. Any suggestions?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greg.1.anderson’s picture

I will disclose one minor problem with using temp directories with sql-sync: the filenames are generated on the local machine, and used on the remote machine. This does mean that there is a chance that, in theory, there might be a conflict with a temp file. In practice, though, since Drush generates the temp file name with the php tempnam function, and since it includes the site alias name in the temporary file name, you are very unlikely to get a conflict (target site must be the same, and tempnam must generate the same filename in each process). If you don't care about potential rsync performance increases that might be realized when reusing the same dump file, then it is fine to ignore the warning message and just rely on the temporary files.

greg.1.anderson’s picture

Status: Active » Fixed
greg.1.anderson’s picture

Please re-open if you are actually seeing files getting clobbered. :)

greggles’s picture

Status: Fixed » Active

Thanks for the quick response.

The filename is always /tmp/sitealias.sql - we are using "drush version 5.8" and 5.9. Is your comment valid on that version?

greg.1.anderson’s picture

Assigned: Unassigned » greg.1.anderson

I tested #1 on Drush-6; I don't remember there being a difference in this code in Drush-5, but I'll take a look.

greg.1.anderson’s picture

I made a mistake in #1. Drush is behaving the same way in both Drush-6 and in Drush-5; the local files are being named via tempnam, but it appears that the remote file is being named after the remote database name. I think it used to work as described in #1, but was apparently changed at some point.

I'll need to investigate further before making a recommendation.

greg.1.anderson’s picture

This behavior changed quite a while ago, when drush_sql_dump_file() was enhanced to ask the remote system where the temp directory is. In the new code, tmpname is no longer used to generate the remote filename. There are a number of possible solutions here.

1. (Preferred) Fix drush_sql_dump_file() to use drush_tmpname() to generate the temp file; if necessary, get the filename with basename(), and concatenate it onto the directory path retrieved from the remote system. (Patches welcome)

2. Modify sql-sync so that the cache feature can be used with remote temporary files. The relevant line is:

        if (!drush_is_windows($source_os) && isset($cache) && !$source_settings['dump-is-temp']) {

Clobbering could still happen under this scenario if the simultaneous sql-syncs happened close enough together; however, they would have to be executing the remote ssh sql dump commands simultaneously, so this would be really rare. File locking could be added to make this even safer, if desired. (Patches also welcome.)

3. (Workaround) Add a 'dump' item to your remote site alias. Since alias files are executable php code, you could call a function that generated any sort of random tmpname you wanted. The disadvantage of this technique is that this code will run every time drush loads this alias file, whether or not the sql-sync command is being used, so you wouldn't want to use anything time consuming here.

greggles’s picture

Version: 7.x-5.x-dev » 8.x-6.x-dev
Status: Active » Needs review

What about exposing the 'dump' variable as a command line option?

It looks like drush_tempnam is discouraged from use and I'm not sure I see how to use that to get a temp name but not temp directory from that function. I've attached a patch that I *think* does what you're talking about in option 1 while also changing the existing call to drush_tempname to use the arguments in the order they are meant to be used.

greggles’s picture

FileSize
834 bytes
moshe weitzman’s picture

Category: support » bug

Sounds like a bug report. I'll leave this to Greg to tackle when he has time. Let's be patient as he is very busy with drush and non-drush work right now.

greg.1.anderson’s picture

#9 looks good, for the most part. The 'sql' should not be appended to the end of the filename unless we know that the file will be used remotely. Even then, having the right extension is a nicety, but not necessary. If it is necessary, we could write our own implementation of tempname, I guess. I'll try to find time to refine and commit soon.

greg.1.anderson’s picture

Simplified the function and insured that the suffix was only added for remote temporary files.

greg.1.anderson’s picture

Tests passing, so committed 1e7b006. Re-open if any further issues persist.

moshe weitzman’s picture

Status: Needs review » Fixed

change status.

greg.1.anderson’s picture

#12 broke for remote directories, where the specified path does not exist on the local machine. Fixed in 465c6a8.

greggles’s picture

I assumed this would be in 8.x-6.0-rc3, but it seems it's not.

At least, I did "pear install drush/6.0.0" and drush --version says I'm on 6.x but I don't have this code. Is there some way with pear to get this or should I just put a git checkout of the right branch into /usr/share/php/drush/ ?

Thanks!

greg.1.anderson’s picture

I think that the pear install modifies the 'drush' script. I'd recommend checking out to some other directory, and diff the drush script before replacing. I think keeping `drush` from the latest pear drush/6.0.0 would work with the latest code from 8.x-6.x, but I have not tried it.

greggles’s picture

OK, got it (thanks, greg.1.anderson and msonnabaum for pointers).

I did "pear uninstall drush/drush-6.0.0" and then "pear install drush/drush-6.0.0RC3" - I added a link to pear.drush.org to the drush project page.

I then did a table transfer and confirmed that it created the temp file with a unique-ish name.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.