You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Thomas Mueller (JIRA)" <ji...@apache.org> on 2010/10/15 16:06:34 UTC
[jira] Created: (JCR-2786) Cluster sync not always done when
calling 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(..)
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ 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(..)
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ 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(..)
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ 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(..)
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Resolved: (JCR-2786) Cluster sync not always done when
calling session.refresh(..)
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Thomas Mueller resolved JCR-2786.
---------------------------------
Resolution: Fixed
> 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.