[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Tue, 6 Mar 2012 09:44:36 +0200

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<danielsh_at_elego.de> writes:
> >>
> >>>Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> >>>>Daniel Shahaf<danielsh_at_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.
Received on 2012-03-06 08:45:28 CET

This is an archived mail posted to the Subversion Dev mailing list.