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.
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? -Hyrum
