You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/05/25 10:21:41 UTC
[GitHub] [cassandra] bereng opened a new pull request #1021: Cassandra 16685 4.0
bereng opened a new pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#issuecomment-858293464
Review comments addressed and I raised the timeout to 250 just to be on the safe side :shrug:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#discussion_r648841693
##########
File path: test/unit/org/apache/cassandra/service/ActiveRepairServiceTest.java
##########
@@ -387,10 +407,13 @@ public void testRejectWhenPoolFullStrategy() throws InterruptedException
{
// expected
}
+
// allow executing tests to complete
blocked.signalAll();
- completed.await(10, TimeUnit.SECONDS);
+ completed.await(11, TimeUnit.SECONDS);
Review comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng closed pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng closed pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a change in pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#discussion_r648047909
##########
File path: test/unit/org/apache/cassandra/service/ActiveRepairServiceTest.java
##########
@@ -387,10 +407,13 @@ public void testRejectWhenPoolFullStrategy() throws InterruptedException
{
// expected
}
+
// allow executing tests to complete
blocked.signalAll();
- completed.await(10, TimeUnit.SECONDS);
+ completed.await(11, TimeUnit.SECONDS);
Review comment:
nit: could we get a comment here please? or even better make `10` a well-named constant so we understand that's the wait happening inside the task.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#discussion_r648428310
##########
File path: test/unit/org/apache/cassandra/service/ActiveRepairServiceTest.java
##########
@@ -376,8 +384,20 @@ public void testRejectWhenPoolFullStrategy() throws InterruptedException
{
Condition blocked = new SimpleCondition();
CountDownLatch completed = new CountDownLatch(2);
+
+ /*
+ * CASSANDRA-16685 When the underlying executor's queue is a SynchronousQueue, there can be races just after
+ * the ThreadPool's initialization while juggling and spinning up threads internally leading to false
+ * rejections. That queue needs a thread ready to pick up the task immediately or it will produce a reject
+ * exception upon 'offer()' method call on the executor's code. If the executor is still initializing or
+ * threads are not ready to take work you can get false rejections.
+ *
+ * A sleep has been added to give time to the thread pool to be ready to get work.
+ */
Review comment:
If this is a Java bug maybe we could explicitly mention it here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#issuecomment-851960884
- [J11](https://app.circleci.com/pipelines/github/bereng/cassandra/326/workflows/33f50b70-2ce7-400c-9b06-e1840a48450d/jobs/3115) repeated
- [J8](https://app.circleci.com/pipelines/github/bereng/cassandra/326/workflows/3f8816a9-0e74-4439-92b8-d8ef81bf2a97/jobs/3113) & [J8/11](https://app.circleci.com/pipelines/github/bereng/cassandra/326/workflows/3f8816a9-0e74-4439-92b8-d8ef81bf2a97/jobs/3114) repeated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#discussion_r648842971
##########
File path: test/unit/org/apache/cassandra/service/ActiveRepairServiceTest.java
##########
@@ -387,10 +407,13 @@ public void testRejectWhenPoolFullStrategy() throws InterruptedException
{
// expected
}
+
// allow executing tests to complete
blocked.signalAll();
- completed.await(10, TimeUnit.SECONDS);
+ completed.await(11, TimeUnit.SECONDS);
+
// Submission is unblocked
+ Thread.sleep(100);
Review comment:
Again the same error is produced if we don't wait. My guess being between task completion and TP readiness there is a small window the queue/TP interaction is broken.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#issuecomment-859317181
Squashed & rebased
CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/339/workflows/e2d8bff5-937a-4b1f-92ce-2f9e9f9a9554)
CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/339/workflows/2ae5710a-4318-45f7-99c9-60feb5cd2284)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #1021: Cassandra 16685 4.0
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #1021:
URL: https://github.com/apache/cassandra/pull/1021#discussion_r648439842
##########
File path: test/unit/org/apache/cassandra/service/ActiveRepairServiceTest.java
##########
@@ -387,10 +407,13 @@ public void testRejectWhenPoolFullStrategy() throws InterruptedException
{
// expected
}
+
// allow executing tests to complete
blocked.signalAll();
- completed.await(10, TimeUnit.SECONDS);
+ completed.await(11, TimeUnit.SECONDS);
+
// Submission is unblocked
+ Thread.sleep(100);
Review comment:
Why do we need this second sleep?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org