[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: Blair Zajac <blair_at_orcaware.com>
Date: Tue, 06 Mar 2012 07:00:04 -0800

On 03/05/2012 01:57 PM, Stefan Fuhrmann wrote:
> 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?

We (Sony Imageworks) have a custom built Subversion server that does
2,000 RPCs/sec, so we LRU cache svn_fs_t, svn_repos_t, and svn_fs_root_t
to avoid filesystem IO.

Blair
Received on 2012-03-06 16:00:42 CET

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