[PATCH 3 of 3] introduce filenamelog repository layout

Adrian Buehlmann adrian at cadifra.com
Tue Jul 22 14:09:54 CDT 2008



On 22.07.2008 17:29, Matt Mackall wrote:
> On Tue, 2008-07-22 at 10:12 +0200, Adrian Buehlmann wrote:
>>> Also note that in the rare occassions when we walk the file list, we're
>>> typically calling stat() to get the file list.
>     ^^^^^^^^^^^^^^^^^^^^^^^^
> 
>>> - storefiles() or filelog.files() skips reporting files that don't exist
>> This assumes we stat for the entries to validate the entry before
>> using it.
> 
> The only user of this thing that matters performance-wise is
> store.walk() as called by streamclone, which is already statting all the
> files to determine their size. 
> 
>> I currently fail to see what would justify doing such a drastic
>> thing.
> 
> It removes all the transaction complexity and completely hides the whole
> issue from everything outside of the opener for the new store.

There is no need to do this, as my patch demonstrates.

This does not justify to abandon the append-only nature
of the filenamelog.

>>> - we rewrite the list if we find errors (extra files)
>> This assumes we have a write lock when reading the filenamelog.
>> This might not be true in the future. For example, in the future,
>> we might be able to implement a lock-free streamclone.
> 
> It's not likely that streamclone can be made lock-free. But take a look
> at how localrepo.status works, starting at:
> 
> 	        # update dirstate for files that are actually clean             
> 
>>> And I think that makes it all transparent to the higher levels?
>>>
>>>>>> +    def hasfnlog(self):
>>>>>> +        return False
>>>>>> +
>>>>>> +    def readfnlog(self):
>>>>>> +        res = {}
>>>>>> +        return res
>>>>> We may be able to kill these.
>>>> verify needs to know if there is a filenamelog that needs to be verified.
>>>> (at least that was my thinking when I created these methods.)
>>> It doesn't, actually. It needn't know anything about the store. All it
>>> needs to know is that for every file the changelog mentions, there is a
>>> file returned by storefiles() and vice-versa. And that test can happen
>>> on any store we can call storefiles() on.
>> There's a new file 'filenamelog' in the store we want to trust to have
>> the filenames of all files in the store.
> 
> And how do we do that? We check that for every file mentioned in the
> changelog and manifest, there's a file mentioned by repo.storefiles(). 
> Which is a good test regardless of what store type we're using.
> 

You haven't replied to my argument that storefiles() needs to dir-walk
the store directory for the non-filenamelog repos.

Again, this is uneeded.

Also, storefiles returns the changelog and the manifest too, so this
is the wrong layer.

Looks like we have to agree that we disagree here, which for me means,
that I'll stop working on this, as you seem to want an entierly
different solution.

I especially don't see the point in rewriting the filenamelog file when
reading it, as I have already delivered good enough code which does
it without rewriting it.





More information about the Mercurial-devel mailing list