You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/08/01 18:56:21 UTC
[GitHub] [solr] madrob opened a new pull request, #959: TestContainerPlugin race condition
madrob opened a new pull request, #959:
URL: https://github.com/apache/solr/pull/959
Seen on a local Jenkins, but not reproducible on my machine. I think this fixes it, but would like some extra eyes.
```
org.junit.ComparisonFailure: Failed on path [result, packages, mypkg[0], files[0]] of {
"responseHeader":{
"status":0,
"QTime":0},
"result":{
"znodeVersion":-1,
"packages":{}}}after attempt #1 expected:<[/myplugin/v1.jar]> but was:<[]>
at __randomizedtesting.SeedInfo.seed([93A5391677407B93:7EFBC167450AAC1A]:0)
at org.junit.Assert.assertEquals(Assert.java:117)
at org.apache.solr.filestore.TestDistribPackageStore.assertResponseValues(TestDistribPackageStore.java:296)
at org.apache.solr.handler.TestContainerPlugin.waitForAllNodesToSync(TestContainerPlugin.java:455)
at org.apache.solr.handler.TestContainerPlugin.testApiFromPackage(TestContainerPlugin.java:255)
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #959: TestContainerPlugin race condition
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #959:
URL: https://github.com/apache/solr/pull/959#discussion_r953997265
##########
solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java:
##########
@@ -66,18 +71,71 @@
public class TestContainerPlugin extends SolrCloudTestCase {
private Phaser phaser;
+ private CountingListener listener;
+
private boolean forceV2;
+ /**
+ * A package listener that will count how many times it has been triggered. Useful to wait for
+ * changes accross multiple cores.
+ *
+ * <p>Use by calling {@link #reset()} before the API calls, and then {@link #waitFor(int)} to
+ * block until <code>num</code> cores have been notified.
+ */
+ class CountingListener implements PackageListeners.Listener {
+ private Semaphore changeCalled = new Semaphore(0);
Review Comment:
minor: could be `final`
##########
solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java:
##########
@@ -66,18 +71,71 @@
public class TestContainerPlugin extends SolrCloudTestCase {
private Phaser phaser;
+ private CountingListener listener;
+
private boolean forceV2;
+ /**
+ * A package listener that will count how many times it has been triggered. Useful to wait for
+ * changes accross multiple cores.
+ *
+ * <p>Use by calling {@link #reset()} before the API calls, and then {@link #waitFor(int)} to
+ * block until <code>num</code> cores have been notified.
+ */
+ class CountingListener implements PackageListeners.Listener {
Review Comment:
minor: could be `private static final`
##########
solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java:
##########
@@ -66,18 +71,71 @@
public class TestContainerPlugin extends SolrCloudTestCase {
private Phaser phaser;
+ private CountingListener listener;
+
private boolean forceV2;
+ /**
+ * A package listener that will count how many times it has been triggered. Useful to wait for
+ * changes accross multiple cores.
+ *
+ * <p>Use by calling {@link #reset()} before the API calls, and then {@link #waitFor(int)} to
+ * block until <code>num</code> cores have been notified.
+ */
+ class CountingListener implements PackageListeners.Listener {
+ private Semaphore changeCalled = new Semaphore(0);
+
+ @Override
+ public String packageName() {
+ return null; // will fire on all package changes
+ }
+
+ @Override
+ public Map<String, PackageAPI.PkgVersion> packageDetails() {
+ return null; // only used to print meta information
+ }
+
+ @Override
+ public void changed(PackageLoader.Package pkg, Ctx ctx) {
+ changeCalled.release();
+ }
+
+ public void reset() {
+ changeCalled.drainPermits();
+ }
+
+ public boolean waitFor(int num) throws InterruptedException {
+ return changeCalled.tryAcquire(num, 10, TimeUnit.SECONDS);
Review Comment:
could potentially `&& changeCalled.availablePermits() == 0` i.e. there should be no additional excess permits available after `num` have been acquired.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob merged pull request #959: TestContainerPlugin race condition
Posted by GitBox <gi...@apache.org>.
madrob merged PR #959:
URL: https://github.com/apache/solr/pull/959
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org