[jira] Commented: (JCR-2786) Cluster sync not always done when calling session.refresh(..)
[
https://issues.apache.org/jira/browse/JCR-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921378#action_12921378
]
Thomas Mueller commented on JCR-2786:
-
The easiest solution is to call syncCount++ before calling journal.sync(), but
I will also replace the volatile syncCount with AtomicInteger.
> Cluster sync not always done when calling session.refresh(..)
> -
>
> Key: JCR-2786
> URL: https://issues.apache.org/jira/browse/JCR-2786
> Project: Jackrabbit Content Repository
> Issue Type: Bug
> Components: clustering
>Reporter: Thomas Mueller
>Assignee: Thomas Mueller
>
> Session.refresh(..) is supposed to synchronize cluster changes, but this
> doesn't always happen, specially if the syncDelay is low. The reason is a
> wrong assumption in ClusterNode.sync: The code there to avoid duplicate sync
> calls doesn't always work as expected. The following algorithm is used:
> int count = syncCount;
> syncLock.acquire();
> if (count == syncCount) {
> journalSync();
> syncCount++;
> }
> syncLock.release();
> The problem is that the background thread might be at the line "syncCount++"
> when Session.refresh(..) is called, so that the main thread believes
> journalSync was already called and thus doesn't call it.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-2786) Cluster sync not always done when calling session.refresh(..)
[
https://issues.apache.org/jira/browse/JCR-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921389#action_12921389
]
Jukka Zitting commented on JCR-2786:
Moving the syncCount increment before the sync() call can cause unnecessary
cluster syncs when multiple sessions are refreshed concurrently.
> Cluster sync not always done when calling session.refresh(..)
> -
>
> Key: JCR-2786
> URL: https://issues.apache.org/jira/browse/JCR-2786
> Project: Jackrabbit Content Repository
> Issue Type: Bug
> Components: clustering
>Reporter: Thomas Mueller
>Assignee: Thomas Mueller
>
> Session.refresh(..) is supposed to synchronize cluster changes, but this
> doesn't always happen, specially if the syncDelay is low. The reason is a
> wrong assumption in ClusterNode.sync: The code there to avoid duplicate sync
> calls doesn't always work as expected. The following algorithm is used:
> int count = syncCount;
> syncLock.acquire();
> if (count == syncCount) {
> journalSync();
> syncCount++;
> }
> syncLock.release();
> The problem is that the background thread might be at the line "syncCount++"
> when Session.refresh(..) is called, so that the main thread believes
> journalSync was already called and thus doesn't call it.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-2786) Cluster sync not always done when calling session.refresh(..)
[
https://issues.apache.org/jira/browse/JCR-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921395#action_12921395
]
Thomas Mueller commented on JCR-2786:
-
Hi Jukka - what do you suggest?
Having some way to enforce a cluster sync is nice; in my view it doesn't need
to be Session.refresh(..).
> Cluster sync not always done when calling session.refresh(..)
> -
>
> Key: JCR-2786
> URL: https://issues.apache.org/jira/browse/JCR-2786
> Project: Jackrabbit Content Repository
> Issue Type: Bug
> Components: clustering
>Reporter: Thomas Mueller
>Assignee: Thomas Mueller
>
> Session.refresh(..) is supposed to synchronize cluster changes, but this
> doesn't always happen, specially if the syncDelay is low. The reason is a
> wrong assumption in ClusterNode.sync: The code there to avoid duplicate sync
> calls doesn't always work as expected. The following algorithm is used:
> int count = syncCount;
> syncLock.acquire();
> if (count == syncCount) {
> journalSync();
> syncCount++;
> }
> syncLock.release();
> The problem is that the background thread might be at the line "syncCount++"
> when Session.refresh(..) is called, so that the main thread believes
> journalSync was already called and thus doesn't call it.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-2786) Cluster sync not always done when calling session.refresh(..)
[
https://issues.apache.org/jira/browse/JCR-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921410#action_12921410
]
Jukka Zitting commented on JCR-2786:
My original thinking behind the syncCount mechanism from JCR-1753 was to skip
the cluster sync if another thread completes the sync after the sync() method
was entered. I missed the case where a thread performs the sync but is then
delayed before it gets to the syncCount++ statement.
Your fix changes the logic from checking whether a sync was completed to
whether a sync was *started* after the sync() method was entered, which raises
the likelihood of extra cluster syncs. However, of the top of my head I don't
see any good way to reliably track the completion of a cluster sync, so for now
I think your solution is the best. At least it can only causes one extra
cluster sync even if n threads were blocked waiting on syncLock.
PS: AtomicInteger enables a more elegant way to implement the
check-and-increment operation:
if (count == syncCount.get()) {
syncCount.incrementAndGet();
...
}
vs.
if (syncCount.compareAndSet(count, count + 1)) {
...;
}
> Cluster sync not always done when calling session.refresh(..)
> -
>
> Key: JCR-2786
> URL: https://issues.apache.org/jira/browse/JCR-2786
> Project: Jackrabbit Content Repository
> Issue Type: Bug
> Components: clustering
>Reporter: Thomas Mueller
>Assignee: Thomas Mueller
>
> Session.refresh(..) is supposed to synchronize cluster changes, but this
> doesn't always happen, specially if the syncDelay is low. The reason is a
> wrong assumption in ClusterNode.sync: The code there to avoid duplicate sync
> calls doesn't always work as expected. The following algorithm is used:
> int count = syncCount;
> syncLock.acquire();
> if (count == syncCount) {
> journalSync();
> syncCount++;
> }
> syncLock.release();
> The problem is that the background thread might be at the line "syncCount++"
> when Session.refresh(..) is called, so that the main thread believes
> journalSync was already called and thus doesn't call it.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
