You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by Michael Dürig <md...@apache.org> on 2012/01/25 18:04:08 UTC
[jr3 Microkernel] how should we handle failed save operations?
Hi,
In an earlier discussion (probably offline), we decided to not implement
Item.refresh() since it doesn't go well with the MVCC model jr3 is based
on. Furthermore, JCR doesn't have an Item.undo() method for undoing changes.
This may lead to problems when a Session.save() fails due to the
underlying Microkernel.commit failing because it detected a conflict.
Now there might be some transient changes (like deletions) which can't
be selectively undone by the user. So the user is left with a transient
space containing his changes but he can only discard them as a whole.
Not very satisfactory.
Possible solutions:
1) The Microkernel makes as much effort as possible to three way merge
changes.
2) The user needs to do a session refresh with keep changes = true and
save again.
3) Introduce a Item.undo method on the JCR API.
1) Mitigates the problem such that it only occurs rarely.
2) Is really nothing more than moving the problem of the failed commit
due to a conflict from the Microkernel to the transient space: now the
transient space needs to do conflict resolution.
3) Is what I think we should do. This enable the user to resolve his
conflicts selectively.
Michael
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Michael Dürig <md...@apache.org>.
On 26.1.12 10:55, Jukka Zitting wrote:
> Hi,
>
> On Wed, Jan 25, 2012 at 6:04 PM, Michael Dürig<md...@apache.org> wrote:
>> In an earlier discussion (probably offline), we decided to not implement
>> Item.refresh() since it doesn't go well with the MVCC model jr3 is based on.
>
> I'd implement Item.refresh() like this:
>
> public void refresh(boolean keepChanges) throws RepositoryException {
> if (!keepChanges) {
> discardTransientChangesBelowThisItem();
> }
>
> // Update the MVCC base state of this session to the latest
> // state of the workspace.
> getSession().refresh(true);
> }
>
> In other words, Item.refresh() would always trigger a refresh() of the
> entire session. That's OK spec-wise, since it's always OK for the
> implementation to refresh the session state at any point of time it
> wants.
That's an interesting approach. Will think about it. I think this
together with 1) will could be a viable solution.
Michael
>
>> Furthermore, JCR doesn't have an Item.undo() method for undoing changes.
>
> I'd treat refresh(false) as undo(), both on Item and Session level.
>
>> This may lead to problems when a Session.save() fails due to the underlying
>> Microkernel.commit failing because it detected a conflict. Now there might
>> be some transient changes (like deletions) which can't be selectively undone
>> by the user. So the user is left with a transient space containing his
>> changes but he can only discard them as a whole. Not very satisfactory.
>
> I think that's fine, especially if we implement Item.refresh(false)
> with an internal discardTransientChangesBelowThisItem() operation as
> described above.
>
>> 1) The Microkernel makes as much effort as possible to three way merge
>> changes.
>
> I think we should do this in any case.
>
>> 2) The user needs to do a session refresh with keep changes = true and save
>> again.
>
> I'd always automatically trigger a refresh(true) at the beginning of a
> save() call.
>
>> 3) Introduce a Item.undo method on the JCR API.
>
> As mentioned above, I think Item.refresh(false) could already cover
> the required functionality.
>
> BR,
>
> Jukka Zitting
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Jan 25, 2012 at 6:04 PM, Michael Dürig <md...@apache.org> wrote:
> In an earlier discussion (probably offline), we decided to not implement
> Item.refresh() since it doesn't go well with the MVCC model jr3 is based on.
I'd implement Item.refresh() like this:
public void refresh(boolean keepChanges) throws RepositoryException {
if (!keepChanges) {
discardTransientChangesBelowThisItem();
}
// Update the MVCC base state of this session to the latest
// state of the workspace.
getSession().refresh(true);
}
In other words, Item.refresh() would always trigger a refresh() of the
entire session. That's OK spec-wise, since it's always OK for the
implementation to refresh the session state at any point of time it
wants.
> Furthermore, JCR doesn't have an Item.undo() method for undoing changes.
I'd treat refresh(false) as undo(), both on Item and Session level.
> This may lead to problems when a Session.save() fails due to the underlying
> Microkernel.commit failing because it detected a conflict. Now there might
> be some transient changes (like deletions) which can't be selectively undone
> by the user. So the user is left with a transient space containing his
> changes but he can only discard them as a whole. Not very satisfactory.
I think that's fine, especially if we implement Item.refresh(false)
with an internal discardTransientChangesBelowThisItem() operation as
described above.
> 1) The Microkernel makes as much effort as possible to three way merge
> changes.
I think we should do this in any case.
> 2) The user needs to do a session refresh with keep changes = true and save
> again.
I'd always automatically trigger a refresh(true) at the beginning of a
save() call.
> 3) Introduce a Item.undo method on the JCR API.
As mentioned above, I think Item.refresh(false) could already cover
the required functionality.
BR,
Jukka Zitting
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Michael Dürig <md...@apache.org>.
>> This may lead to problems when a Session.save() fails due to the underlying
>> Microkernel.commit failing because it detected a conflict. Now there might
>> be some transient changes (like deletions) which can't be selectively undone
>> by the user. So the user is left with a transient space containing his
>> changes but he can only discard them as a whole. Not very satisfactory.
>
> this is IMO a rather theoretical problem. i am not aware of any jcr client code
> which actually does re-try a failed save operation after having
> partially discarded
> and fixed transient changes by calling Item.refresh(false);
>
> however, i agree that, theroretically, it is an issue ;)
In don't think so. Consider two users concurrently working on the same
web page. The user who saves last and runs into a conflict loses *all*
his work. No way to merge, partially apply or keep it otherwise. His
only option is to discard all of his work and to do it again from start.
Michael
>
>>
>> Possible solutions:
>>
>> 1) The Microkernel makes as much effort as possible to three way merge
>> changes.
>
> +1, agreed.
>
>>
>> 2) The user needs to do a session refresh with keep changes = true and save
>> again.
>>
>> 3) Introduce a Item.undo method on the JCR API.
>
> i don't think we need that because IMO there's no real world use case.
>
> cheers
> stefan
>
>>
>>
>> 1) Mitigates the problem such that it only occurs rarely.
>>
>> 2) Is really nothing more than moving the problem of the failed commit due
>> to a conflict from the Microkernel to the transient space: now the transient
>> space needs to do conflict resolution.
>>
>> 3) Is what I think we should do. This enable the user to resolve his
>> conflicts selectively.
>>
>> Michael
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Stefan Guggisberg <st...@gmail.com>.
On Wed, Jan 25, 2012 at 6:04 PM, Michael Dürig <md...@apache.org> wrote:
>
> Hi,
>
> In an earlier discussion (probably offline), we decided to not implement
> Item.refresh() since it doesn't go well with the MVCC model jr3 is based on.
> Furthermore, JCR doesn't have an Item.undo() method for undoing changes.
please note that Item.refresh() will most likely be deprecated in JCR 2.1.
>
> This may lead to problems when a Session.save() fails due to the underlying
> Microkernel.commit failing because it detected a conflict. Now there might
> be some transient changes (like deletions) which can't be selectively undone
> by the user. So the user is left with a transient space containing his
> changes but he can only discard them as a whole. Not very satisfactory.
this is IMO a rather theoretical problem. i am not aware of any jcr client code
which actually does re-try a failed save operation after having
partially discarded
and fixed transient changes by calling Item.refresh(false);
however, i agree that, theroretically, it is an issue ;)
>
> Possible solutions:
>
> 1) The Microkernel makes as much effort as possible to three way merge
> changes.
+1, agreed.
>
> 2) The user needs to do a session refresh with keep changes = true and save
> again.
>
> 3) Introduce a Item.undo method on the JCR API.
i don't think we need that because IMO there's no real world use case.
cheers
stefan
>
>
> 1) Mitigates the problem such that it only occurs rarely.
>
> 2) Is really nothing more than moving the problem of the failed commit due
> to a conflict from the Microkernel to the transient space: now the transient
> space needs to do conflict resolution.
>
> 3) Is what I think we should do. This enable the user to resolve his
> conflicts selectively.
>
> Michael
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
>but you're right, the above scenario could/should be
>handled gracefully.
That makes sense. I have added a test case and changed the
MemoryKernelImpl in the sandbox.
Regards,
Thomas
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Stefan Guggisberg <st...@gmail.com>.
On Thu, Jan 26, 2012 at 2:42 PM, Michael Dürig <md...@apache.org> wrote:
>
>> BTW: the current microkernel prototype doesn't consider concurrent
>> removal of a specific node a conflict, i.e. it already implements the
>> proposed behavior.
>
>
> Hmm no?
>
> String head = mk.getHeadRevision();
> head = mk.commit("/", "+\"qoo\":{}", head, "");
>
> String r1 = mk.commit("/", "-\"qoo\"", head, "");
> String r2 = mk.commit("/", "-\"qoo\"", head, "");
>
> The commit for r2 gives me an error:
> org.apache.jackrabbit.mk.api.MicroKernelException:
> org.apache.jackrabbit.mk.store.NotFoundException: /qoo
i should have been more specific. i meant concurrent, i.e.
overlapping, not sequential commits ;)
but you're right, the above scenario could/should be
handled gracefully.
cheers
stefan
>
> Michael
>
>
>
>>
>> cheers
>> stefan
>>
>>>
>>> Regards,
>>> Thomas
>>>
>>>
>>>
>>>
>>>
>>> On 1/25/12 6:04 PM, "Michael Dürig"<md...@apache.org> wrote:
>>>
>>>>
>>>> Hi,
>>>>
>>>> In an earlier discussion (probably offline), we decided to not implement
>>>> Item.refresh() since it doesn't go well with the MVCC model jr3 is based
>>>> on. Furthermore, JCR doesn't have an Item.undo() method for undoing
>>>> changes.
>>>>
>>>> This may lead to problems when a Session.save() fails due to the
>>>> underlying Microkernel.commit failing because it detected a conflict.
>>>> Now there might be some transient changes (like deletions) which can't
>>>> be selectively undone by the user. So the user is left with a transient
>>>> space containing his changes but he can only discard them as a whole.
>>>> Not very satisfactory.
>>>>
>>>> Possible solutions:
>>>>
>>>> 1) The Microkernel makes as much effort as possible to three way merge
>>>> changes.
>>>>
>>>> 2) The user needs to do a session refresh with keep changes = true and
>>>> save again.
>>>>
>>>> 3) Introduce a Item.undo method on the JCR API.
>>>>
>>>>
>>>> 1) Mitigates the problem such that it only occurs rarely.
>>>>
>>>> 2) Is really nothing more than moving the problem of the failed commit
>>>> due to a conflict from the Microkernel to the transient space: now the
>>>> transient space needs to do conflict resolution.
>>>>
>>>> 3) Is what I think we should do. This enable the user to resolve his
>>>> conflicts selectively.
>>>>
>>>> Michael
>>>
>>>
>
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Michael Dürig <md...@apache.org>.
> BTW: the current microkernel prototype doesn't consider concurrent
> removal of a specific node a conflict, i.e. it already implements the
> proposed behavior.
Hmm no?
String head = mk.getHeadRevision();
head = mk.commit("/", "+\"qoo\":{}", head, "");
String r1 = mk.commit("/", "-\"qoo\"", head, "");
String r2 = mk.commit("/", "-\"qoo\"", head, "");
The commit for r2 gives me an error:
org.apache.jackrabbit.mk.api.MicroKernelException:
org.apache.jackrabbit.mk.store.NotFoundException: /qoo
Michael
>
> cheers
> stefan
>
>>
>> Regards,
>> Thomas
>>
>>
>>
>>
>>
>> On 1/25/12 6:04 PM, "Michael Dürig"<md...@apache.org> wrote:
>>
>>>
>>> Hi,
>>>
>>> In an earlier discussion (probably offline), we decided to not implement
>>> Item.refresh() since it doesn't go well with the MVCC model jr3 is based
>>> on. Furthermore, JCR doesn't have an Item.undo() method for undoing
>>> changes.
>>>
>>> This may lead to problems when a Session.save() fails due to the
>>> underlying Microkernel.commit failing because it detected a conflict.
>>> Now there might be some transient changes (like deletions) which can't
>>> be selectively undone by the user. So the user is left with a transient
>>> space containing his changes but he can only discard them as a whole.
>>> Not very satisfactory.
>>>
>>> Possible solutions:
>>>
>>> 1) The Microkernel makes as much effort as possible to three way merge
>>> changes.
>>>
>>> 2) The user needs to do a session refresh with keep changes = true and
>>> save again.
>>>
>>> 3) Introduce a Item.undo method on the JCR API.
>>>
>>>
>>> 1) Mitigates the problem such that it only occurs rarely.
>>>
>>> 2) Is really nothing more than moving the problem of the failed commit
>>> due to a conflict from the Microkernel to the transient space: now the
>>> transient space needs to do conflict resolution.
>>>
>>> 3) Is what I think we should do. This enable the user to resolve his
>>> conflicts selectively.
>>>
>>> Michael
>>
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Stefan Guggisberg <st...@gmail.com>.
On Thu, Jan 26, 2012 at 9:48 AM, Thomas Mueller <mu...@adobe.com> wrote:
> Hi,
>
> I would try 1). I would not try 3) because the user wouldn't know which
> item conflicted (well he could parse the message, but that would be
> weird). Also, I would try to avoid a new API.
>
> For the case 'double delete' another solution is possible.
>
> * session1 deleted node /test and does some other changes
> * session2 deleted node /test and does some other changes
> * session1 save (successful)
> * session2 save (fails: /test can't be deleted because it doesn't exist
> any more)
>
> 4): The microkernel could ignore delete operations on paths that don't
> currently exist. This is somewhat similar to what databases do: "delete
> from test where id=1" will be successful if there is no row with id 1. Or
> with JCR setProperty(..., null) for a non-existent property (which
> currently works fine).
BTW: the current microkernel prototype doesn't consider concurrent
removal of a specific node a conflict, i.e. it already implements the
proposed behavior.
cheers
stefan
>
> Regards,
> Thomas
>
>
>
>
>
> On 1/25/12 6:04 PM, "Michael Dürig" <md...@apache.org> wrote:
>
>>
>>Hi,
>>
>>In an earlier discussion (probably offline), we decided to not implement
>>Item.refresh() since it doesn't go well with the MVCC model jr3 is based
>>on. Furthermore, JCR doesn't have an Item.undo() method for undoing
>>changes.
>>
>>This may lead to problems when a Session.save() fails due to the
>>underlying Microkernel.commit failing because it detected a conflict.
>>Now there might be some transient changes (like deletions) which can't
>>be selectively undone by the user. So the user is left with a transient
>>space containing his changes but he can only discard them as a whole.
>>Not very satisfactory.
>>
>>Possible solutions:
>>
>>1) The Microkernel makes as much effort as possible to three way merge
>>changes.
>>
>>2) The user needs to do a session refresh with keep changes = true and
>>save again.
>>
>>3) Introduce a Item.undo method on the JCR API.
>>
>>
>>1) Mitigates the problem such that it only occurs rarely.
>>
>>2) Is really nothing more than moving the problem of the failed commit
>>due to a conflict from the Microkernel to the transient space: now the
>>transient space needs to do conflict resolution.
>>
>>3) Is what I think we should do. This enable the user to resolve his
>>conflicts selectively.
>>
>>Michael
>
Re: [jr3 Microkernel] how should we handle failed save operations?
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
I would try 1). I would not try 3) because the user wouldn't know which
item conflicted (well he could parse the message, but that would be
weird). Also, I would try to avoid a new API.
For the case 'double delete' another solution is possible.
* session1 deleted node /test and does some other changes
* session2 deleted node /test and does some other changes
* session1 save (successful)
* session2 save (fails: /test can't be deleted because it doesn't exist
any more)
4): The microkernel could ignore delete operations on paths that don't
currently exist. This is somewhat similar to what databases do: "delete
from test where id=1" will be successful if there is no row with id 1. Or
with JCR setProperty(..., null) for a non-existent property (which
currently works fine).
Regards,
Thomas
On 1/25/12 6:04 PM, "Michael Dürig" <md...@apache.org> wrote:
>
>Hi,
>
>In an earlier discussion (probably offline), we decided to not implement
>Item.refresh() since it doesn't go well with the MVCC model jr3 is based
>on. Furthermore, JCR doesn't have an Item.undo() method for undoing
>changes.
>
>This may lead to problems when a Session.save() fails due to the
>underlying Microkernel.commit failing because it detected a conflict.
>Now there might be some transient changes (like deletions) which can't
>be selectively undone by the user. So the user is left with a transient
>space containing his changes but he can only discard them as a whole.
>Not very satisfactory.
>
>Possible solutions:
>
>1) The Microkernel makes as much effort as possible to three way merge
>changes.
>
>2) The user needs to do a session refresh with keep changes = true and
>save again.
>
>3) Introduce a Item.undo method on the JCR API.
>
>
>1) Mitigates the problem such that it only occurs rarely.
>
>2) Is really nothing more than moving the problem of the failed commit
>due to a conflict from the Microkernel to the transient space: now the
>transient space needs to do conflict resolution.
>
>3) Is what I think we should do. This enable the user to resolve his
>conflicts selectively.
>
>Michael