You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "ehsavoie (via GitHub)" <gi...@apache.org> on 2023/09/22 11:29:28 UTC
[GitHub] [activemq-artemis] ehsavoie opened a new pull request, #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
ehsavoie opened a new pull request, #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628
When we know that a node leaves a clustercleanly we shouldn't log WARN messages about it.
Issue: https://issues.apache.org/jira/browse/ARTEMIS-4436
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "ehsavoie (via GitHub)" <gi...@apache.org>.
ehsavoie commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1734105327
@gemmellr Yes that's correct. Moving the message to info level is another solution. Currently people removing a node from a cluster and seeing warning is also a problem. They are doing something completely fine so WARN is a bit too high
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1337525083
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java:
##########
@@ -1593,4 +1593,10 @@ void noQueueIdDefined(org.apache.activemq.artemis.api.core.Message message,
@LogMessage(id = 224130, value = "The {} value {} will override the {} value {} since both are set.", level = LogMessage.Level.INFO)
void configParamOverride(String overridingParam, Object overridingParamValue, String overriddenParam, Object overriddenParamValue);
+
+ @LogMessage(id = 224131, value = "Connection closed with failedOver={}", level = LogMessage.Level.INFO)
+ void bridgeConnectionClosed(Boolean failedOver);
+
+ @LogMessage(id = 224132, value = "There is a possible split brain on nodeID {} closing. Topology update ignored", level = LogMessage.Level.INFO)
+ void possibleSplitBrainAfterClosing(String nodeID);
Review Comment:
I didn't even see this message amongst the original changes. A "possible split brain" situation seems like its always a warning to me?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -554,9 +554,13 @@ public boolean updateMember(long uniqueEventID, String nodeId, TopologyMemberImp
* @return
*/
@Override
- public boolean removeMember(final long uniqueEventID, final String nodeId) {
+ public boolean removeMember(final long uniqueEventID, final String nodeId, final boolean disconnect) {
if (nodeId.equals(nodeManager.getNodeId().toString())) {
- ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
+ if ( disconnect) {
Review Comment:
Inconsistent spacing before/after (slightly surprised Checkstyle isnt configured to reject this given all the other things it barfs on hehe).
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/RemotingConnectionImpl.java:
##########
@@ -198,9 +199,12 @@ public void fail(final ActiveMQException me, String scaleDownTargetNodeID) {
destroyed = true;
}
- if (!(me instanceof ActiveMQRemoteDisconnectException) && !(me instanceof ActiveMQRoutingException)) {
+ if (!(me instanceof ActiveMQRemoteDisconnectException) && !(me instanceof ActiveMQRoutingException) && !(me instanceof ActiveMQDisconnectedException)) {
ActiveMQClientLogger.LOGGER.connectionFailureDetected(transportConnection.getRemoteAddress(), me.getMessage(), me.getType());
}
+ if (me instanceof ActiveMQDisconnectedException) {
Review Comment:
Feels like else-if would be more appropriate here since this is an either-or situation, emit one message or the other depending on situation.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "ehsavoie (via GitHub)" <gi...@apache.org>.
ehsavoie commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1339837647
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -554,10 +554,12 @@ public boolean updateMember(long uniqueEventID, String nodeId, TopologyMemberImp
* @return
*/
@Override
- public boolean removeMember(final long uniqueEventID, final String nodeId) {
+ public boolean removeMember(final long uniqueEventID, final String nodeId, final boolean disconnect) {
if (nodeId.equals(nodeManager.getNodeId().toString())) {
- ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
- return false;
+ if (!disconnect) {
+ ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
+ return false;
+ }
Review Comment:
yes maybe I should leave it to return false.
Leaving it to WARN will mean having a warning when a node leaves which seems strange to me
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1334344933
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/warnings/ClusterCleanNodeShutdownTest.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.cluster.warnings;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.postoffice.Binding;
+import org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.logs.AssertionLoggerHandler;
+import org.apache.activemq.artemis.tests.integration.cluster.distribution.ClusterTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ClusterCleanNodeShutdownTest extends ClusterTestBase {
+
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Test
+ public void testNoWarningErrorsDuringRestartingNodesInCluster() throws Exception {
+ setupServer(0, isFileStorage(), isNetty());
+ setupServer(1, isFileStorage(), isNetty());
+
+ setupClusterConnection("cluster0", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 0, 1);
+
+ setupClusterConnection("cluster1", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 1, 0);
+
+ startServers(0, 1);
+ Thread.sleep(2000);
+
+ logger.debug("server 0 = {}", getServer(0).getNodeID());
+ logger.debug("server 1 = {}", getServer(1).getNodeID());
+
+ setupSessionFactory(0, isNetty(), 15);
+ setupSessionFactory(1, isNetty());
+
+ // now create the 2 queues and make sure they are durable
+ createQueue(0, "queues.testaddress", "queue10", null, true);
+ createQueue(1, "queues.testaddress", "queue10", null, true);
+
+ addConsumer(0, 0, "queue10", null);
+
+ waitForBindings(0, "queues.testaddress", 1, 1, true);
+ waitForBindings(1, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, false);
+ waitForBindings(1, "queues.testaddress", 1, 1, false);
+
+ printBindings(2);
+
+ sendInRange(1, "queues.testaddress", 0, 10, true, null);
+
+ try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) {
+ logger.info("*****************************************************************************");
+ stopServers(0);
+ // Waiting some time after stopped
+ Thread.sleep(2000);
+ logger.info("*****************************************************************************");
+ Assert.assertFalse("Connection failure detected for an expected DISCONNECT event", loggerHandler.findText("AMQ212037", " [code=DISCONNECTED]"));
+ Assert.assertFalse("WARN found",loggerHandler.hasLevel(AssertionLoggerHandler.LogLevel.WARN));
+ }
+ startServers(0);
+
+ waitForBindings(0, "queues.testaddress", 1, 1, true);
+ waitForBindings(1, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, false);
+ waitForBindings(1, "queues.testaddress", 1, 1, false);
+
+ printBindings(2);
Review Comment:
do you always need the printBindings? Can't you add it only when something "bad" happened?
or at the worst case... if (logger.isDebugEnabled()) ?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster. [activemq-artemis]
Posted by "ehsavoie (via GitHub)" <gi...@apache.org>.
ehsavoie commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1876865582
@gemmellr @clebertsuconic @jbertram could you please take a look now that I've updated the PR
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster. [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1878919442
This looks good to me. I'll wait for others to review before merging.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1334344310
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/warnings/ClusterCleanNodeShutdownTest.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.cluster.warnings;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.postoffice.Binding;
+import org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.logs.AssertionLoggerHandler;
+import org.apache.activemq.artemis.tests.integration.cluster.distribution.ClusterTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ClusterCleanNodeShutdownTest extends ClusterTestBase {
+
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Test
+ public void testNoWarningErrorsDuringRestartingNodesInCluster() throws Exception {
+ setupServer(0, isFileStorage(), isNetty());
+ setupServer(1, isFileStorage(), isNetty());
+
+ setupClusterConnection("cluster0", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 0, 1);
+
+ setupClusterConnection("cluster1", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 1, 0);
+
+ startServers(0, 1);
+ Thread.sleep(2000);
+
+ logger.debug("server 0 = {}", getServer(0).getNodeID());
+ logger.debug("server 1 = {}", getServer(1).getNodeID());
+
+ setupSessionFactory(0, isNetty(), 15);
+ setupSessionFactory(1, isNetty());
+
+ // now create the 2 queues and make sure they are durable
+ createQueue(0, "queues.testaddress", "queue10", null, true);
+ createQueue(1, "queues.testaddress", "queue10", null, true);
+
+ addConsumer(0, 0, "queue10", null);
+
+ waitForBindings(0, "queues.testaddress", 1, 1, true);
+ waitForBindings(1, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, false);
+ waitForBindings(1, "queues.testaddress", 1, 1, false);
+
+ printBindings(2);
+
+ sendInRange(1, "queues.testaddress", 0, 10, true, null);
+
+ try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) {
+ logger.info("*****************************************************************************");
+ stopServers(0);
+ // Waiting some time after stopped
+ Thread.sleep(2000);
Review Comment:
Can. you use a Wait Clause 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1734090863
This looks like it aims to suppress a [warning] notice that the other broker has gone away, is that correct? Its not clear to me that entirely makes sense. I could certainly see reducing the notice to Info level, but it still seems like one should be given? I don't necessarily care that the other broker was shut down cleanly, often just that it isn't there any more and was expected to be.
(I've also seen several times where people 'cleanly' shut down things without realising they are doing it, then come asking why it has 'crashed' or 'shut down by itself')
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1334773782
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/warnings/ClusterCleanNodeShutdownTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cluster.warnings;
+
+import java.lang.invoke.MethodHandles;
+import org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.logs.AssertionLoggerHandler;
+import org.apache.activemq.artemis.tests.integration.cluster.distribution.ClusterTestBase;
+import org.apache.activemq.artemis.utils.Wait;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ClusterCleanNodeShutdownTest extends ClusterTestBase {
+
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Test
+ public void testNoWarningErrorsDuringRestartingNodesInCluster() throws Exception {
+ setupServer(0, isFileStorage(), isNetty());
+ setupServer(1, isFileStorage(), isNetty());
+
+ setupClusterConnection("cluster0", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 0, 1);
+ setupClusterConnection("cluster1", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 1, 0);
+
+ startServers(0, 1);
+ Wait.assertTrue(() -> {
+ getServer(0).getClusterManager().getClusterController().awaitConnectionToReplicationCluster();
+ return true;
+ }, 2000L);
+ Wait.assertTrue(() -> {
+ getServer(1).getClusterManager().getClusterController().awaitConnectionToReplicationCluster();
+ return true;
+ }, 2000L);
+
+ logger.debug("server 0 = {}", getServer(0).getNodeID());
+ logger.debug("server 1 = {}", getServer(1).getNodeID());
+
+ setupSessionFactory(0, isNetty(), 15);
+ setupSessionFactory(1, isNetty());
+
+ // now create the 2 queues and make sure they are durable
+ createQueue(0, "queues.testaddress", "queue10", null, true);
+ createQueue(1, "queues.testaddress", "queue10", null, true);
+
+ addConsumer(0, 0, "queue10", null);
+
+ waitForBindings(0, "queues.testaddress", 1, 1, true);
+ waitForBindings(1, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, false);
+ waitForBindings(1, "queues.testaddress", 1, 1, false);
+
+ sendInRange(1, "queues.testaddress", 0, 10, true, null);
+ try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) {
+ logger.info("*****************************************************************************");
Review Comment:
logger.debug();
for these two statements...
Everything else looks good... I run the whole test suite and it was good
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1334774030
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/warnings/ClusterCleanNodeShutdownTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cluster.warnings;
+
+import java.lang.invoke.MethodHandles;
+import org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.logs.AssertionLoggerHandler;
+import org.apache.activemq.artemis.tests.integration.cluster.distribution.ClusterTestBase;
+import org.apache.activemq.artemis.utils.Wait;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ClusterCleanNodeShutdownTest extends ClusterTestBase {
+
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Test
+ public void testNoWarningErrorsDuringRestartingNodesInCluster() throws Exception {
+ setupServer(0, isFileStorage(), isNetty());
+ setupServer(1, isFileStorage(), isNetty());
+
+ setupClusterConnection("cluster0", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 0, 1);
+ setupClusterConnection("cluster1", "queues", MessageLoadBalancingType.ON_DEMAND, 1, isNetty(), 1, 0);
+
+ startServers(0, 1);
+ Wait.assertTrue(() -> {
+ getServer(0).getClusterManager().getClusterController().awaitConnectionToReplicationCluster();
+ return true;
+ }, 2000L);
+ Wait.assertTrue(() -> {
+ getServer(1).getClusterManager().getClusterController().awaitConnectionToReplicationCluster();
+ return true;
+ }, 2000L);
+
+ logger.debug("server 0 = {}", getServer(0).getNodeID());
+ logger.debug("server 1 = {}", getServer(1).getNodeID());
+
+ setupSessionFactory(0, isNetty(), 15);
+ setupSessionFactory(1, isNetty());
+
+ // now create the 2 queues and make sure they are durable
+ createQueue(0, "queues.testaddress", "queue10", null, true);
+ createQueue(1, "queues.testaddress", "queue10", null, true);
+
+ addConsumer(0, 0, "queue10", null);
+
+ waitForBindings(0, "queues.testaddress", 1, 1, true);
+ waitForBindings(1, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, false);
+ waitForBindings(1, "queues.testaddress", 1, 1, false);
+
+ sendInRange(1, "queues.testaddress", 0, 10, true, null);
+ try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) {
+ logger.info("*****************************************************************************");
Review Comment:
I may amend myself and merge it today (later...)
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1735090296
Yes, per my comment, I dont think it makes sense to remove notice that clients/brokers-acting-as-clients lost connection just because the other side was shut down 'cleanly'. This side still expected to be connected and no longer is, thats detail they'll often want/need to know, regardless of whether the other side went away intentionally (which, per my comment, I've seen people do unwittingly on several occasions).
I agree a warning level message is excessive, but suppressing notice entirely seems the wrong fix to that...I'd still report it happened, but at Info level.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1734343261
@gemmellr do you have any concerns on this PR? If not I think it should be merged.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster. [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "ehsavoie (via GitHub)" <gi...@apache.org>.
ehsavoie commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1338336447
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java:
##########
@@ -1593,4 +1593,10 @@ void noQueueIdDefined(org.apache.activemq.artemis.api.core.Message message,
@LogMessage(id = 224130, value = "The {} value {} will override the {} value {} since both are set.", level = LogMessage.Level.INFO)
void configParamOverride(String overridingParam, Object overridingParamValue, String overriddenParam, Object overriddenParamValue);
+
+ @LogMessage(id = 224131, value = "Connection closed with failedOver={}", level = LogMessage.Level.INFO)
+ void bridgeConnectionClosed(Boolean failedOver);
+
+ @LogMessage(id = 224132, value = "There is a possible split brain on nodeID {} closing. Topology update ignored", level = LogMessage.Level.INFO)
+ void possibleSplitBrainAfterClosing(String nodeID);
Review Comment:
Correct, this should go
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1339785211
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -554,10 +554,12 @@ public boolean updateMember(long uniqueEventID, String nodeId, TopologyMemberImp
* @return
*/
@Override
- public boolean removeMember(final long uniqueEventID, final String nodeId) {
+ public boolean removeMember(final long uniqueEventID, final String nodeId, final boolean disconnect) {
if (nodeId.equals(nodeManager.getNodeId().toString())) {
- ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
- return false;
+ if (!disconnect) {
+ ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
+ return false;
+ }
Review Comment:
With my other comment about the alternative 'info message if disconnect' about split-brain possibility you had added previously here, I was meaning more it seemed preferable to leave it always logging the warning, whereas now this is back to not saying anything at all in this case.
Unless we know for sure there isnt such possibility because it was a shutdown, still feels it should be saying something? Its not clear to me if there is no possibilty of it, I dont know enough about the clustering to say either way.
This also seems to change it from 'rejecting a topology update' to accepting it in this case, which seems a larger change than just the logging, and which I'm similarly not clear on the effect of. Need others with more knowledge here...@clebertsuconic @jbertram ?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4628: ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster.
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#discussion_r1339785211
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -554,10 +554,12 @@ public boolean updateMember(long uniqueEventID, String nodeId, TopologyMemberImp
* @return
*/
@Override
- public boolean removeMember(final long uniqueEventID, final String nodeId) {
+ public boolean removeMember(final long uniqueEventID, final String nodeId, final boolean disconnect) {
if (nodeId.equals(nodeManager.getNodeId().toString())) {
- ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
- return false;
+ if (!disconnect) {
+ ActiveMQServerLogger.LOGGER.possibleSplitBrain(nodeId);
+ return false;
+ }
Review Comment:
With my other comment about the 'info message if disconnect' you added previously, I was meaning more to leave it always logging the warning, wheres now this is back to not saying anything.
Unless we know for sure there isnt such possibility because it was a shutdown? Its not clear to me, I dont know enough about the clustering to say either way.
This also seems to change it from 'rejecting a topology update' to accepting it in this case, which seems a larger change than just the logging, and which I'm similarly not clear on the effect of. Need others with more knowledge here...@clebertsuconic @jbertram ?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4436 Artemis is logging warnings during clean shutdown of server in cluster. [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4628:
URL: https://github.com/apache/activemq-artemis/pull/4628#issuecomment-1880939190
My last feedback was that I didnt know enough to say whether it was ok or not, and to ask for others to finish reviewing, so at this point I'll defer to those that do.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org