You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "Michael Dürig (JIRA)" <ji...@apache.org> on 2012/11/22 12:02:58 UTC
[jira] [Created] (OAK-464) RootImpl.rebase() doesn't handle move
operations correctly
Michael Dürig created OAK-464:
---------------------------------
Summary: RootImpl.rebase() doesn't handle move operations correctly
Key: OAK-464
URL: https://issues.apache.org/jira/browse/OAK-464
Project: Jackrabbit Oak
Issue Type: Bug
Reporter: Michael Dürig
Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508698#comment-13508698 ]
Thomas Mueller commented on OAK-464:
------------------------------------
The title of this issue is "RootImpl.rebase() doesn't handle move operations correctly", to me it sounds like this would be best solved in oak-core.
Jukka, in an earlier comment, you wrote:
> Given the move in the first commit, the second commit in this case should be interpreted as "+/b/x:0"
Now you write :
> I think it's fine if the MK chooses to simply throw an exception in such a case.
I'm a bit confused now, which would be the right way? If both is fine, wouldn't that mean the original problem (move operations) still need to be handled in oak-core if the MicroKernel doesn't handle this conflict, so are still inefficient?
> instead of oak-core having its own, slightly different way of handling conflicts.
I think oak-core should know about the original JCR operations, so it should be a much better position to handle conflicts than the MicroKernel.
I don't understand what problem we want to solve here, if not just more functionality down from oak-core to the MicroKernel. I guess it would help if you provide a concrete example, instead of high level description?
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505306#comment-13505306 ]
Thomas Mueller commented on OAK-464:
------------------------------------
> needs similar logic for processing normal commits whose base revision is older than the latest one.
I don't think the MicroKernel specification currently requires each MicroKernel implementation to do something as complex as this.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508730#comment-13508730 ]
Thomas Mueller commented on OAK-464:
------------------------------------
> that solution requires an extra roundtrip
If it's just a potential performance problem, I would rather defer it until it does turn out to be really a problem.
I see a bigger potential performance gain if oak-core would avoid calling branch / merge for the default case. If it wouldn't branch / merge, then the problem with rebase wouldn't even exist I guess.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505305#comment-13505305 ]
Thomas Mueller commented on OAK-464:
------------------------------------
Ok, thanks. It sounds rather complex to implement. If it is really required, I think this is not something that all MicroKernel implementations should need to support, otherwise implementing a MicroKernel (which was supposed to be simple) becomes even more complicated.
I guess the logic would always be the same, independent of the MicroKernel implementation. So I think it should be possible to solve that in a generic way, so that it only needs to be implemented once (in oak-core for example).
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505316#comment-13505316 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
bq. I don't think the MicroKernel specification currently requires each MicroKernel implementation to do something as complex as this.
I think it's fine if a MK opts for a simpler logic, for example just re-applying the diff as-is if possible and throwing an exception when not. Even in that case it would be useful if oak-core could leverage the same logic through an explicit {{rebase()}} call on a branch instead of having to implement the logic in a potentially different way with the caveats mentioned in issue summary.
The relevant MK API could be something like this:
{code}
/**
* Re-applies all branch commits up to the given branch revision to
* a new branch based on the given new base revision. Returns the
* resulting new branch revision.
*/
String rebase(String branchRevision, String newBaseRevision);
{code}
The implementation would be equivalent to something like this:
{code}
public String rebase(String branchRevision, String newBaseRevision) {
String revision = branch(newBaseRevision);
for (String jsop : getBranchCommitsUntil(branchRevision)) {
revision = commit("/", jsop, revision, null);
}
return revision;
}
{code}
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505317#comment-13505317 ]
Michael Dürig commented on OAK-464:
-----------------------------------
bq. It seems like the MicroKernel might be in the best position to perform the rebase operation.
I agree that the Microkernel is probably in the best position to do an efficient rebase operation. However, this way we will lose the current conflict resolution mechanism we have in place through the {{ConflictHandler}} interface.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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] [Comment Edited] (OAK-464) RootImpl.rebase() doesn't handle
move operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508698#comment-13508698 ]
Thomas Mueller edited comment on OAK-464 at 12/3/12 12:52 PM:
--------------------------------------------------------------
The title of this issue is "RootImpl.rebase() doesn't handle move operations correctly", to me it sounds like this would be best solved in oak-core.
Jukka, in an earlier comment, you wrote:
> Given the move in the first commit, the second commit in this case should be interpreted as "+/b/x:0"
Now you write :
> I think it's fine if the MK chooses to simply throw an exception in such a case.
I'm a bit confused now, which would be the right way? If both is fine, wouldn't that mean the original problem (move operations) still need to be handled in oak-core if the MicroKernel doesn't handle this conflict, so are still inefficient?
> instead of oak-core having its own, slightly different way of handling conflicts.
I think oak-core should know about the original JCR operations, so it should be a much better position to handle conflicts than the MicroKernel.
I don't understand what problem we want to solve here, if not just move functionality down from oak-core to the MicroKernel. I guess it would help if you provide a concrete example, instead of high level description?
was (Author: tmueller):
The title of this issue is "RootImpl.rebase() doesn't handle move operations correctly", to me it sounds like this would be best solved in oak-core.
Jukka, in an earlier comment, you wrote:
> Given the move in the first commit, the second commit in this case should be interpreted as "+/b/x:0"
Now you write :
> I think it's fine if the MK chooses to simply throw an exception in such a case.
I'm a bit confused now, which would be the right way? If both is fine, wouldn't that mean the original problem (move operations) still need to be handled in oak-core if the MicroKernel doesn't handle this conflict, so are still inefficient?
> instead of oak-core having its own, slightly different way of handling conflicts.
I think oak-core should know about the original JCR operations, so it should be a much better position to handle conflicts than the MicroKernel.
I don't understand what problem we want to solve here, if not just more functionality down from oak-core to the MicroKernel. I guess it would help if you provide a concrete example, instead of high level description?
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508638#comment-13508638 ]
Thomas Mueller commented on OAK-464:
------------------------------------
I'm wondering why "rebase operation on top of the Microkernel" need diffing between the involved node states. Is the problem that "getBranchCommitsUntil" is not possible with the current MicroKernel API? If yes, should we rather change the MicroKernel API to allow it (getJournal to support reading the journal for branch commits)?
I'm just against pushing more functionality down to the MicroKernel implementation. It should be simple to implement.
Jukka, to your code example "The implementation would be equivalent to something like this" - the loop contains multiple commits. Is each of those commits atomic? What about conflicts?
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13507482#comment-13507482 ]
Michael Dürig commented on OAK-464:
-----------------------------------
Here is a short summary of our options on this. TBD.
* Move the rebase operation down to the Microkernel
** Easy and efficient to implement since the Microkernel has full access to the journal
** No additional roundtrips between Microkernel and oak-core
** We lose the conflict handling logic that we currently have in oak-core (i.e. {{ConflictHandler}})
* Implement the rebase operation on top of the Microkernel (status quo)
** Easy to implement, needs diffing between the involved node states though.
** Additional roundtrips between Microkernel and oak-core due to the diffing in oak-core
** Inefficient wrt. to moving large subtrees. These will degrade to a full deep move (i.e. each moved node will be newly added)
* Drop the rebase operation completely
** Increased changes of conflicts on commit
** No facility for resolving conflicts on client side
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508717#comment-13508717 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
Yes, we can do this also with the branch journal, though that solution requires an extra roundtrip of all commits between the MK and oak-core, which might get pretty expensive when dealing with large commits (big import, etc.) and/or remote MK access (many roundtrips). The journal approach also misses the potential for lower level optimizations I mentioned above. Adding a MK method with such a default implementation based on the branch journal would allow more advanced MK implementations to further optimize this case.
Re: "should be interpreted"; Yes, sorry about the confusing language. My intention with that phrasing was that ideally a MK implementation would be able to automatically resolve conflicts like that (i.e. SHOULD rather than MUST), but a simpler implementation could just throw exceptions in such cases (as long as it doesn't need to support fully concurrent commits). In the latter case such commits would simply fail without oak-core trying to do anything extra about the matter.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508674#comment-13508674 ]
Thomas Mueller commented on OAK-464:
------------------------------------
> The loss of custom conflict handling logic is unfortunate
I think conflict resolution needs to handled in oak-core. I'm worried that it is not possible to (correctly) solve all possible conflicts within the MicroKernel.
> Implement the rebase operation on top of the Microkernel (status quo)
> -0 For the performance and complexity overhead.
To me it sounds like you want to avoid complexity in oak-core and therefore request to implement it in the MicroKernel. To me it sounds like the wrong thing to do.
> Is each of those commits atomic?
> Yes, though since they're against a branch, there's no need to worry about concurrent commits.
My question was, what should the MicroKernel do if one of the commit fails because of a conflict. My question wasn't about concurrency.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13504706#comment-13504706 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
It seems like the MicroKernel might be in the best position to perform the rebase operation. It has the full change log of a branch, and it in any case needs similar logic for processing normal commits whose base revision is older than the latest one.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13504712#comment-13504712 ]
Thomas Mueller commented on OAK-464:
------------------------------------
> similar logic
I don't know what a rebase does exactly, could you describe this in more detail?
I would prefer if the MicroKernel would stay simple :-)
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508677#comment-13508677 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
bq. I'm worried that it is not possible to (correctly) solve all possible conflicts within the MicroKernel.
The MK already needs to deal with conflicts. How otherwise would you deal with the above-mentioned situation?
{code}
String base = mk.getHeadRevision();
mk.commit("/", ">/a:/b", base, null);
mk.commit("/", "+/a/x:0", base, null);
{code}
Note that I think it's fine if the MK chooses to simply throw an exception in such a case. The main point here is that the MK in any case needs to have at least some mechanism of dealing with conflicts. Why would we want or need to implement the same thing also in oak-core?
bq. To me it sounds like you want to avoid complexity in oak-core and therefore request to implement it in the MicroKernel.
Indeed, especially since the MK in any case needs (and already to a degree has) this logic for dealing with normal commits.
bq. My question was, what should the MicroKernel do if one of the commit fails because of a conflict.
ACK. That's up to the MK to decide, a simple implementation could for example just throw an exception. IMO it's better if that case is handled in exactly the same manner as the above code snippet gets handled, instead of oak-core having its own, slightly different way of handling conflicts.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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] [Updated] (OAK-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Dürig updated OAK-464:
------------------------------
Description:
Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
was:
Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508649#comment-13508649 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
bq. Move the rebase operation down to the Microkernel
+1 The loss of custom conflict handling logic is unfortunate, but that's already the case with MK implementations that that allow concurrent commits from multiple clients - there's no way for oak-core to add custom handling for those cases.
bq. Implement the rebase operation on top of the Microkernel (status quo)
-0 For the performance and complexity overhead.
bq. Drop the rebase operation completely
-1 The only way not to have at least some level of rebase functionality is if we only allow commits based on the latest revision, essentially serializing all writes.
bq. Is each of those commits atomic?
Yes, though since they're against a branch, there's no need to worry about concurrent commits.
BTW, there's no particular need for doing multiple individual commits here if the underlying MK can apply all the changes in a branch as a single operation. The possibility of such lower level optimizations is one reason for why I'd prefer to have this functionality in the MK instead of in oak-core.
bq. What about conflicts?
The only possible source of conflicts is the updated base revision of the branch. Handling of those is the point of having a rebase operation.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508704#comment-13508704 ]
Thomas Mueller commented on OAK-464:
------------------------------------
About:
{code}
public String rebase(String branchRevision, String newBaseRevision) {
String revision = branch(newBaseRevision);
for (String jsop : getBranchCommitsUntil(branchRevision)) {
revision = commit("/", jsop, revision, null);
}
return revision;
}
{code}
It seems the only reason this can not currently be implemented on top of the MicroKernel API is because getJournal is specified to not work for branch commits. If getJournal can return branch commits, then I don't see a reason why this couldn't be implemented on top of oak-core.
So, if this is really the only missing piece, I would rather support branch commits in getJournal, and then implement the feature above in oak-core.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13504735#comment-13504735 ]
Jukka Zitting commented on OAK-464:
-----------------------------------
bq. I don't know what a rebase does exactly, could you describe this in more detail?
Consider for example:
{code}
String base = mk.getHeadRevision();
mk.commit("/", ">/a:/b", base, null);
mk.commit("/", "+/a/x:0", base, null);
{code}
Given the move in the first commit, the second commit in this case should be interpreted as {{"+/b/x:0"}} since the property change refers to the original /a node in the base revision that was meanwhile moved to /b.
Basically the rebase operation takes a given commit and reapplies the contained changes on top of another base revision (typically the head revision).
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13502719#comment-13502719 ]
Michael Dürig commented on OAK-464:
-----------------------------------
The underlying problem of this has been discussed on various occasions already. See for example
* http://markmail.org/message/3c22jqy6gzzkyxx2
* http://markmail.org/message/picnwsyr32cbb63l
* http://markmail.org/message/panl3wxfekvmcfyw
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add.
--
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] [Comment Edited] (OAK-464) RootImpl.rebase() doesn't handle
move operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13507482#comment-13507482 ]
Michael Dürig edited comment on OAK-464 at 12/3/12 11:02 AM:
-------------------------------------------------------------
Here is a short summary of our options on this. TBD.
* Move the rebase operation down to the Microkernel
** Easy and efficient to implement since the Microkernel has full access to the journal
** No additional roundtrips between Microkernel and oak-core
** We lose the conflict handling logic that we currently have in oak-core (i.e. {{ConflictHandler}})
* Implement the rebase operation on top of the Microkernel (status quo)
** Easy to implement, needs diffing between the involved node states though.
** Additional roundtrips between Microkernel and oak-core due to the diffing in oak-core
** Inefficient wrt. to moving large subtrees. These will degrade to a full deep move (i.e. each moved node will be newly added)
* Drop the rebase operation completely
** Increased changes of conflicts on commit
** No facility for resolving conflicts on client side
* Implement the rebase operation on to of the Microkernel using the journal supplied from the Microkernel
** Easy to implement
** Additional roundtrips between Microkernel and oak-core for retrieving the journal
** Needs support from the Microkernel. Currently {{getJournal}} does not work on branches.
was (Author: mduerig):
Here is a short summary of our options on this. TBD.
* Move the rebase operation down to the Microkernel
** Easy and efficient to implement since the Microkernel has full access to the journal
** No additional roundtrips between Microkernel and oak-core
** We lose the conflict handling logic that we currently have in oak-core (i.e. {{ConflictHandler}})
* Implement the rebase operation on top of the Microkernel (status quo)
** Easy to implement, needs diffing between the involved node states though.
** Additional roundtrips between Microkernel and oak-core due to the diffing in oak-core
** Inefficient wrt. to moving large subtrees. These will degrade to a full deep move (i.e. each moved node will be newly added)
* Drop the rebase operation completely
** Increased changes of conflicts on commit
** No facility for resolving conflicts on client side
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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-464) RootImpl.rebase() doesn't handle move
operations correctly
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508648#comment-13508648 ]
Michael Dürig commented on OAK-464:
-----------------------------------
bq. I'm wondering why "rebase operation on top of the Microkernel" need diffing between the involved node states.
Right, that's another option. I added it to the above list.
If we could extend the Microkernel to support journals for branches, I think this might the best way for the time being.
> RootImpl.rebase() doesn't handle move operations correctly
> ----------------------------------------------------------
>
> Key: OAK-464
> URL: https://issues.apache.org/jira/browse/OAK-464
> Project: Jackrabbit Oak
> Issue Type: Bug
> Reporter: Michael Dürig
>
> Doing {{RootImpl.rebase()}} causes moves to be changed to remove followed by add. Which causes moves of large sub trees to become very expensive.
--
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