[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576745#comment-13576745 ] Mete Atamel commented on OAK-535: - Note that the patch I attached to OAK-586 works with FailFastMergeCommand.patch as well. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Assignee: Marcel Reutegger Attachments: FailFastMergeCommand.patch, mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13566658#comment-13566658 ] Marcel Reutegger commented on OAK-535: -- bq. and getNonConflictingCommitsDiff() considers these as conflicting right? Yes, that's correct. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Assignee: Marcel Reutegger Attachments: FailFastMergeCommand.patch, mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565186#comment-13565186 ] Marcel Reutegger commented on OAK-535: -- Now that I think more about this, I'd even go as far as ripping out all the merge code from MergeCommand. If we look at MergeCommand and what it is supposed to do, then it's basically gathering all the commits from the branch and apply it to trunk based on the branch root revision. That's the same as calling MK.commit() with the combined branch commit diffs and the branch root revision. The point is, MK.commit() must implement he rebase anyway and CommitCommand does that already, but IMO insufficiently. I think detecting concurrent and possibly conflicting commits and performing the merge should be done in CommitCommand (CommitCommand.readAndMergeExistingNodes()). MergeCommand will then simply gather the diffs and pass it to CommitCommand. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565218#comment-13565218 ] Mete Atamel commented on OAK-535: - Currently, readAndMergeExistingNodes tries to merge nodes between baseRevisionId and headRevisionId, then nodes/commits are saved and then saveAndSetHeadRevision tries to detect commits that happened since nodes/commits are saved and see if they are conflicting. If they are not conflicting, saveAndSetHeadRevision tries to update the baseRevisionIds and let things progress (OAK-585). Are you suggesting that we also need to somehow combine readAndMergeExistingNodes saveAndSetHeadRevision? MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565249#comment-13565249 ] Marcel Reutegger commented on OAK-535: -- No, I suggest to implement conflict detection in readAndMergeExistingNodes(). MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565291#comment-13565291 ] Marcel Reutegger commented on OAK-535: -- The check we have in saveAndSetHeadRevision() just detects concurrent commits, but it cannot tell whether concurrent commits conflict. I think we could keep saveAndSetHeadRevision() as is. If saveAndSetHeadRevision() fails, then the commit is retried with the new head revision. Conflict detection and resolution (if needed) on the tree level then happens in readAndMergeExistingNodes(). MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565302#comment-13565302 ] Mete Atamel commented on OAK-535: - What you described regarding saveAndSetHeadRevision is already the case but then we have OAK-585 about too many commit retries. Let's continue the discussion in OAK-585. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13565408#comment-13565408 ] Michael Dürig commented on OAK-535: --- From the oak-core perspective the important part here is the fast forward merge. That is, the merge where there are no changes on trunk. Since oak-core has not enough information to handle the case where a merge conflicts (see OAK-496), the current implementation of oak-core rebases a branch on top of the current trunk and then merges the rebased trunk. Since the current MicroKernel does not expose any rebase functionality (OAK-536) oak-core implements rebasing by creating a new branch and applying all changes from the current branch to that new branch. Apart from being expensive (wasting branches, additional round trips), this also suffers from the same [race condition Mete describes|https://issues.apache.org/jira/browse/OAK-535?focusedCommentId=13565288page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13565288] only worse since this is exposed over component boundaries. So, since oak-core doesn't care too much about the conflict case on merge but quite in the contrary, I suggest not to care about this case too much. I think it is fine to just be able to detect conflicts and throw an exception in this case. Make it dead simple: there is a conflict in merging a branch to trunk when the base revision of the trunk is not equal to the head revision of the head. It is the responsibility of the caller to make sure that branches are properly rebased before being merge. See also http://markmail.org/message/wtaarmdtgyf5lvjt. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13564343#comment-13564343 ] Marcel Reutegger commented on OAK-535: -- I was thinking about this problem again and was wondering if we can leverage the commit history again when we have a concurrent (potentially conflicting) commit. Instead of reading the tree and performing a diff, couldn't we just apply the commits between the branch root and the current head revision on top of the trunk? While doing this we could check for conflicts and abort if necessary. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13564362#comment-13564362 ] Mete Atamel commented on OAK-535: - You need to be able to apply these branch commits to trunk without actually committing to trunk until you're sure that all branch commits are OK to commit to trunk. Because one or more branch commits might be conflicting with trunk and in that case, no branch commit should go to trunk. That's why currently MergeCommand reads the tree and generates a diff and applies that diff to trunk in one commit. How would you do that in this approach? MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13554910#comment-13554910 ] Marcel Reutegger commented on OAK-535: -- Optimized diff normalization in revision: 1433884 MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13555104#comment-13555104 ] Mete Atamel commented on OAK-535: - Yeah I tried to minimize FetchCommit and looping as much as I can and I think overlapping affected paths should be detected with this algorithm. Every time an affected path is detected for branch/trunk, it's checked against trunk/branch set. If trunk/brach set does not have it yet, then the affected path will be put into branch/trunk set and checked again for trunk/branch against branch/trunk set. Since we fetch all commits from branch root until latest commit, it should contain all the relevant affected paths. But if you can think of a case where the algorithms fails, please let me know. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergecommandoptimization.patch, mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553629#comment-13553629 ] Marcel Reutegger commented on OAK-535: -- I now also committed my OAK-535.patch from yesterday. Trivial merges are now simply re-applied to trunk. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch, movepatch.diff, moves-in-commit.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13552571#comment-13552571 ] Marcel Reutegger commented on OAK-535: -- Please note that the diff aggregation is pretty quick'n'dirty and creates a very verbose diff with absolute paths for every diff operation. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13552632#comment-13552632 ] Mete Atamel commented on OAK-535: - BTW, movesInBranch test looks like a general problem with MoveNodeInstruction. If you apply the same sequence against trunk with no branch/merge, you get the same failure. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch, OAK-535.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13546973#comment-13546973 ] Mete Atamel commented on OAK-535: - Removed one of the getNodes call in rev 1430348. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13537029#comment-13537029 ] Mete Atamel commented on OAK-535: - To provide more context why this is the case, when a branch is merged into the trunk, MergeCommand does a 3-way merge (borrowed from MicroKernelImpl) among branch root, current trunk head, and the common ancestor of trunk and branch. So it needs to fetch nodes at those mentioned paths in order to do comparisons but the problem is it's not obvious what the path and depth of that fetch should be. So as a start, I went with / as path and unlimited depth to fetch everything but that obviously is not going to work for lots of nodes. After some testing, it looked to me that we should be able to limit the depth to the max depth of the branch and that works for the branch/merge tests but fails in other scenarios like quickstart. We might also be able to limit the path of the getNodes calls (instead of using /) but haven't had the chance to investigate that yet. MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-535) MergeCommand reads complete tree into memory
[ https://issues.apache.org/jira/browse/OAK-535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13537081#comment-13537081 ] Marcel Reutegger commented on OAK-535: -- I'm wondering if we need the full fledged 3-way merge in all cases. Shouldn't it be possible to skip the step of merging head into the branch, when we can detect there were no changes between the point the branch was created and the commit (aka the request to merge the branch into head)? MergeCommand reads complete tree into memory Key: OAK-535 URL: https://issues.apache.org/jira/browse/OAK-535 Project: Jackrabbit Oak Issue Type: Bug Components: mongomk Affects Versions: 0.5 Reporter: Marcel Reutegger Attachments: mergefixattempt.patch Merging commits to a branch back to head in MongoMK calls MergeCommand, which reads the complete tree into memory. The more content is in the repository, the longer it takes to store even a simple update as a property change. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
