[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16352269#comment-16352269
]
Andrei Dulceanu commented on OAK-7162:
--
Backported to 1.8 branch at r1823155.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch,
> OAK-7162-test.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16352232#comment-16352232
]
Andrei Dulceanu commented on OAK-7162:
--
[~mduerig], thanks for reviewing and for your input! I committed in trunk an
updated version of the test case at r1823149.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch,
> OAK-7162-test.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16352135#comment-16352135
]
Michael Dürig commented on OAK-7162:
[~dulceanu], I agree with your reasoning regarding removing the call to
#{{refreshHead}}. Should there be future cases where the head is changed from
outside the scheduler we should still be on the safe side as #{{refreshHead}}
gets called every time the root node is retrieved. The only difference is
delaying distribution of the content changes from that concurrent modification
for a bit.
A few minor remarks wrt. the patch:
* {{LockBasedSchedulerTest#temporaryFolder}} is not used.
* Please add a message to the the assertion asserting the commit tasks do not
return {{null}}. The message should somehow capture that {{null}} means a
commit got lost because of a race condition.
* There is the {{EmptyHook}} class that you could use in {{#createCommit}}.
* Please add a few words of Javadoc to {{#testSimulatedRaceOnRevisions}}
explaining what the tests is guarding against. This issue's description might
be a good start.
Otherwise the test looks really good, great stuff!
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch,
> OAK-7162-test.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16351715#comment-16351715
]
Andrei Dulceanu commented on OAK-7162:
--
[~mduerig], could you take a look at [^OAK-7162-test.patch]? I simulated the
situation detailed in the issue description, by having a parallel task which
sweeps out everything from the root state by replacing the current revisions
head with an initial head with no content. To be 100% testable (at least for
this kind of test I came up with), the code in {{LockBasedScheduler#execute}}
needs to be changed to omit the call to #{{refreshHead}}, because at this point
the other thread might jump in and erase the modifications just committed by
substituting the correct head with the empty one.
I see no impact of this change in production code, since anyways, this kind of
interaction can happen only with OnRC. The effect is that the next time a
commit is scheduled an additional rebase needs to be done.
As expected, the test case fails consistently on 1.8 branch, without the fix
from OAK-7162 applied.
/cc [~frm]
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch,
> OAK-7162-test.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346521#comment-16346521
]
Michael Dürig commented on OAK-7162:
Reopened since backport to 1.8 is still pending.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16345024#comment-16345024
]
Francesco Mari commented on OAK-7162:
-
[~dulceanu], it looks good to me.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16345021#comment-16345021
]
Andrei Dulceanu commented on OAK-7162:
--
[~mduerig], I attached another patch for taking care the nitpicking. If
everything looks good to you, I'll commit it.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162-03.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344997#comment-16344997
]
Michael Dürig commented on OAK-7162:
Looks good. Just a bit of nitpicking:
* I'd prefer "retrying after" instead of "backing off" as I think the is more
clear.
* Let's not hard code the "13 attempts" but use a counter in the loop. Ugly I
know, but it is more resilient when we later change the value of
{{MAXIMUM_BACKOFF}}.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344962#comment-16344962
]
Andrei Dulceanu commented on OAK-7162:
--
[~mduerig], I agree with both your suggestions. I addressed them in a new
version of the patch. Could you have a look, please?
/cc [~frm]
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162-02.patch, OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344887#comment-16344887
]
Michael Dürig commented on OAK-7162:
Re. the patch:
* Shouldn't the initial call to {{refreshHead()}} go into the back-off loop?
If we end up in a further iteration of that loops this means that someone
changed the head and we should dispatch the changes.
* I would be more explicit with the log messages. I.e. the message should
mention after how much time the retry is going to happen and how many time it
tried already. The final message should probably say how many times the commit
was tried and how long this took overall.
Re. coming up with a test case, let me know if I can be of any help here.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344841#comment-16344841
]
Andrei Dulceanu commented on OAK-7162:
--
[~mduerig], [~frm], could you take a look at the attached patch? Will come back
later with a test case.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.2
>
> Attachments: OAK-7162.patch
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7162) Race condition on revisions head between compaction and scheduler could result in skipped commit
[
https://issues.apache.org/jira/browse/OAK-7162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16327248#comment-16327248
]
Michael Dürig commented on OAK-7162:
Good catch, [~dulceanu]. I agree changing the if statement to a loop would be
the right thing here. To be safe the loop should come with an upper bound, a
back-off and logging.
> Race condition on revisions head between compaction and scheduler could
> result in skipped commit
>
>
> Key: OAK-7162
> URL: https://issues.apache.org/jira/browse/OAK-7162
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: segment-tar
>Affects Versions: 1.8.0
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Blocker
> Labels: scalability
> Fix For: 1.9.0, 1.10, 1.8.1
>
>
> There is a race condition on {{TarRevisions#head}} between a running
> compaction trying to set the new head [0] and the scheduler doing the same
> after executing a specific commit [1]. If the compaction thread is first,
> then the head assignment in the scheduler will fail and not be re-attempted.
> IMO, the simple if statement should be changed to a while loop in which the
> head is refreshed and the commit is re-applied against the new head, before
> attempting again to set a new head in {{TarRevisions}}. This is somehow
> similar to what we previously had [2], but without the unneeded
> optimistic/pessimistic strategies involving tokens.
> [0]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java#L764
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java#L253
> [2]
> https://github.com/apache/jackrabbit-oak/blob/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStore.java#L686
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
