diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 0dfb90b2efd..378b4a8b439 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -142,7 +142,8 @@ Other Changes * SOLR-12152: Split up TriggerIntegrationTest into multiple tests to isolate and increase reliability. (shalin) -* SOLR-12133: Fix race conditions that caused TriggerIntegrationTest.testEventQueue to fail. (Mark Miller, shalin) +* SOLR-12133: Fix race conditions that caused TriggerIntegrationTest.testEventQueue and testNodeMarkersRegistration + to fail. (Mark Miller, shalin) * SOLR-12169: Fix ComputePlanActionTest.testSelectedCollections fails on jenkins by aggressively cleaning up trigger state left by other test methods in the test setup. (shalin) diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java index 6190a499175..7f389e1cfd1 100644 --- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java +++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java @@ -184,6 +184,7 @@ public class NodeAddedTrigger extends TriggerBase { private void removeMarker(String nodeName) { String path = ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH + "/" + nodeName; try { + log.debug("NodeAddedTrigger {} - removing marker path: {}", name, path); if (stateManager.hasData(path)) { stateManager.removeData(path, -1); } diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/NodeMarkersRegistrationTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/NodeMarkersRegistrationTest.java index 38c2165725d..912f71f14c4 100644 --- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/NodeMarkersRegistrationTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/NodeMarkersRegistrationTest.java @@ -31,6 +31,7 @@ import java.util.concurrent.locks.ReentrantLock; import org.apache.lucene.util.LuceneTestCase; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.cloud.autoscaling.TriggerEventProcessorStage; import org.apache.solr.client.solrj.cloud.autoscaling.TriggerEventType; import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.client.solrj.impl.CloudSolrClient; @@ -52,9 +53,8 @@ import static org.apache.solr.cloud.autoscaling.AutoScalingHandlerTest.createAut public class NodeMarkersRegistrationTest extends SolrCloudTestCase { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private static CountDownLatch actionInitCalled; private static CountDownLatch triggerFiredLatch; - private static CountDownLatch actionConstructorCalled; + private static CountDownLatch listenerEventLatch; private static Set events = ConcurrentHashMap.newKeySet(); private static ZkStateReader zkStateReader; private static ReentrantLock lock = new ReentrantLock(); @@ -81,11 +81,8 @@ public class NodeMarkersRegistrationTest extends SolrCloudTestCase { @Test public void testNodeMarkersRegistration() throws Exception { - // for this test we want to create two triggers so we must assert that the actions were created twice - actionInitCalled = new CountDownLatch(2); - // similarly we want both triggers to fire - triggerFiredLatch = new CountDownLatch(2); - actionConstructorCalled = new CountDownLatch(1); + triggerFiredLatch = new CountDownLatch(1); + listenerEventLatch = new CountDownLatch(1); TestLiveNodesListener listener = registerLiveNodesListener(); NamedList overSeerStatus = cluster.getSolrClient().request(CollectionAdminRequest.getOverseerStatus()); @@ -155,6 +152,18 @@ public class NodeMarkersRegistrationTest extends SolrCloudTestCase { response = solrClient.request(req); assertEquals(response.get("result").toString(), "success"); + String setListener = "{\n" + + " \"set-listener\" : {\n" + + " \"name\" : \"listener_node_added_triggerMR\",\n" + + " \"trigger\" : \"node_added_triggerMR\",\n" + + " \"stage\" : \"STARTED\",\n" + + " \"class\" : \"" + AssertingListener.class.getName() + "\"\n" + + " }\n" + + "}"; + req = createAutoScalingRequest(SolrRequest.METHOD.POST, setListener); + response = solrClient.request(req); + assertEquals(response.get("result").toString(), "success"); + overSeerStatus = cluster.getSolrClient().request(CollectionAdminRequest.getOverseerStatus()); overseerLeader = (String) overSeerStatus.get("leader"); overseerLeaderIndex = 0; @@ -178,6 +187,9 @@ public class NodeMarkersRegistrationTest extends SolrCloudTestCase { pathAdded = ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH + "/" + node1.getNodeName(); assertTrue("Path " + pathAdded + " wasn't created", zkClient().exists(pathAdded, true)); + listenerEventLatch.countDown(); // let the trigger thread continue + + assertTrue(triggerFiredLatch.await(10, TimeUnit.SECONDS)); Thread.sleep(5000); // nodeAdded marker should be consumed now by nodeAdded trigger assertFalse("Path " + pathAdded + " should have been deleted", zkClient().exists(pathAdded, true)); @@ -237,10 +249,6 @@ public class NodeMarkersRegistrationTest extends SolrCloudTestCase { public static class TestEventMarkerAction extends TriggerActionBase { - public TestEventMarkerAction() { - actionConstructorCalled.countDown(); - } - @Override public void process(TriggerEvent event, ActionContext actionContext) { boolean locked = lock.tryLock(); @@ -262,8 +270,20 @@ public class NodeMarkersRegistrationTest extends SolrCloudTestCase { @Override public void init(Map args) { log.info("TestEventMarkerAction init"); - actionInitCalled.countDown(); super.init(args); } } + + public static class AssertingListener extends TriggerListenerBase { + @Override + public void onEvent(TriggerEvent event, TriggerEventProcessorStage stage, String actionName, ActionContext context, Throwable error, String message) throws Exception { + if (!Thread.currentThread().getName().startsWith("ScheduledTrigger")) { + // for future safety + throw new IllegalThreadStateException("AssertingListener should have been invoked by a thread from the scheduled trigger thread pool"); + } + log.debug(" --- listener fired for event: {}, stage: {}", event, stage); + listenerEventLatch.await(); + log.debug(" --- listener wait complete for event: {}, stage: {}", event, stage); + } + } }