Fixing .hg file open ordering

[Ryan McElroy]

On 11/22/16 8:19 PM, Ryan McElroy wrote:
> On 11/10/16 5:11 PM, Bryan O'Sullivan wrote:
>>
>> On Thu, Nov 10, 2016 at 2:29 AM, Durham Goode <durham at fb.com 
>> <mailto:durham at fb.com>> wrote:
>>
>>     We worked out a solution to Bryan’s main concern, but there
>>     hasn’t been any discussion of my main proposal. Internally we
>>     worked around it by forcing bookmarks and the changelog to load
>>     in the right order very early during repo setup, but that’s a
>>     hack and doesn’t address the greater problem.
>>
>>
>> For the Real Problem (TM), I'd rather try the simpler solution first, 
>> of reading the bookmarks file in the changelog constructor, then 
>> splitting it into lines in the bmstore constructor. I'd hope that 
>> could be both lightweight enough that it could avoid a performance 
>> hit, and unlikely enough to introduce bugs later on that we wouldn't 
>> have to worry about a more heavyweight "make bugs impossible" 
>> solution. If I turn out to be wrong and we need something more formal 
>> (which we've gotten away without for years with revlogs, FWIW), we 
>> haven't really lost much.
>>
>>
>
> I think that fixing the read open ordering is not sufficient.
>
> I was just chatting with Jun Wu (quark) about this, and after that 
> discussion, it seems that we also have to ensure the write order is 
> correct. Specifically, bookmarks must be written only after the 
> changelog is written, otherwise, regardless of the order that we open 
> files for read, we can have a bookmarks file that points to commits 
> that no reader can see yet.
>
> Based on a very quick look at the transaction code, it looks like 
> filegenerators in transactions are stored in a dict, so we have no 
> guarantees about what order they will be called in, so it's very 
> likely that we often call the write of the bookmarks file before the 
> write of the changelog, making it so that no read-side fix alone can 
> be sufficient to solve this problem

Well, I looked a little more deeply and actually filegenerators have an 
order parameter (sorry I missed that the first time).

However, the order parameter is unset by the bmstore when it calls 
addfilegenerator.

However (and now I'm remembering more of this), I see that we have a set 
called "postfinalizegenerators" which are forced to be after the 
finalize steps which is where the changelog is written (introduced in 
a5009789960c by durham). So it seems we're already okay here, assuming 
all that code it working as intended.

Sorry for forgetting about this.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161122/9df5fda1/attachment.html>