Bug 8847 - detect-renamed.diff update to ensure existence of directory for partial-dir
Summary: detect-renamed.diff update to ensure existence of directory for partial-dir
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.0.9
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-05 20:52 UTC by J.R.
Modified: 2015-07-12 19:46 UTC (History)
2 users (show)

See Also:


Attachments
replacement for patches/detect-renamed.diff (26.47 KB, patch)
2012-04-05 20:52 UTC, J.R.
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description J.R. 2012-04-05 20:52:27 UTC
Created attachment 7435 [details]
replacement for patches/detect-renamed.diff

When using --detect-renamed, rsync creates "partial-dir"s for use as a place to hold files being deleted as part of a rename.  These held-files are then used as the basis files for the files being renamed.  If the file is indeed being renamed, this use provides significant bandwidth savings in that the file does not need to be re-uploaded.

By default, the partial-dir is a directory called ".~tmp~" that will be created in the directory of the renamed file.

As described by an earlier developer of detect-renamed.diff <a href="http://lists.samba.org/archive/rsync/2007-December/019469.html">here</a>, there exist circumstances when the partial-dir cannot be created because the directory for it does not yet exist.  This is the case when a directory is renamed or a file is moved to a new directory.  In such cases, which are, in fact, quite common, detect-rename fails because the new directory does not exist when rsync tries to create the partial dir.  So, the file(s) in question are re-uploaded instead of being detected as renamed existing files.

The attached update to detect-renamed.diff addresses this problem by ensuring that the needed directories for partial-dir are created.  This is done by use of the equivalent of "mkdir -p" when creating the partial-dir.  A new function, do_mkdir_path(), is provided in syscall.c that recursively creates any needed directories.

With this change, detect-renamed properly detects files moved to new directories as well as directory renames.

The attached detect-renamed.diff replaces the current file.
Comment 1 Russell Black 2013-02-01 01:35:37 UTC
+1 for this patch.  It looks like a simple change that will result in a huge performance improvement in certain common situations.
Comment 2 Russell Black 2013-02-02 22:59:17 UTC
If it helps to evaluate the patch, here is the difference in the code between this one and the regular detect-renamed patch.  Seems pretty straightforward.

$ diff rsync-3.0.9\ 2 rsync-3.0.9\ 3
diff rsync-3.0.9 2/proto.h rsync-3.0.9 3/proto.h
307a308
> int do_mkdir_path(char *fname, mode_t mode);

diff rsync-3.0.9 2/syscall.c rsync-3.0.9 3/syscall.c
219a220,235
> int do_mkdir_path(char *fname, mode_t mode)
> {
> 	char fnametmp[MAXPATHLEN], *fnametmpptr;
> 	STRUCT_STAT st;
> 
> 	if (fname) {
> 		strcpy(fnametmp, fname);
> 		if ((fnametmpptr = strrchr(fnametmp, '/')) != NULL) {
> 			*fnametmpptr = '\0';
> 			if (do_stat(fnametmp, &st) < 0)
> 				do_mkdir_path(fnametmp, mode);
> 		}
> 	}
> 	return do_mkdir(fname, mode);
> }
> 

diff rsync-3.0.9 2/util.c rsync-3.0.9 3/util.c
1171c1171
< 		if (statret < 0 && do_mkdir(dir, 0700) < 0) {
---
> 		if (statret < 0 && do_mkdir_path(dir, 0700) < 0) {
Comment 3 bjquinn 2014-08-03 02:54:59 UTC
I'm going to do some testing, but it appears that this fix is probably in 3.1.1.

In the process of applying the modifications suggested here (basically, add the do_mkdir_path function and call it in place of do_mkdir in util.c/handle_partial_dir), I noticed that it appears that util.c/handle_partial_dir now calls make_path, which seems to be a wrapper around do_mkdir that intends to create the entire path, like do_mkdir_path did.

I'll do some testing to follow up, but I'm guessing that this bug can likely be closed.
Comment 4 Wayne Davison 2015-07-12 19:46:35 UTC
This got fixed in the patches for 3.1.x back in June of 2013.  It now uses the make_path() function.