gemmellr commented on code in PR #4485: URL: https://github.com/apache/activemq-artemis/pull/4485#discussion_r1201934685
########## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/federation/FederatedQueueConsumerTest.java: ########## @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.tests.integration.federation; + +import org.apache.activemq.artemis.core.config.FederationConfiguration; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.federation.FederatedQueueConsumerImpl; +import org.apache.activemq.artemis.core.server.federation.Federation; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.activemq.artemis.tests.util.RandomUtil; +import org.apache.activemq.artemis.utils.Wait; +import org.junit.Test; + +public class FederatedQueueConsumerTest extends ActiveMQTestBase { + + @Test + public void testClose() throws Exception { + ActiveMQServer server = createServer(false, createDefaultInVMConfig()); + server.start(); + Federation federation = new Federation(server, new FederationConfiguration().setName(RandomUtil.randomString())); + federation.start(); + FederatedQueueConsumerImpl consumer = new FederatedQueueConsumerImpl(federation, server, null, null, null, null); + consumer.start(); + Wait.waitFor(() -> consumer.getConnectionAttemptTimestamp() > 0); + consumer.close(); + long closed = System.currentTimeMillis(); + assertFalse(Wait.waitFor(() -> consumer.getConnectionAttemptTimestamp() > closed, 5000, 100)); Review Comment: My basic suggestion yesterday was just inc/dec a counter, e.g instead of setting/resetting a timestamp as you did, keep a simple count of outstanding connect tasks. If there are none outstanding there will be no connect without calling start. Since the initial scheduling is all synchronized now, and the connect task reschedules itself on failure, its either going to be 0 (nothing scheduled, no connect will happen), or 1 (connect scheduled, or task still actually in progress), or 2 (a task in progress and failing, plus 1 retry it has just scheduled). You could then just wait for it to be 0 again. A better alternative would probably be keeping the currently-thrown-away ScheduledFuture for the latest scheduled connect task. You could then tell whether there is another attempt scheduled. You could also use it in close() to cancel any future connect attempt that hasnt been started yet, rather than just relying on it to no-op potentially much later. This could also make the test even faster than the ~1sec typical of the first retry period, since then it wouldnt need to wait for the retry to happen, it could then tell it wont happen. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
