[jira] [Commented] (OAK-7801) CompositeNodeStore.merge() may trigger conflicting branches

["Marcel Reutegger (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-7801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636880#comment-16636880 ] 

Marcel Reutegger commented on OAK-7801:
---------------------------------------

Some more details on why this only happens on a global DocumentNodeStore. The conflict handling is slightly different than specified. The DocumentNodeStore identifies conflicts already when changes are written to a branch (or to the DocumentStore in general). When a conflict is identified, a collision marker is put on one of the pending commits. The implementation currently tries to abort the 'other' pending commit. If the other commit is not pending anymore, then the implementation will trigger an abort of the current commit.

The CompositeNodeStore.merge() uses a {{CommitHookEnhancer}}, which may trigger conflicting branches within the same merge() call. This is the relevant part in {{CommitHookEnhancer.processCommit()}}:
{noformat}
        NodeState result = hook.processCommit(compositeBefore, compositeAfter, info);
        updatedBuilder = Optional.of(toComposite(result, compositeBefore));
{noformat}

The {{result}} represents the first branch. The method {{toComposite()}} then applies the changes again on top of the {{compositeBefore}} state:
{noformat}
    private CompositeNodeBuilder toComposite(NodeState nodeState, CompositeNodeState compositeRoot) {
        CompositeNodeBuilder builder = compositeRoot.builder();
        nodeState.compareAgainstBaseState(compositeRoot, new ApplyDiff(builder));
        return builder;
    }
{noformat}

The returned builder is the second branch, conflicting with the first one and therefore causing it to abort.

To me it looks like {{CommitHookEnhancer.updatedBuilder}} are remains of the attempt to support multiple writable node stores. The {{updatedBuilder}} is later only used by {{CompositeNodeStore.merge()}}:
{noformat}
        CommitHookEnhancer hookEnhancer = new CommitHookEnhancer(commitHook, ctx, nodeBuilder);
        NodeState globalResult = globalStore.getNodeStore().merge(nodeBuilder.getNodeBuilder(globalStore), hookEnhancer, info);
        if (!hookEnhancer.getUpdatedBuilder().isPresent()) {
            // it means that the commit hook wasn't invoked, because there were
            // no changes on the global store. we should invoke it anyway.
            hookEnhancer.processCommit(globalResult, globalResult, info);
        }
{noformat}

I'm wondering if this check is even necessary anymore.

[~tomek.rekawek], [~rombert], can you shed some light on what above {{if}} clause is about?

> CompositeNodeStore.merge() may trigger conflicting branches
> -----------------------------------------------------------
>
>                 Key: OAK-7801
>                 URL: https://issues.apache.org/jira/browse/OAK-7801
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: composite, documentmk
>    Affects Versions: 1.9.0
>            Reporter: Marcel Reutegger
>            Assignee: Marcel Reutegger
>            Priority: Major
>             Fix For: 1.10
>
>
> This issue only affects a CompositeNodeStore with a global DocumentNodeStore. The merge() may trigger the creation of two conflicting DocumentNodeStore branches, which then fails the merge operation even though there is no real conflicting change. This issue does not happen with 1.8 or earlier because those releases keep changes introduced by commit hooks in memory. See also OAK-7401.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)