Stefan Fuhrmann wrote on Mon, Mar 05, 2012 at 22:57:30 +0100: > On 05.03.2012 15:20, Daniel Shahaf wrote: > >Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000: > >>Daniel Shahaf<danie...@elego.de> writes: > >> > >>>Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000: > >>>>Daniel Shahaf<danie...@elego.de> writes: > >>>> > >>>>>You're right, I misread the code: I mistakenly thought line 2867 will > >>>>>always re-read the revprop-gen file from disk. How about: > >>>>> > >>>>>Index: subversion/libsvn_fs_fs/fs_fs.c > >>>>>=================================================================== > >>>>>--- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002) > >>>>>+++ subversion/libsvn_fs_fs/fs_fs.c (working copy) > >>>>>@@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs, > >>>>> fs_fs_data_t *ffd = fs->fsap_data; > >>>>> if (ffd->format>= SVN_FS_FS__MIN_PACKED_FORMAT) > >>>>> SVN_ERR(update_min_unpacked_rev(fs, pool)); > >>>>> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path, > >>>>> pool)); > >>>>>+ SVN_ERR(read_revprop_generation(fs, pool)); > >>>>> err = body(baton, subpool); > >>>>> } > >>>>> > >>>>That looks like it works. But it only works if all writers update the > >>>>generation file so this whole caching scheme requires an FSFS format > >>>>bump to exclude 1.7 and earlier. > >>>+1 > >>> > >>>(It was pointed on IRC, too.) > >>If we were to clear the cache after taking the write lock, either > >>unconditionally or if the revprop generation has changed, then we would > >>become compatible with pre-1.8 style writers. > >> > >>Of course this does highlight a change in svn_fs_t behaviour. A long > >>lived svn_fs_t may never see revprop updates as the cache never expires. > >>The cached values might get pushed out if the process (thead?) reads > >>other revisions, to get reasonably up-to-date values the caller must not > >>hold svn_fs_t objects too long. > >Which is a regression --- even a process that does proplist() in a loop > >should eventually see changes. > > Hm. I would expect the live of a fs_t to be limited > by the life of the RA session. At least for our CL > client, this would not be a problem. > > For TSVN dialogs like the repository browser that > keep client contexts open for a long time, it all > hinges on how long the RA session is kept open > by the SVN libs and whether the client has any > control over it. > > Can anyone comment on that? >
I believe you cannot assume svn_fs_t's will be short lived, and _certainly_ cannot assume that the access pattern to an svn_fs_t will be in any way related to random clients having random RA sessions; for all the FS API knows, svnserve is a daemon that calls svn_fs_open() once per reboot. And yes, people do this. > -- Stefan^2.
