You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Marcel Reutegger <mr...@adobe.com> on 2013/04/25 12:23:21 UTC
race conditions in commit hooks/editors
hi all,
today I encountered a race condition in one of the commit hooks,
which points to a more general problem.
first the specific issue with the PermissionHook. it creates some
nodes if they don't yet exist (/jcr:system/jcr:permissionStore/default)
this happens with the first commit to a workspace. the problem now is,
concurrent commits will fails when they are issues right after the
repository is initialized. to solve this specific issue, we should
probably move the code to a RepositoryInitializer.
more generally, these kind of race conditions may also happen in
other editors and hooks, even if the changes of some concurrent
commits otherwise don't overlap. e.g. creating a versionable node
will currently create a version history with some more or less
randomly named ancestor nodes, which may overlap with other
versionable nodes created at the same time. similarly the p2
index implementation may introduce a conflict in the :index
storage because of its hierarchical layout even when changes
triggering the index update don't conflict.
I'd be interested to hear what people think about this problem.
should we retry commits when these kind of conflicts where
introduced by hooks or editor or do we require the latter to
take care of this and only manipulate content in a non-conflicting
way?
regards
marcel
RE: race conditions in commit hooks/editors
Posted by Marcel Reutegger <mr...@adobe.com>.
see: https://issues.apache.org/jira/browse/OAK-797
regards
marcel
> -----Original Message-----
> From: Angela Schreiber [mailto:anchela@adobe.com]
> Sent: Donnerstag, 25. April 2013 13:20
> To: oak-dev@jackrabbit.apache.org
> Subject: Re: race conditions in commit hooks/editors
>
> hi marcel
>
> as far as the specific problem with the permission hook is
> concerned: moving to a repository initializer doesn't work
> IMHO for the following reasons:
>
> a) being part of the overall access control configuration
> the permission stuff is pluggable at runtime. initializing
> this once in the very beginning will not work.
>
> b) if a new workspace is added the corresponding section in
> the permission store must be added as well.
>
> so, i would suggest to try moving this to a WorkspaceInitializer.
> may i ask you to create an issue for that? merci.
>
> kind regards
> angela
>
>
> On 4/25/13 12:23 PM, Marcel Reutegger wrote:
> > hi all,
> >
> > today I encountered a race condition in one of the commit hooks,
> > which points to a more general problem.
> >
> > first the specific issue with the PermissionHook. it creates some
> > nodes if they don't yet exist (/jcr:system/jcr:permissionStore/default)
> >
> > this happens with the first commit to a workspace. the problem now is,
> > concurrent commits will fails when they are issues right after the
> > repository is initialized. to solve this specific issue, we should
> > probably move the code to a RepositoryInitializer.
> >
> > more generally, these kind of race conditions may also happen in
> > other editors and hooks, even if the changes of some concurrent
> > commits otherwise don't overlap. e.g. creating a versionable node
> > will currently create a version history with some more or less
> > randomly named ancestor nodes, which may overlap with other
> > versionable nodes created at the same time. similarly the p2
> > index implementation may introduce a conflict in the :index
> > storage because of its hierarchical layout even when changes
> > triggering the index update don't conflict.
> >
> > I'd be interested to hear what people think about this problem.
> > should we retry commits when these kind of conflicts where
> > introduced by hooks or editor or do we require the latter to
> > take care of this and only manipulate content in a non-conflicting
> > way?
> >
> > regards
> > marcel
> >
Re: race conditions in commit hooks/editors
Posted by Angela Schreiber <an...@adobe.com>.
hi marcel
as far as the specific problem with the permission hook is
concerned: moving to a repository initializer doesn't work
IMHO for the following reasons:
a) being part of the overall access control configuration
the permission stuff is pluggable at runtime. initializing
this once in the very beginning will not work.
b) if a new workspace is added the corresponding section in
the permission store must be added as well.
so, i would suggest to try moving this to a WorkspaceInitializer.
may i ask you to create an issue for that? merci.
kind regards
angela
On 4/25/13 12:23 PM, Marcel Reutegger wrote:
> hi all,
>
> today I encountered a race condition in one of the commit hooks,
> which points to a more general problem.
>
> first the specific issue with the PermissionHook. it creates some
> nodes if they don't yet exist (/jcr:system/jcr:permissionStore/default)
>
> this happens with the first commit to a workspace. the problem now is,
> concurrent commits will fails when they are issues right after the
> repository is initialized. to solve this specific issue, we should
> probably move the code to a RepositoryInitializer.
>
> more generally, these kind of race conditions may also happen in
> other editors and hooks, even if the changes of some concurrent
> commits otherwise don't overlap. e.g. creating a versionable node
> will currently create a version history with some more or less
> randomly named ancestor nodes, which may overlap with other
> versionable nodes created at the same time. similarly the p2
> index implementation may introduce a conflict in the :index
> storage because of its hierarchical layout even when changes
> triggering the index update don't conflict.
>
> I'd be interested to hear what people think about this problem.
> should we retry commits when these kind of conflicts where
> introduced by hooks or editor or do we require the latter to
> take care of this and only manipulate content in a non-conflicting
> way?
>
> regards
> marcel
>
Re: race conditions in commit hooks/editors
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Apr 25, 2013 at 3:47 PM, Marcel Reutegger <mr...@adobe.com> wrote:
>> We'd need to revise the MicroKernel contract for that, essentially to
>> require that a commit be rejected if it isn't based on the latest
>> revision. Without separate journals like in the SegmentMK, that would
>> introduce a major scalability bottleneck.
>
> hmm, I don't think so. the contract allows the implementation to merge
> concurrent changes when the commit() (or merge()) isn't done with the
> current head revision.
Right, I was thinking of the specific logic used by the SegmentMK
(based on your wording "same").
The alternative of retry-on-conflict was already covered in the other
part of this thread.
BR,
Jukka Zitting
RE: race conditions in commit hooks/editors
Posted by Marcel Reutegger <mr...@adobe.com>.
> We'd need to revise the MicroKernel contract for that, essentially to
> require that a commit be rejected if it isn't based on the latest
> revision. Without separate journals like in the SegmentMK, that would
> introduce a major scalability bottleneck.
hmm, I don't think so. the contract allows the implementation to merge
concurrent changes when the commit() (or merge()) isn't done with the
current head revision. the contract isn't very specific and definitively
needs an update to include the conflict handling work Michael did for
rebase(), but I don't see why this is a scalability problem.
e.g. the MongoMK performs conflict detection on the MongoDB document
level without the use of a journal or some other global synchronization
point.
the only downside of this design is that it only guarantees snapshot isolation
and permits write skew anomalies. see also previous discussions we had
on this list.
regards
marcel
Re: race conditions in commit hooks/editors
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Apr 25, 2013 at 3:08 PM, Marcel Reutegger <mr...@adobe.com> wrote:
>> Note that the SegmentMK is designed to avoid this problem; it can
>> guarantee that the commit hooks get run against the latest state of
>> the repository and thus can't possibly conflict.
>
> it does it with a retry logic, right?
Yes. If there were other concurrent changes, the current change set is
rebased and commit hooks re-applied before the retry.
> the same could be implemented in the KernelNodeStore...
We'd need to revise the MicroKernel contract for that, essentially to
require that a commit be rejected if it isn't based on the latest
revision. Without separate journals like in the SegmentMK, that would
introduce a major scalability bottleneck.
BR,
Jukka Zitting
RE: race conditions in commit hooks/editors
Posted by Marcel Reutegger <mr...@adobe.com>.
> On Thu, Apr 25, 2013 at 1:23 PM, Marcel Reutegger <mr...@adobe.com>
> wrote:
> > I'd be interested to hear what people think about this problem.
> > should we retry commits when these kind of conflicts where
> > introduced by hooks or editor or do we require the latter to
> > take care of this and only manipulate content in a non-conflicting
> > way?
>
> Note that the SegmentMK is designed to avoid this problem; it can
> guarantee that the commit hooks get run against the latest state of
> the repository and thus can't possibly conflict.
it does it with a retry logic, right? the same could be implemented
in the KernelNodeStore...
> More generally though the best approach here is probably to combine
> the approaches you mention. Commit hooks should generally try to avoid
> creating conflicts (like what the p2 index is already doing), but if
> doing so is not possible or too expensive (as it in many cases may be)
> it would be best if the commit could fail and be retried after
> rebasing.
sounds like a good plan.
regards
marcel
Re: race conditions in commit hooks/editors
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Apr 25, 2013 at 1:23 PM, Marcel Reutegger <mr...@adobe.com> wrote:
> I'd be interested to hear what people think about this problem.
> should we retry commits when these kind of conflicts where
> introduced by hooks or editor or do we require the latter to
> take care of this and only manipulate content in a non-conflicting
> way?
Note that the SegmentMK is designed to avoid this problem; it can
guarantee that the commit hooks get run against the latest state of
the repository and thus can't possibly conflict.
More generally though the best approach here is probably to combine
the approaches you mention. Commit hooks should generally try to avoid
creating conflicts (like what the p2 index is already doing), but if
doing so is not possible or too expensive (as it in many cases may be)
it would be best if the commit could fail and be retried after
rebasing.
BR,
Jukka Zitting