jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Reutegger (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OAK-7801) CompositeNodeStore.merge() may trigger conflicting branches
Date Wed, 03 Oct 2018 12:08:00 GMT

    [ 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()}}:
        NodeState result = hook.processCommit(compositeBefore, compositeAfter, info);
        updatedBuilder = Optional.of(toComposite(result, compositeBefore));

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

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()}}:
        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);

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

View raw message