jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mete Atamel <mata...@adobe.com>
Subject Re: MicroKernelIT#conflictingMove
Date Tue, 15 Jan 2013 11:16:26 GMT
Right, when CommitCommandNew detects a concurrent commit, it retries the
whole commit loop with mergeNodes etc. but I think the real problem
regarding conflictingMove test is that CommitCommandNew#mergeNodes does
not detect merge conflicts between base revision nodes and head nodes at
the moment. I'm not sure if there's an easy way to detect those merge
conflicts but committing against head revision (instead of base revision)
seems to get rid of this problem. So I guess I need to understand why it
is important to commit against base revision in CommitCommandNew.

-Mete

On 1/15/13 11:23 AM, "Marcel Reutegger" <mreutegg@adobe.com> wrote:

>Hi,
>
>Mete and I started a discussion about conflicting commits in private
>emails and I'd like to move this to the dev list. feel free to jump in
>and discuss.
>
>history (most recent first):
>
>hmm, thinking a bit more about this, I think you might be right and
>committing
>against the given base revision isn't that useful when we later have to
>rebase
>the commit again.
>
>it would be better to only do it once. but I'm not sure if that's really
>possible.
>I guess the reason why we have it that way at the moment, is the inner
>retry logic
>in CommitCommandNew (not the one in the executor). it will retry the
>commit
>when it detects a concurrent commit. in this case it will possibly have
>to rebase the
>commit. as done in mergeNodes()?
>
><previousMessage/>
>
>Hi Mete,
>
>> I looked into why 2 tests failing in MicroKernelIT for MongoMK. One of
>> them is conflictingMove test. This test passes for MicroKernelImpl and
>>it
>> looks like the reason is MicroKernelImpl always commits against head.
>>Take
>> a look at CommitBuilder around line 112. There's a check whether
>>baseRevId
>> is not equal to currentHead, and if so, the staged tree is reset to
>> currentHead and the operation is retried. This effectively means that
>>the
>> operation is committed against head, right?
>
>hmm, not sure. I'd first have to look at the code. if it just blindly
>commits to
>head then it's certainly wrong and it also means it wouldn't detect the
>conflict, right? So, I guess what it does is some kind of rebase.
>
>> We recently changed CommitCommand to commit against baseRevisionId
>> instead
>> of head but I'm thinking maybe we should change it back to committing
>> against head? Or maybe we commit against baseRevisionId but we add some
>> kind of a conflicting commit detection (although I'm not too sure how
>>we'd
>> detect conflicting commits easily). WDYT?
>
>this is missing right now. merging with the *head* revision tree is done
>in
>CommitCommandNew.mergeNodes(). that is, mergeNodes() basically does
>the rebase I mentioned above. I recently added a FIXME to this method.
>I think this is the place where we'd need to find out if there is a
>conflict.
>
>


Mime
View raw message