[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: Mon, 5 Mar 2012 16:20:30 +0200

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.

>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Received on 2012-03-05 15:22:28 CET

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