On Sun, Nov 22, 2009 at 7:01 AM, Hyrum K. Wright <hyrum_wri...@mail.utexas.edu> wrote: > > On Nov 21, 2009, at 5:00 PM, Ivan Zhakov wrote: > >> On Sun, Nov 22, 2009 at 1:37 AM, Hyrum K. Wright >> <hyrum_wri...@mail.utexas.edu> wrote: >>> >>> On Nov 21, 2009, at 4:35 PM, Ivan Zhakov wrote: >>> >>>> On Fri, Nov 20, 2009 at 10:06 PM, <hwri...@apache.org> wrote: >>>>> Author: hwright >>>>> Date: Fri Nov 20 19:05:42 2009 >>>>> New Revision: 882679 >>>>> >>>>> URL: http://svn.apache.org/viewvc?rev=882679&view=rev >>>>> Log: >>>>> Enable packing of revision properties into a mutable sqlite database. >>>>> >>>>> Patch by: Paul Querna <chip{_AT_}force-elite.com> >>>>> me >>>>> >>>> >>>>> iterpool = svn_pool_create(pool); >>>>> for (i = min_unpacked_rev / max_files_per_dir; i < completed_shards; >>>>> i++) >>>>> @@ -7260,9 +7496,20 @@ >>>>> SVN_ERR(pack_shard(data_path, pb->fs->path, i, max_files_per_dir, >>>>> pb->notify_func, pb->notify_baton, >>>>> pb->cancel_func, pb->cancel_baton, iterpool)); >>>>> - /* We can't pack revprops, because they aren't immutable :( >>>>> - If we ever do get clever and figure out how to pack revprops, >>>>> - this is the place to do it. */ >>>>> + } >>>>> + >>>>> + for (i = min_unpacked_revprop / max_files_per_dir; i < >>>>> completed_shards; i++) >>>>> + { >>>>> + svn_pool_clear(iterpool); >>>>> + >>>>> + if (pb->cancel_func) >>>>> + SVN_ERR(pb->cancel_func(pb->cancel_baton)); >>>>> + >>>>> + SVN_ERR(pack_revprop_shard(pb->fs, >>>>> + revprops_path, pb->fs->path, i, >>>>> + max_files_per_dir, >>>>> + pb->notify_func, pb->notify_baton, >>>>> + pb->cancel_func, pb->cancel_baton, >>>>> iterpool)); >>>> You have to write-lock repository since revision properties can be >>>> changed during populating SQLite database. For example if repository >>>> being synced using svnsync. >>> >>> We currently grab the write lock for the entire packing process (see >>> svn_fs_fs__pack()). >>> >> Oops, missed that fact. >> >> Anyway svnsync sometimes fails with error that revision property not >> found if repository packed in background. Probably this happened due >> the following race condition: >> 1. svnsync open FS using svn_fs_fs__open(), repository isn't packed so >> fs->min_unpacked_revprop == 0 >> 2. svnadmin packs revprop and updates min-unpacked-revprop to 1000. >> 3. svnsync changes revision property for revision 0, since repository >> already open and min_unpacked_revprop == 0 it tries to write >> properties to file 'revprops\1\0' instead of writing them to SQLite >> database. >> >> The same situation may happen when svnsync tries to get revision property. > FYI: I've added test for both get/set revprop while repository being packed in r883062.
> I see a few possible solutions: > * Re-read the min-unpacked-rev and min-unpacked-revprop values > whenever acquiring the write lock. Hmm, but the problem could happen to > readers, too, and those don't grab the lock. > * Wrap these files with a separate lock. Ugly, and probably a performance > hit. > * On error, re-read the min-unpacked-rev and min-unpacked-revprop values and > try again (if different). > > I'm leaning toward the last one, since it seems like a rather rare > occurrence, and > it wouldn't kill performance. Thoughts? > I don't see reasons why it shouldn't work, but I've more time to think about it. -- Ivan Zhakov VisualSVN Team
