[jira] [Commented] (CURATOR-42) Background guaranteed delete considers NoNode to be a failed delete, and retries it
[ https://issues.apache.org/jira/browse/CURATOR-42?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077447#comment-14077447 ] Cameron McKenzie commented on CURATOR-42: - Just had a look at this, the delete definitely gets retried even though the node doesn't exist. But it only gets retried once. This is because the retry is called with inBackground() with no callback, so the failure no the retry never gets propagated to the code that does another retry. I will fix it so that it doesn't do the initial retry, as it's unnecessary. > Background guaranteed delete considers NoNode to be a failed delete, and > retries it > --- > > Key: CURATOR-42 > URL: https://issues.apache.org/jira/browse/CURATOR-42 > Project: Apache Curator > Issue Type: Bug >Reporter: Shevek >Assignee: Jordan Zimmerman > Fix For: awaiting-response > > > In delete(), NoNode should be a special case which succeeds instantly, and > does not consider the delete failed. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (CURATOR-128) There is no namespace-safe way to refer to the root node
[
https://issues.apache.org/jira/browse/CURATOR-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077398#comment-14077398
]
Cameron McKenzie commented on CURATOR-128:
--
This sounds ok to me. It assumes that no one is basing any logic on the
makePath() method throwing errors for particular cases though. It seems like an
unlikely use case.
> There is no namespace-safe way to refer to the root node
>
>
> Key: CURATOR-128
> URL: https://issues.apache.org/jira/browse/CURATOR-128
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Reporter: Scott Blum
>
> Add the following test to TestNamespaceFacade:
> {code}
> @Test
> public void testRootAccess() throws Exception
> {
> CuratorFrameworkclient =
> CuratorFrameworkFactory.newClient(server.getConnectString(), new
> RetryOneTime(1));
> try
> {
> client.start();
> client.create().forPath("/one");
>
> Assert.assertNotNull(client.getZookeeperClient().getZooKeeper().exists("/one",
> false));
> Assert.assertNotNull(client.checkExists().forPath("/"));
> try
> {
> client.checkExists().forPath("");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
>
> Assert.assertNotNull(client.usingNamespace("one").checkExists().forPath(""));
> try
> {
> client.usingNamespace("one").checkExists().forPath("/");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
> }
> finally
> {
> CloseableUtils.closeQuietly(client);
> }
> }
> {code}
> This tests PASSES, which means that there's no canonical way to refer to the
> root node. If the client is not namespaced, "/" works and "" does not work.
> If the client is namespaced, "" works and "/" does not.
> In either case, I think ZKPaths.makePath mishandles certain cases.
> If you append "/foo" and "/" the result is "/foo/" which is an invalid path.
> On the other hand, if you append "" and "bar" the result is "//bar" which is
> also invalid.
> What's the right behavior here? Does the root node / root of a namespace
> always need to be referred to as "/" or is empty string an acceptable alias?
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-128) There is no namespace-safe way to refer to the root node
[
https://issues.apache.org/jira/browse/CURATOR-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077388#comment-14077388
]
Scott Blum commented on CURATOR-128:
It couldn't be made more lenient?
e.g.
"/" + "/foo" = "/foo"
"" + "foo" = "/foo
"/" + "foo" = "/foo"
"" + "/foo" = "/foo
> There is no namespace-safe way to refer to the root node
>
>
> Key: CURATOR-128
> URL: https://issues.apache.org/jira/browse/CURATOR-128
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Reporter: Scott Blum
>
> Add the following test to TestNamespaceFacade:
> {code}
> @Test
> public void testRootAccess() throws Exception
> {
> CuratorFrameworkclient =
> CuratorFrameworkFactory.newClient(server.getConnectString(), new
> RetryOneTime(1));
> try
> {
> client.start();
> client.create().forPath("/one");
>
> Assert.assertNotNull(client.getZookeeperClient().getZooKeeper().exists("/one",
> false));
> Assert.assertNotNull(client.checkExists().forPath("/"));
> try
> {
> client.checkExists().forPath("");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
>
> Assert.assertNotNull(client.usingNamespace("one").checkExists().forPath(""));
> try
> {
> client.usingNamespace("one").checkExists().forPath("/");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
> }
> finally
> {
> CloseableUtils.closeQuietly(client);
> }
> }
> {code}
> This tests PASSES, which means that there's no canonical way to refer to the
> root node. If the client is not namespaced, "/" works and "" does not work.
> If the client is namespaced, "" works and "/" does not.
> In either case, I think ZKPaths.makePath mishandles certain cases.
> If you append "/foo" and "/" the result is "/foo/" which is an invalid path.
> On the other hand, if you append "" and "bar" the result is "//bar" which is
> also invalid.
> What's the right behavior here? Does the root node / root of a namespace
> always need to be referred to as "/" or is empty string an acceptable alias?
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-128) There is no namespace-safe way to refer to the root node
[
https://issues.apache.org/jira/browse/CURATOR-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077258#comment-14077258
]
Jordan Zimmerman commented on CURATOR-128:
--
The problem in ZKPaths.makePath() is longstanding but there's not much that can
be done about it now. It would break too much existing code.
> There is no namespace-safe way to refer to the root node
>
>
> Key: CURATOR-128
> URL: https://issues.apache.org/jira/browse/CURATOR-128
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Reporter: Scott Blum
>
> Add the following test to TestNamespaceFacade:
> {code}
> @Test
> public void testRootAccess() throws Exception
> {
> CuratorFrameworkclient =
> CuratorFrameworkFactory.newClient(server.getConnectString(), new
> RetryOneTime(1));
> try
> {
> client.start();
> client.create().forPath("/one");
>
> Assert.assertNotNull(client.getZookeeperClient().getZooKeeper().exists("/one",
> false));
> Assert.assertNotNull(client.checkExists().forPath("/"));
> try
> {
> client.checkExists().forPath("");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
>
> Assert.assertNotNull(client.usingNamespace("one").checkExists().forPath(""));
> try
> {
> client.usingNamespace("one").checkExists().forPath("/");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
> }
> finally
> {
> CloseableUtils.closeQuietly(client);
> }
> }
> {code}
> This tests PASSES, which means that there's no canonical way to refer to the
> root node. If the client is not namespaced, "/" works and "" does not work.
> If the client is namespaced, "" works and "/" does not.
> In either case, I think ZKPaths.makePath mishandles certain cases.
> If you append "/foo" and "/" the result is "/foo/" which is an invalid path.
> On the other hand, if you append "" and "bar" the result is "//bar" which is
> also invalid.
> What's the right behavior here? Does the root node / root of a namespace
> always need to be referred to as "/" or is empty string an acceptable alias?
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-128) There is no namespace-safe way to refer to the root node
[
https://issues.apache.org/jira/browse/CURATOR-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077241#comment-14077241
]
Cameron McKenzie commented on CURATOR-128:
--
I think that it would be preferable to be consistent. So, "/" would refer to
the root of the namespace, or the root if non namespace is used.
To fix this without potentially breaking peoples code would rely on making "/"
and "" synonymous. I think it would be cleaner to always refer to the root as
"/" though.
> There is no namespace-safe way to refer to the root node
>
>
> Key: CURATOR-128
> URL: https://issues.apache.org/jira/browse/CURATOR-128
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Reporter: Scott Blum
>
> Add the following test to TestNamespaceFacade:
> {code}
> @Test
> public void testRootAccess() throws Exception
> {
> CuratorFrameworkclient =
> CuratorFrameworkFactory.newClient(server.getConnectString(), new
> RetryOneTime(1));
> try
> {
> client.start();
> client.create().forPath("/one");
>
> Assert.assertNotNull(client.getZookeeperClient().getZooKeeper().exists("/one",
> false));
> Assert.assertNotNull(client.checkExists().forPath("/"));
> try
> {
> client.checkExists().forPath("");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
>
> Assert.assertNotNull(client.usingNamespace("one").checkExists().forPath(""));
> try
> {
> client.usingNamespace("one").checkExists().forPath("/");
> Assert.fail("IllegalArgumentException expected");
> }
> catch ( IllegalArgumentException expected )
> {
> }
> }
> finally
> {
> CloseableUtils.closeQuietly(client);
> }
> }
> {code}
> This tests PASSES, which means that there's no canonical way to refer to the
> root node. If the client is not namespaced, "/" works and "" does not work.
> If the client is namespaced, "" works and "/" does not.
> In either case, I think ZKPaths.makePath mishandles certain cases.
> If you append "/foo" and "/" the result is "/foo/" which is an invalid path.
> On the other hand, if you append "" and "bar" the result is "//bar" which is
> also invalid.
> What's the right behavior here? Does the root node / root of a namespace
> always need to be referred to as "/" or is empty string an acceptable alias?
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Created] (CURATOR-128) There is no namespace-safe way to refer to the root node
Scott Blum created CURATOR-128:
--
Summary: There is no namespace-safe way to refer to the root node
Key: CURATOR-128
URL: https://issues.apache.org/jira/browse/CURATOR-128
Project: Apache Curator
Issue Type: Bug
Components: Framework
Reporter: Scott Blum
Add the following test to TestNamespaceFacade:
{code}
@Test
public void testRootAccess() throws Exception
{
CuratorFrameworkclient =
CuratorFrameworkFactory.newClient(server.getConnectString(), new
RetryOneTime(1));
try
{
client.start();
client.create().forPath("/one");
Assert.assertNotNull(client.getZookeeperClient().getZooKeeper().exists("/one",
false));
Assert.assertNotNull(client.checkExists().forPath("/"));
try
{
client.checkExists().forPath("");
Assert.fail("IllegalArgumentException expected");
}
catch ( IllegalArgumentException expected )
{
}
Assert.assertNotNull(client.usingNamespace("one").checkExists().forPath(""));
try
{
client.usingNamespace("one").checkExists().forPath("/");
Assert.fail("IllegalArgumentException expected");
}
catch ( IllegalArgumentException expected )
{
}
}
finally
{
CloseableUtils.closeQuietly(client);
}
}
{code}
This tests PASSES, which means that there's no canonical way to refer to the
root node. If the client is not namespaced, "/" works and "" does not work.
If the client is namespaced, "" works and "/" does not.
In either case, I think ZKPaths.makePath mishandles certain cases.
If you append "/foo" and "/" the result is "/foo/" which is an invalid path.
On the other hand, if you append "" and "bar" the result is "//bar" which is
also invalid.
What's the right behavior here? Does the root node / root of a namespace
always need to be referred to as "/" or is empty string an acceptable alias?
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077185#comment-14077185
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50420280
Sweet, looks good.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50420280 Sweet, looks good. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Resolved] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jordan Zimmerman resolved CURATOR-126.
--
Resolution: Fixed
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077168#comment-14077168
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50419120
Sorry Scott - it looks like CURATOR-126 hasn’t sync’d yet to the Github
repo. The whole Github sync think is a lot of black magic. It is, however,
here:
https://git-wip-us.apache.org/repos/asf?p=curator.git;a=tree;h=refs/heads/CURATOR-126;hb=CURATOR-126
From: Scott Blum
Reply: apache/curator >
Date: July 28, 2014 at 7:03:46 PM
To: apache/curator >
Cc: Jordan Zimmerman >
Subject: Re: [curator] CURATOR-126: Fix race condition in
CuratorFrameworkImpl.close() (#23)
Where are the commits? I did a git remote update on both my fork and apache
and couldn't find them.
—
Reply to this email directly or view it on GitHub.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50419120 Sorry Scott - it looks like CURATOR-126 hasnât syncâd yet to the Github repo. The whole Github sync think is a lot of black magic. It is, however, here:Â https://git-wip-us.apache.org/repos/asf?p=curator.git;a=tree;h=refs/heads/CURATOR-126;hb=CURATOR-126 From:Â Scott Blum Reply:Â apache/curator > Date:Â July 28, 2014 at 7:03:46 PM To:Â apache/curator > Cc:Â Jordan Zimmerman > Subject:Â Re: [curator] CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() (#23) Where are the commits? I did a git remote update on both my fork and apache and couldn't find them. â Reply to this email directly or view it on GitHub. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077166#comment-14077166
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50418934
Where are the commits? I did a git remote update on both my fork and
apache and couldn't find them.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077167#comment-14077167
]
ASF GitHub Bot commented on CURATOR-126:
Github user cammckenzie commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50419008
They're there for me, I just pulled off the apache master (CURATOR-126
branch). The changes look good to me Jordan, I was trying to avoid changing the
CuratorFrameworkImpl to add additional debug stuff, but I think it's the way to
go.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user cammckenzie commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50419008 They're there for me, I just pulled off the apache master (CURATOR-126 branch). The changes look good to me Jordan, I was trying to avoid changing the CuratorFrameworkImpl to add additional debug stuff, but I think it's the way to go. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50418934 Where are the commits? I did a git remote update on both my fork and apache and couldn't find them. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50418758 I pushed an update to the test that has an assert. I tested it with the old background thread code and the test fails. It succeeds with the new code. -JZ From:Â cammckenzie Reply:Â apache/curator > Date:Â July 28, 2014 at 6:29:46 PM To:Â apache/curator > Cc:Â Jordan Zimmerman > Subject:Â Re: [curator] CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() (#23) I have pushed a unit test up onto the CURATOR-126 branch. Have a play with it, I still seem to be getting the error logged when running against the fix. Could be something bogus in the test though. â Reply to this email directly or view it on GitHub. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077164#comment-14077164
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50418758
I pushed an update to the test that has an assert. I tested it with the old
background thread code and the test fails. It succeeds with the new code.
-JZ
From: cammckenzie
Reply: apache/curator >
Date: July 28, 2014 at 6:29:46 PM
To: apache/curator >
Cc: Jordan Zimmerman >
Subject: Re: [curator] CURATOR-126: Fix race condition in
CuratorFrameworkImpl.close() (#23)
I have pushed a unit test up onto the CURATOR-126 branch. Have a play with
it, I still seem to be getting the error logged when running against the fix.
Could be something bogus in the test though.
—
Reply to this email directly or view it on GitHub.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077122#comment-14077122
]
ASF GitHub Bot commented on CURATOR-126:
Github user cammckenzie commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50416483
I have pushed a unit test up onto the CURATOR-126 branch. Have a play with
it, I still seem to be getting the error logged when running against the fix.
Could be something bogus in the test though.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user cammckenzie commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50416483 I have pushed a unit test up onto the CURATOR-126 branch. Have a play with it, I still seem to be getting the error logged when running against the fix. Could be something bogus in the test though. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076966#comment-14076966
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50407804
That would be great. If you can send me a commit or diff, I'd be happy to
play with trying to test it programmatically.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50407804 That would be great. If you can send me a commit or diff, I'd be happy to play with trying to test it programmatically. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: FYI - changing 2.6.1 to 2.7.0
I’m not a fan of cherry picking, etc. I’ve seen too many mistakes made. Officially, Curator follows Github Flow: http://scottchacon.com/2011/08/31/github-flow.html - which boils down to have a master and lots of other branches. So, maybe even having a 2.7.0 branch would be a mistake. Features ready to be released are in master and everything else is just waiting in its branch. -Jordan From: Mike Drob Reply: dev@curator.apache.org > Date: July 28, 2014 at 4:49:56 PM To: dev@curator.apache.org > Cc: Cameron McKenzie > Subject: Re: FYI - changing 2.6.1 to 2.7.0 might make more sense to create a 2.6.1 branch from the 2.6.0 release tag and cherry pick all of the relevant commits in. leave master as pointing to 2.7.0. if you have both branches active at once then commit to master and continue to cherry-pick as appropriate. or commit to the branch and merge to master, that one is probably easier for committers but harder for contributors so i'd advise against it. then, when you release 2.6.1, just kill the branch, and if you decide to do a 2.6.2 then create a new one at that point. On Mon, Jul 28, 2014 at 4:43 PM, Jordan Zimmerman < jor...@jordanzimmerman.com> wrote: > That’s a good point. I think, we’d have master just be whatever the next > release will be. Then, we could have a 2.7.0 branch that would become > master after 2.6.1 is released. OK? > > -JZ > > From: Cameron McKenzie > Reply: dev@curator.apache.org > > Date: July 28, 2014 at 4:42:57 PM > To: dev@curator.apache.org > > Subject: Re: FYI - changing 2.6.1 to 2.7.0 > > How would we manage this with our current branching structure? > > > On Tue, Jul 29, 2014 at 7:36 AM, Mike Drob wrote: > > > Is it possible (or desirable?) to split some of the bug fixes into a > 2.6.1 > > before adding the APIs for 2.7.0? > > > > > > On Mon, Jul 28, 2014 at 4:34 PM, Jordan Zimmerman < > > jor...@jordanzimmerman.com> wrote: > > > > > FYI > > > > > > There will be some new APIs in the next release (CURATOR-126 for > example) > > > that suggests making this 2.7.0 instead of 2.6.1 > > > > > > -JZ > > > > > > > > >
Re: FYI - changing 2.6.1 to 2.7.0
might make more sense to create a 2.6.1 branch from the 2.6.0 release tag and cherry pick all of the relevant commits in. leave master as pointing to 2.7.0. if you have both branches active at once then commit to master and continue to cherry-pick as appropriate. or commit to the branch and merge to master, that one is probably easier for committers but harder for contributors so i'd advise against it. then, when you release 2.6.1, just kill the branch, and if you decide to do a 2.6.2 then create a new one at that point. On Mon, Jul 28, 2014 at 4:43 PM, Jordan Zimmerman < jor...@jordanzimmerman.com> wrote: > That’s a good point. I think, we’d have master just be whatever the next > release will be. Then, we could have a 2.7.0 branch that would become > master after 2.6.1 is released. OK? > > -JZ > > From: Cameron McKenzie > Reply: dev@curator.apache.org > > Date: July 28, 2014 at 4:42:57 PM > To: dev@curator.apache.org > > Subject: Re: FYI - changing 2.6.1 to 2.7.0 > > How would we manage this with our current branching structure? > > > On Tue, Jul 29, 2014 at 7:36 AM, Mike Drob wrote: > > > Is it possible (or desirable?) to split some of the bug fixes into a > 2.6.1 > > before adding the APIs for 2.7.0? > > > > > > On Mon, Jul 28, 2014 at 4:34 PM, Jordan Zimmerman < > > jor...@jordanzimmerman.com> wrote: > > > > > FYI > > > > > > There will be some new APIs in the next release (CURATOR-126 for > example) > > > that suggests making this 2.7.0 instead of 2.6.1 > > > > > > -JZ > > > > > > > > >
Re: FYI - changing 2.6.1 to 2.7.0
That’s a good point. I think, we’d have master just be whatever the next release will be. Then, we could have a 2.7.0 branch that would become master after 2.6.1 is released. OK? -JZ From: Cameron McKenzie Reply: dev@curator.apache.org > Date: July 28, 2014 at 4:42:57 PM To: dev@curator.apache.org > Subject: Re: FYI - changing 2.6.1 to 2.7.0 How would we manage this with our current branching structure? On Tue, Jul 29, 2014 at 7:36 AM, Mike Drob wrote: > Is it possible (or desirable?) to split some of the bug fixes into a 2.6.1 > before adding the APIs for 2.7.0? > > > On Mon, Jul 28, 2014 at 4:34 PM, Jordan Zimmerman < > jor...@jordanzimmerman.com> wrote: > > > FYI > > > > There will be some new APIs in the next release (CURATOR-126 for example) > > that suggests making this 2.7.0 instead of 2.6.1 > > > > -JZ > > > > >
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076932#comment-14076932
]
ASF GitHub Bot commented on CURATOR-126:
Github user cammckenzie commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50406175
Do you have a unit test to reproduce? I cooked one up which could reproduce
the issue, but I hadn't found a way for it to work with assertions. I could
only verify that the exception was being logged. I can still commit it though
if it's considered of use.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076934#comment-14076934
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/23#issuecomment-50406296
Yeah - please do. Can’t hurt.
From: cammckenzie
Reply: apache/curator >
Date: July 28, 2014 at 4:41:57 PM
To: apache/curator >
Cc: Jordan Zimmerman >
Subject: Re: [curator] CURATOR-126: Fix race condition in
CuratorFrameworkImpl.close() (#23)
Do you have a unit test to reproduce? I cooked one up which could reproduce
the issue, but I hadn't found a way for it to work with assertions. I could
only verify that the exception was being logged. I can still commit it though
if it's considered of use.
—
Reply to this email directly or view it on GitHub.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50406296 Yeah - please do. Canât hurt. From:Â cammckenzie Reply:Â apache/curator > Date:Â July 28, 2014 at 4:41:57 PM To:Â apache/curator > Cc:Â Jordan Zimmerman > Subject:Â Re: [curator] CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() (#23) Do you have a unit test to reproduce? I cooked one up which could reproduce the issue, but I hadn't found a way for it to work with assertions. I could only verify that the exception was being logged. I can still commit it though if it's considered of use. â Reply to this email directly or view it on GitHub. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: FYI - changing 2.6.1 to 2.7.0
How would we manage this with our current branching structure? On Tue, Jul 29, 2014 at 7:36 AM, Mike Drob wrote: > Is it possible (or desirable?) to split some of the bug fixes into a 2.6.1 > before adding the APIs for 2.7.0? > > > On Mon, Jul 28, 2014 at 4:34 PM, Jordan Zimmerman < > jor...@jordanzimmerman.com> wrote: > > > FYI > > > > There will be some new APIs in the next release (CURATOR-126 for example) > > that suggests making this 2.7.0 instead of 2.6.1 > > > > -JZ > > > > >
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user cammckenzie commented on the pull request: https://github.com/apache/curator/pull/23#issuecomment-50406175 Do you have a unit test to reproduce? I cooked one up which could reproduce the issue, but I hadn't found a way for it to work with assertions. I could only verify that the exception was being logged. I can still commit it though if it's considered of use. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: FYI - changing 2.6.1 to 2.7.0
Is it possible (or desirable?) to split some of the bug fixes into a 2.6.1 before adding the APIs for 2.7.0? On Mon, Jul 28, 2014 at 4:34 PM, Jordan Zimmerman < jor...@jordanzimmerman.com> wrote: > FYI > > There will be some new APIs in the next release (CURATOR-126 for example) > that suggests making this 2.7.0 instead of 2.6.1 > > -JZ > >
FYI - changing 2.6.1 to 2.7.0
FYI There will be some new APIs in the next release (CURATOR-126 for example) that suggests making this 2.7.0 instead of 2.6.1 -JZ
[jira] [Updated] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jordan Zimmerman updated CURATOR-126:
-
Fix Version/s: 2.6.1
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Assigned] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jordan Zimmerman reassigned CURATOR-126:
Assignee: Jordan Zimmerman (was: Cameron McKenzie)
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Jordan Zimmerman
> Fix For: 2.7.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076878#comment-14076878
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15491274
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
---
@@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int
connectionTimeoutMs)
}
/**
+ * @param closeWaitMs time to wait during close to join background
threads
--- End diff --
done; what about the constant?
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076884#comment-14076884
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15491355
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
---
@@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int
connectionTimeoutMs)
}
/**
+ * @param closeWaitMs time to wait during close to join background
threads
--- End diff --
I think that’s less important as it’s not visible. Up to you.
From: Scott Blum
Reply: apache/curator >
Date: July 28, 2014 at 4:13:43 PM
To: apache/curator >
Cc: Jordan Zimmerman >
Subject: Re: [curator] CURATOR-126: Fix race condition in
CuratorFrameworkImpl.close() (#23)
In
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java:
> @@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int
connectionTimeoutMs)
> }
>
> /**
> + * @param closeWaitMs time to wait during close to join
background threads
done; what about the constant?
—
Reply to this email directly or view it on GitHub.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15491355 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java --- @@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int connectionTimeoutMs) } /** + * @param closeWaitMs time to wait during close to join background threads --- End diff -- I think thatâs less important as itâs not visible. Up to you. From:Â Scott Blum Reply:Â apache/curator > Date:Â July 28, 2014 at 4:13:43 PM To:Â apache/curator > Cc:Â Jordan Zimmerman > Subject:Â Re: [curator] CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() (#23) In curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java: > @@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int connectionTimeoutMs) > } > > /** > + * @param closeWaitMs time to wait during close to join background threads done; what about the constant? â Reply to this email directly or view it on GitHub. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15491274 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java --- @@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int connectionTimeoutMs) } /** + * @param closeWaitMs time to wait during close to join background threads --- End diff -- done; what about the constant? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076842#comment-14076842
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15490376
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
---
@@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int
connectionTimeoutMs)
}
/**
+ * @param closeWaitMs time to wait during close to join background
threads
--- End diff --
Sorry to be pedantic, but this would be better named "maxCloseWaitMs".
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15490376 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java --- @@ -239,6 +241,16 @@ public Builder connectionTimeoutMs(int connectionTimeoutMs) } /** + * @param closeWaitMs time to wait during close to join background threads --- End diff -- Sorry to be pedantic, but this would be better named "maxCloseWaitMs". --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076839#comment-14076839
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15490243
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
Fair point, I'll revert.
BTW: I tried tracing back through the code and was not able to locate the
code path from ExecutorService.shutdownNow() -> FutureTask.cancel() under 1.6.
So I might actually be confused and thinking of a Guava executor and not a JDK
one, it's possible the JDK shutdownNow() is actually safe. If so, apologizes
for the mis-info.
Either way, CloseableExecutorService.close() probably does have the issue
since it does an explicit cancel(true).
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15490243 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- Fair point, I'll revert. BTW: I tried tracing back through the code and was not able to locate the code path from ExecutorService.shutdownNow() -> FutureTask.cancel() under 1.6. So I might actually be confused and thinking of a Guava executor and not a JDK one, it's possible the JDK shutdownNow() is actually safe. If so, apologizes for the mis-info. Either way, CloseableExecutorService.close() probably does have the issue since it does an explicit cancel(true). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076766#comment-14076766
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15488051
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
I'm just concerned it's a larger problem. Executors are used throughout
Curator and I've relied on shutdownNow to work correctly. I didn't know about
the bug. So, I'd feel better having an open issue to deal with it directly.
Curator already has the CloseableExecutorService wrapper and my gut feeling is
that it can be fixed inside of there.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15488051 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- I'm just concerned it's a larger problem. Executors are used throughout Curator and I've relied on shutdownNow to work correctly. I didn't know about the bug. So, I'd feel better having an open issue to deal with it directly. Curator already has the CloseableExecutorService wrapper and my gut feeling is that it can be fixed inside of there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076754#comment-14076754
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15487723
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
Potentially. I guess if you feel super strongly I can revert that part of
the change and just add an awaitTermination.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15487369 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- But doesn't that mean that Executor.shutdownNow() is broken throughout the code? If so, I think there should be a separate issue to fix it globally. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15487723 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- Potentially. I guess if you feel super strongly I can revert that part of the change and just add an awaitTermination. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076743#comment-14076743
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15487369
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
But doesn't that mean that Executor.shutdownNow() is broken throughout the
code? If so, I think there should be a separate issue to fix it globally.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076733#comment-14076733
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15487030
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
The bug is simple: Executor.shutdownNow() calls FutureTask.cancel(), which
on older JDK versions has a bug where it can interrupt the wrong thread,
causing all sorts of problems. Manually interrupting a thread does not have
this problem. Given that this Executor is only every used to run one task (the
backgroundOperationsLoop) we can simply switch to a Thread and avoid that bug.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076736#comment-14076736
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15487087
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
Done. I set the default to 1 second, let me know if should be different.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15487087
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
Done. I set the default to 1 second, let me know if should be different.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15487030 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- The bug is simple: Executor.shutdownNow() calls FutureTask.cancel(), which on older JDK versions has a bug where it can interrupt the wrong thread, causing all sorts of problems. Manually interrupting a thread does not have this problem. Given that this Executor is only every used to run one task (the backgroundOperationsLoop) we can simply switch to a Thread and avoid that bug. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076718#comment-14076718
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486439
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
I just checked - that's the only instance of it in the code. If you don't
mind, please fix it as part of this PR.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076715#comment-14076715
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486333
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Actually, that's a bug! I didn't see it. It should be
Thread.currentThread().isInterrupted() which doesn't clear the state. Normally
I use that one. I'm going to greg the code for other instances of this.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486439
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
I just checked - that's the only instance of it in the code. If you don't
mind, please fix it as part of this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486333
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Actually, that's a bug! I didn't see it. It should be
Thread.currentThread().isInterrupted() which doesn't clear the state. Normally
I use that one. I'm going to greg the code for other instances of this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076709#comment-14076709
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486219
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Let me be more clear. The way the loop is constructed:
```
private void backgroundOperationsLoop()
{
while ( !Thread.interrupted() )
{ ... }
```
ALREADY eats the interrupted status. Simply checking
`Thread.interrupted()` consumes it. If you want to consistently enforce a rule
that you always re-interrupt threads (which is a good rule in general, although
not necessary here) then you need an unconditional re-interrupt at the end of
the method.
Do you want me to add that?
My point is that putting the interrupt only in the catch block is
inconsistent. It re-interrupts in the case where an InterruptedException gets
throws, and fails to re-interrupt when the loop exits without exception.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15486219
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Let me be more clear. The way the loop is constructed:
```
private void backgroundOperationsLoop()
{
while ( !Thread.interrupted() )
{ ... }
```
ALREADY eats the interrupted status. Simply checking
`Thread.interrupted()` consumes it. If you want to consistently enforce a rule
that you always re-interrupt threads (which is a good rule in general, although
not necessary here) then you need an unconditional re-interrupt at the end of
the method.
Do you want me to add that?
My point is that putting the interrupt only in the catch block is
inconsistent. It re-interrupts in the case where an InterruptedException gets
throws, and fails to re-interrupt when the loop exits without exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076701#comment-14076701
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485966
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
Sure thing.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concur
[jira] [Issue Comment Deleted] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jordan Zimmerman updated CURATOR-125: - Comment: was deleted (was: GitHub user dragonsinth opened a pull request: https://github.com/apache/curator/pull/22 CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() You can merge this pull request into a Git repository by running: $ git pull https://github.com/dragonsinth/curator CURATOR-125 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/22.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22 commit e3b93f221b2a43c93d71a0f51510fc5f83b51968 Author: Scott Blum Date: 2014-07-28T18:10:37Z CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() ) > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Issue Comment Deleted] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jordan Zimmerman updated CURATOR-125: - Comment: was deleted (was: Github user dragonsinth closed the pull request at: https://github.com/apache/curator/pull/22 ) > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485966
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
Sure thing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Issue Comment Deleted] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jordan Zimmerman updated CURATOR-125: - Comment: was deleted (was: Please delete the above 3 comments (and this one) opened a PR with the wrong number.) > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Issue Comment Deleted] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jordan Zimmerman updated CURATOR-125: - Comment: was deleted (was: Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/22#issuecomment-50380674 oops wrong number ) > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076696#comment-14076696
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485833
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -74,7 +71,7 @@
private final NamespaceFacadeCache namespaceFacadeCache;
private final NamespaceWatcherMap namespaceWatcherMap = new
NamespaceWatcherMap(this);
-private volatile ExecutorService executorService;
+private volatile Thread backgroundThread;
--- End diff --
I disagree with changing this to Thread. I understand that there are some
bugs with Executor but that's a much larger issues. Curator uses Executor
everywhere. If the Executor bug will exhibit here it will do so everywhere. We
can consider a global fix for the bug (which I don't yet understand).
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/23#discussion_r15485833 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java --- @@ -74,7 +71,7 @@ private final NamespaceFacadeCache namespaceFacadeCache; private final NamespaceWatcherMap namespaceWatcherMap = new NamespaceWatcherMap(this); -private volatile ExecutorService executorService; +private volatile Thread backgroundThread; --- End diff -- I disagree with changing this to Thread. I understand that there are some bugs with Executor but that's a much larger issues. Curator uses Executor everywhere. If the Executor bug will exhibit here it will do so everywhere. We can consider a global fix for the bug (which I don't yet understand). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076688#comment-14076688
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485690
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Threads should always be re-interrupted. It may be that a break statement
should be added.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076691#comment-14076691
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485742
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
This is not correct. _Always_ re-set the interrupted state of threads. The
break statement exists the while condition.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485742
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
This is not correct. _Always_ re-set the interrupted state of threads. The
break statement exists the while condition.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485690
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
Threads should always be re-interrupted. It may be that a break statement
should be added.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076686#comment-14076686
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485603
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
Yes, please revert. Create a separate PR if you like.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.ja
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076684#comment-14076684
]
ASF GitHub Bot commented on CURATOR-126:
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485565
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
Add a field to the CuratorFrameworkFactory builder so that this value can
be configured.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485603
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
Yes, please revert. Create a separate PR if you like.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15485565
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
Add a field to the CuratorFrameworkFactory builder so that this value can
be configured.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076617#comment-14076617
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483300
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
I totally made this number up. What's the right thing here?
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483300
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -300,12 +295,20 @@ public Void apply(CuratorListener listener)
listeners.clear();
unhandledErrorListeners.clear();
connectionStateManager.close();
+if (backgroundThread != null) {
+backgroundThread.interrupt();
+try
+{
+backgroundThread.join(1000);
--- End diff --
I totally made this number up. What's the right thing here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076610#comment-14076610
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483196
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
not necessary; also inconsistent with the while condition. If the loop
exits because the while condition test reads the interrupted value, it's not
resetting the interrupted flag anyway, so there's no point doing it on this
exit path either.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483196
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -770,9 +769,8 @@ private void backgroundOperationsLoop()
debugListener.listen(operationAndData);
}
}
-catch ( InterruptedException e )
+catch ( InterruptedException ignored )
{
-Thread.currentThread().interrupt();
--- End diff --
not necessary; also inconsistent with the while condition. If the loop
exits because the while condition test reads the interrupted value, it's not
resetting the interrupted flag anyway, so there's no point doing it on this
exit path either.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076603#comment-14076603
]
ASF GitHub Bot commented on CURATOR-126:
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483096
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
This is unrelated to the main change. While I was in here, I noticed this
method was more complicated than it needed to be. I could revert this section
if you guys would prefer to isolate changes.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
>
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/curator/pull/23#discussion_r15483096
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
---
@@ -722,39 +725,35 @@ public void retriesExhausted(OperationAndData
operationAndData)
private void
handleBackgroundOperationException(OperationAndData
operationAndData, Throwable e)
{
-do
+if ( (operationAndData != null) && RetryLoop.isRetryException(e) )
{
-if ( (operationAndData != null) &&
RetryLoop.isRetryException(e) )
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
+{
+log.debug("Retry-able exception received", e);
+}
+if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
{
if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-log.debug("Retry-able exception received", e);
+log.debug("Retrying operation");
}
-if (
client.getRetryPolicy().allowRetry(operationAndData.getThenIncrementRetryCount(),
operationAndData.getElapsedTimeMs(), operationAndData) )
+backgroundOperations.offer(operationAndData);
+return;
+}
+else
+{
+if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retrying operation");
-}
-backgroundOperations.offer(operationAndData);
-break;
+log.debug("Retry policy did not allow retry");
}
-else
+if ( operationAndData.getErrorCallback() != null )
{
-if (
!Boolean.getBoolean(DebugUtils.PROPERTY_DONT_LOG_CONNECTION_ISSUES) )
-{
-log.debug("Retry policy did not allow retry");
-}
-if ( operationAndData.getErrorCallback() != null )
-{
-
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
-}
+
operationAndData.getErrorCallback().retriesExhausted(operationAndData);
}
}
-
-logError("Background exception was not retry-able or retry
gave up", e);
}
-while ( false );
+
+logError("Background exception was not retry-able or retry gave
up", e);
--- End diff --
This is unrelated to the main change. While I was in here, I noticed this
method was more complicated than it needed to be. I could revert this section
if you guys would prefer to isolate changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076576#comment-14076576
]
Scott Blum commented on CURATOR-126:
You're right, Thread.interrupt(), Thread.join() is almost identical to
Executor.shutdownNow(), Executor.awaitTermination() EXCEPT for the JDK bug
which affects Executor.shutdownNow().
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7132378
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076573#comment-14076573 ] Scott Blum commented on CURATOR-125: Please delete the above 3 comments (and this one) opened a PR with the wrong number. > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076565#comment-14076565 ] ASF GitHub Bot commented on CURATOR-125: GitHub user dragonsinth opened a pull request: https://github.com/apache/curator/pull/22 CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() You can merge this pull request into a Git repository by running: $ git pull https://github.com/dragonsinth/curator CURATOR-125 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/22.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22 commit e3b93f221b2a43c93d71a0f51510fc5f83b51968 Author: Scott Blum Date: 2014-07-28T18:10:37Z CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076567#comment-14076567 ] ASF GitHub Bot commented on CURATOR-125: Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/22#issuecomment-50380674 oops wrong number > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076570#comment-14076570
]
ASF GitHub Bot commented on CURATOR-126:
GitHub user dragonsinth opened a pull request:
https://github.com/apache/curator/pull/23
CURATOR-126: Fix race condition in CuratorFrameworkImpl.close()
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dragonsinth/curator CURATOR-126
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/curator/pull/23.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #23
commit 247e02152a96b58f344439d589d02065f1bd2632
Author: Scott Blum
Date: 2014-07-28T18:10:37Z
CURATOR-126: Fix race condition in CuratorFrameworkImpl.close()
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-125) ConnectionStateListener is confused by READ_ONLY state
[ https://issues.apache.org/jira/browse/CURATOR-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076568#comment-14076568 ] ASF GitHub Bot commented on CURATOR-125: Github user dragonsinth closed the pull request at: https://github.com/apache/curator/pull/22 > ConnectionStateListener is confused by READ_ONLY state > -- > > Key: CURATOR-125 > URL: https://issues.apache.org/jira/browse/CURATOR-125 > Project: Apache Curator > Issue Type: Bug > Components: Client >Affects Versions: 2.6.0 > Environment: Ubuntu 12.04 > 3 ZK with readonlymode.enabled >Reporter: Benjamin Jaton > Attachments: Test.java > > > To reproduce: > - have a 3 nodes ZK ensemble with readonlymode.enabled > - shut down 2 of the 3 ZK servers ( we start in read only mode) > Then create a piece of code (see Test.java attached): > - a curator client (keep the timeout reasonably short) > - a NodeCache listener on '/' > - a separate ZooKeeper client > -> the connection goes into READ_ONLY/ConnectedReadOnly as expected > - start another ZooKeeper > -> the connection goes into SUSPENDED/Disconnected, then > CONNECTED/SyncConnected, fine. > - stop one of the 2 ZooKeeper alive > -> the connection goes: > ZOOKEEPER STATE: Disconnected > CURATOR STATE: SUSPENDED > ZOOKEEPER STATE: ConnectedReadOnly > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > CURATOR STATE: SUSPENDED > CURATOR STATE: READ_ONLY > So it's flaky. Sometimes it doesn't switch back and forth, sometimes twice > only, sometimes a lot more. > Depending on the timeout on the client, it might take more time to appear. > I attached a sample code that would reproduce it in 20-30 seconds. > Note that the problem may ultimately be on the ZooKeeper side but at that > point I just don't know. -- This message was sent by Atlassian JIRA (v6.2#6252)
[GitHub] curator pull request: CURATOR-126: Fix race condition in CuratorFr...
GitHub user dragonsinth opened a pull request: https://github.com/apache/curator/pull/23 CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() You can merge this pull request into a Git repository by running: $ git pull https://github.com/dragonsinth/curator CURATOR-126 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/23.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #23 commit 247e02152a96b58f344439d589d02065f1bd2632 Author: Scott Blum Date: 2014-07-28T18:10:37Z CURATOR-126: Fix race condition in CuratorFrameworkImpl.close() --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-125: Fix race condition in CuratorFr...
Github user dragonsinth closed the pull request at: https://github.com/apache/curator/pull/22 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-125: Fix race condition in CuratorFr...
Github user dragonsinth commented on the pull request: https://github.com/apache/curator/pull/22#issuecomment-50380674 oops wrong number --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] curator pull request: CURATOR-125: Fix race condition in CuratorFr...
GitHub user dragonsinth opened a pull request: https://github.com/apache/curator/pull/22 CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() You can merge this pull request into a Git repository by running: $ git pull https://github.com/dragonsinth/curator CURATOR-125 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/22.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22 commit e3b93f221b2a43c93d71a0f51510fc5f83b51968 Author: Scott Blum Date: 2014-07-28T18:10:37Z CURATOR-125: Fix race condition in CuratorFrameworkImpl.close() --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076457#comment-14076457
]
Scott Blum commented on CURATOR-126:
Also, versions of Java < 1.8 have a bug in FutureTask.cancel() where the *wrong
thread* will sometimes get interrupted (and that's the underlying thing that
happens when you call Executor.shutdownNow()).
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076460#comment-14076460
]
Jordan Zimmerman commented on CURATOR-126:
--
An Executor is just a thread as well. I'm not sure how changing to a Thread
would help.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076452#comment-14076452
]
Scott Blum commented on CURATOR-126:
Honestly, the executor is only used to start one thread. I would just replace
the Executor with a Thread, and join() it. If you guys like I can cook up a
patch.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (CURATOR-126) IllegalStateException in performBackgroundOperation during close
[
https://issues.apache.org/jira/browse/CURATOR-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076204#comment-14076204
]
Jordan Zimmerman commented on CURATOR-126:
--
I think the above change is a good idea, but it probably won't totally solve
the problem. executorService.shutdownNow() will just interrupt the threads and,
so, the client will still get closed while a background thread is executing.
We could consider calling executorService.awaitTermination() but that would be
a significant change.
> IllegalStateException in performBackgroundOperation during close
>
>
> Key: CURATOR-126
> URL: https://issues.apache.org/jira/browse/CURATOR-126
> Project: Apache Curator
> Issue Type: Bug
> Components: Framework
>Affects Versions: 2.5.0
>Reporter: Scott Blum
>Assignee: Cameron McKenzie
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> {code}
> [CuratorFramework-0] ERROR
> org.apache.curator.framework.imps.CuratorFrameworkImpl - Background
> exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:176)
> at
> org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:807)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.backgroundOperationsLoop(CuratorFrameworkImpl.java:793)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.access$400(CuratorFrameworkImpl.java:57)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl$4.call(CuratorFrameworkImpl.java:275)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:744)
> {code}
> I see this sometimes during test runs; I believe this happens because
> CuratorZookeeperClient.started gets set to false during shutdown, but the
> backgroundOperation loop can still be running since shutting down the
> backgroundOperation loop is inherently racy.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
