You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "clebertsuconic (via GitHub)" <gi...@apache.org> on 2023/02/18 12:49:52 UTC

[GitHub] [activemq-artemis] clebertsuconic opened a new pull request, #4375: ARTEMIS-4175 JournalFileImpl Leaking

clebertsuconic opened a new pull request, #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375

   I am now removing the negatives once the file is removed or reused


-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1119635292


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
##########
@@ -418,6 +418,17 @@ public void cancel(Object brokerConsumer, Message message, boolean updateCounts)
       }
    }
 
+   /** to be used when credits were drained between handle and actual delivery */
+   public void returnToQueue(Object brokerConsumer, MessageReference reference) {

Review Comment:
   The issue here is really about a memory leak. 
   
   I had credits when handleDelivery was called. later on I called deliver, the AMQPStandardMessage would be referenced on a DeliveryImpl that would stay forever in the Link.workingList until the session or the connection is closed.
   
   
   I had other issues that I fixed as part of this PR regarding QueueConsumer. where the iterator would hung in the Iterators. Perhaps that one would cause some issues.
   
   https://github.com/apache/activemq-artemis/pull/4375/commits/a1aea9a79aa844673ef705d2a60fadb4435accfc#diff-fc4a9a0dc01f79ecc1435246dc85446fa57d6d04e1dffed53c522d8c145623fdR3098-R3105



-- 
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 #4375: ARTEMIS-4175 JournalFileImpl Leaking

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1113794757


##########
tests/leak-tests/src/test/java/org/apache/activemq/artemis/tests/leak/ClientLeakTest.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.leak;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+
+import io.github.checkleak.core.CheckLeak;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.ActiveMQServers;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.tests.util.CFUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.qpid.proton.engine.impl.ReceiverImpl;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.activemq.artemis.tests.leak.MemoryAssertions.assertMemory;
+
+// This test spawns the server as a separate VM
+// as we need to count exclusively client objects from qpid-proton
+public class ClientLeakTest extends ActiveMQTestBase {
+
+   private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final String LEAK_SERVER = "LEAK-SERVER-STARTED";
+   Process serverProcess;
+
+   public static void main(String[] arg) {
+
+      try {
+         ConfigurationImpl configuration = new ConfigurationImpl().setSecurityEnabled(false).setJournalMinFiles(2).setJournalFileSize(100 * 1024).setJournalType(getDefaultJournalType()).setJournalDirectory("./data/journal").setBindingsDirectory("./data/binding").setPagingDirectory("./data/page").setLargeMessagesDirectory("./data/lm").setJournalCompactMinFiles(0).setJournalCompactPercentage(0).setClusterPassword(CLUSTER_PASSWORD).setJournalDatasync(false);
+         configuration.addAcceptorConfiguration(new TransportConfiguration(NETTY_ACCEPTOR_FACTORY, new HashMap<String, Object>(), "netty", new HashMap<String, Object>()));
+         ActiveMQServer server = ActiveMQServers.newActiveMQServer(configuration, false);
+         server.start();
+         System.out.println(LEAK_SERVER);
+      } catch (Throwable e) {
+         e.printStackTrace();
+         System.exit(-1);
+      }
+
+   }
+
+   @BeforeClass
+   public static void beforeClass() throws Exception {
+      Assume.assumeTrue(CheckLeak.isLoaded());
+   }
+
+   @Override
+   @Before
+   public void setUp() throws Exception {
+      serverProcess = SpawnedVMSupport.spawnVM(ClientLeakTest.class.getName());
+      runAfter(serverProcess::destroyForcibly);
+
+      boolean success = false;
+      long time = System.currentTimeMillis() + 5_000;
+
+      // this loop will keep trying a connection until the serer has started
+      do {
+         try {
+            ConnectionFactory cf = CFUtil.createConnectionFactory("AMQP", "tcp://localhost:61616");
+            try (Connection connection = cf.createConnection()) {
+               success = true;
+            }
+         } catch (Throwable e) {
+            logger.debug(e.getMessage(), e);
+            Thread.sleep(100);
+         }
+
+      } while (success == false && System.currentTimeMillis() < time);
+      Assert.assertTrue(success);
+   }
+
+   @After
+   public void stopServer() throws Exception {
+      serverProcess.destroyForcibly();
+   }
+
+   @Test
+   public void testRepeatAMQPSessions() throws Exception {
+      CheckLeak checkLeak = new CheckLeak();
+
+      ConnectionFactory cf = CFUtil.createConnectionFactory("AMQP", "tcp://localhost:61616");
+      Connection connection = cf.createConnection();
+      for (int i = 0; i < 10; i++) {
+         for (int j = 0; j < 10; j++) {
+            Session session = connection.createSession(true, Session.SESSION_TRANSACTED);
+            MessageProducer producer = session.createProducer(session.createQueue("test"));
+            producer.send(session.createTextMessage("test"));
+            session.commit();
+            session.close();
+         }
+
+         for (int j = 0; j < 10; j++) {
+            Session session = connection.createSession(true, Session.SESSION_TRANSACTED);
+            MessageConsumer consumer = session.createConsumer(session.createQueue("test"));
+            connection.start();
+            Message message = consumer.receive(1000);
+            Assert.assertNotNull(message);
+            session.commit();
+            // consumer.close(); // uncomment this and the test will pass.

Review Comment:
   @gemmellr I am not sure if this has any relation to anything I'm seeing now... but if I remove this comment, the test will pass. closing the consumer will throw some log.warns on the server that I'm investigating and are expected to happen, but this will make the test to not keep any references on  any client Receivers.



-- 
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 #4375: ARTEMIS-4175 JournalFileImpl Leaking

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1113600106


##########
tests/leak-tests/src/test/java/org/apache/activemq/artemis/tests/leak/ClientLeakTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.leak;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+
+import io.github.checkleak.core.CheckLeak;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl;
+import org.apache.activemq.artemis.core.protocol.core.impl.RemotingConnectionImpl;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.ActiveMQServers;
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl;
+import org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl;
+import org.apache.activemq.artemis.core.server.impl.ServerStatus;
+import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.tests.util.CFUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.qpid.proton.engine.impl.DeliveryImpl;
+import org.apache.qpid.proton.engine.impl.ReceiverImpl;
+import org.apache.qpid.proton.engine.impl.SenderImpl;
+import org.apache.qpid.proton.engine.impl.SessionImpl;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.activemq.artemis.tests.leak.MemoryAssertions.assertMemory;
+import static org.apache.activemq.artemis.tests.leak.MemoryAssertions.basicMemoryAsserts;
+
+// This test spawns the server as a separate VM
+// as we need to count exclusively client objects from qpid-proton
+public class ClientLeakTest extends ActiveMQTestBase {
+
+   private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final String LEAK_SERVER = "LEAK-SERVER-STARTED";
+
+   public static void main(String arg[]) {
+
+      try {
+         ConfigurationImpl configuration = new ConfigurationImpl().setSecurityEnabled(false).setJournalMinFiles(2).setJournalFileSize(100 * 1024).setJournalType(getDefaultJournalType()).setJournalDirectory("./data/journal").setBindingsDirectory("./data/binding").setPagingDirectory("./data/page").setLargeMessagesDirectory("./data/lm").setJournalCompactMinFiles(0).setJournalCompactPercentage(0).setClusterPassword(CLUSTER_PASSWORD).setJournalDatasync(false);
+         configuration.addAcceptorConfiguration(new TransportConfiguration(NETTY_ACCEPTOR_FACTORY, new HashMap<String, Object>(), "netty", new HashMap<String, Object>()));
+         ActiveMQServer server = ActiveMQServers.newActiveMQServer(configuration, false);
+         server.start();
+         System.out.println(LEAK_SERVER);
+      } catch (Throwable e) {
+         e.printStackTrace();
+         System.exit(-1);
+      }
+
+   }
+
+   @BeforeClass
+   public static void beforeClass() throws Exception {
+      Assume.assumeTrue(CheckLeak.isLoaded());
+   }
+
+   Process serverProcess;
+
+   @Override
+   @Before
+   public void setUp() throws Exception {
+      serverProcess = SpawnedVMSupport.spawnVM(ClientLeakTest.class.getName());
+      runAfter(serverProcess::destroyForcibly);
+
+      boolean success = false;
+      long time = System.currentTimeMillis() + 5_000;
+
+      // this loop will keep trying a connection until the serer has started
+      do {
+         try {
+            ConnectionFactory cf = CFUtil.createConnectionFactory("AMQP", "tcp://localhost:61616");
+            try (Connection connection = cf.createConnection()) {
+               success = true;
+            }
+         } catch (Throwable e) {
+            logger.debug(e.getMessage(), e);
+            Thread.sleep(100);
+         }
+
+      } while (success == false && System.currentTimeMillis() < time);
+      Assert.assertTrue(success);
+   }
+
+   @After
+   public void stopServer() throws Exception {
+      serverProcess.destroyForcibly();
+   }
+
+   @Test
+   public void testAMQP() throws Exception {
+      final String protocol = "AMQP";
+      CheckLeak checkLeak = new CheckLeak();
+      int REPEATS = 100;
+      int MESSAGES = 20;
+      basicMemoryAsserts();
+
+      ConnectionFactory cf = CFUtil.createConnectionFactory(protocol, "tcp://localhost:61616");
+
+      try (Connection producerConnection = cf.createConnection(); Connection consumerConnection = cf.createConnection()) {
+
+         Session producerSession = producerConnection.createSession(true, Session.SESSION_TRANSACTED);
+
+         Session consumerSession = consumerConnection.createSession(true, Session.SESSION_TRANSACTED);
+         consumerConnection.start();
+         assertMemory(checkLeak, 0, ReceiverImpl.class.getName());
+
+         for (int i = 0; i < REPEATS; i++) {
+            System.out.println("********************************************************************************");
+            {
+               Destination source = producerSession.createQueue("source");
+               try (MessageProducer sourceProducer = producerSession.createProducer(source)) {
+                  for (int msg = 0; msg < MESSAGES; msg++) {
+                     Message message = producerSession.createTextMessage("hello " + msg);
+                     message.setIntProperty("i", msg);
+                     sourceProducer.send(message);
+                  }
+                  sourceProducer.close();
+                  producerSession.commit();
+               }
+            }
+
+            // it shouldn't have any sender connects still. The retry loop is leaving a few behind.:w
+            assertMemory(checkLeak, 5, SenderImpl.class.getName());

Review Comment:
   @gemmellr SenderImpl keeps piling up on this test... Even though the producers and consumers are  closed... 
   
   I think this is similar to what I see on the server's.



-- 
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 #4375: ARTEMIS-4175 JournalFileImpl Leaking

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1113604153


##########
tests/leak-tests/src/test/java/org/apache/activemq/artemis/tests/leak/ClientLeakTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.leak;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+
+import io.github.checkleak.core.CheckLeak;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl;
+import org.apache.activemq.artemis.core.protocol.core.impl.RemotingConnectionImpl;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.ActiveMQServers;
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl;
+import org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl;
+import org.apache.activemq.artemis.core.server.impl.ServerStatus;
+import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.tests.util.CFUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.qpid.proton.engine.impl.DeliveryImpl;
+import org.apache.qpid.proton.engine.impl.ReceiverImpl;
+import org.apache.qpid.proton.engine.impl.SenderImpl;
+import org.apache.qpid.proton.engine.impl.SessionImpl;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.activemq.artemis.tests.leak.MemoryAssertions.assertMemory;
+import static org.apache.activemq.artemis.tests.leak.MemoryAssertions.basicMemoryAsserts;
+
+// This test spawns the server as a separate VM
+// as we need to count exclusively client objects from qpid-proton
+public class ClientLeakTest extends ActiveMQTestBase {
+
+   private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final String LEAK_SERVER = "LEAK-SERVER-STARTED";
+
+   public static void main(String arg[]) {
+
+      try {
+         ConfigurationImpl configuration = new ConfigurationImpl().setSecurityEnabled(false).setJournalMinFiles(2).setJournalFileSize(100 * 1024).setJournalType(getDefaultJournalType()).setJournalDirectory("./data/journal").setBindingsDirectory("./data/binding").setPagingDirectory("./data/page").setLargeMessagesDirectory("./data/lm").setJournalCompactMinFiles(0).setJournalCompactPercentage(0).setClusterPassword(CLUSTER_PASSWORD).setJournalDatasync(false);
+         configuration.addAcceptorConfiguration(new TransportConfiguration(NETTY_ACCEPTOR_FACTORY, new HashMap<String, Object>(), "netty", new HashMap<String, Object>()));
+         ActiveMQServer server = ActiveMQServers.newActiveMQServer(configuration, false);
+         server.start();
+         System.out.println(LEAK_SERVER);
+      } catch (Throwable e) {
+         e.printStackTrace();
+         System.exit(-1);
+      }
+
+   }
+
+   @BeforeClass
+   public static void beforeClass() throws Exception {
+      Assume.assumeTrue(CheckLeak.isLoaded());
+   }
+
+   Process serverProcess;
+
+   @Override
+   @Before
+   public void setUp() throws Exception {
+      serverProcess = SpawnedVMSupport.spawnVM(ClientLeakTest.class.getName());
+      runAfter(serverProcess::destroyForcibly);
+
+      boolean success = false;
+      long time = System.currentTimeMillis() + 5_000;
+
+      // this loop will keep trying a connection until the serer has started
+      do {
+         try {
+            ConnectionFactory cf = CFUtil.createConnectionFactory("AMQP", "tcp://localhost:61616");
+            try (Connection connection = cf.createConnection()) {
+               success = true;
+            }
+         } catch (Throwable e) {
+            logger.debug(e.getMessage(), e);
+            Thread.sleep(100);
+         }
+
+      } while (success == false && System.currentTimeMillis() < time);
+      Assert.assertTrue(success);
+   }
+
+   @After
+   public void stopServer() throws Exception {
+      serverProcess.destroyForcibly();
+   }
+
+   @Test
+   public void testAMQP() throws Exception {
+      final String protocol = "AMQP";
+      CheckLeak checkLeak = new CheckLeak();
+      int REPEATS = 100;
+      int MESSAGES = 20;
+      basicMemoryAsserts();
+
+      ConnectionFactory cf = CFUtil.createConnectionFactory(protocol, "tcp://localhost:61616");
+
+      try (Connection producerConnection = cf.createConnection(); Connection consumerConnection = cf.createConnection()) {
+
+         Session producerSession = producerConnection.createSession(true, Session.SESSION_TRANSACTED);
+
+         Session consumerSession = consumerConnection.createSession(true, Session.SESSION_TRANSACTED);
+         consumerConnection.start();
+         assertMemory(checkLeak, 0, ReceiverImpl.class.getName());
+
+         for (int i = 0; i < REPEATS; i++) {
+            System.out.println("********************************************************************************");
+            {
+               Destination source = producerSession.createQueue("source");
+               try (MessageProducer sourceProducer = producerSession.createProducer(source)) {
+                  for (int msg = 0; msg < MESSAGES; msg++) {
+                     Message message = producerSession.createTextMessage("hello " + msg);
+                     message.setIntProperty("i", msg);
+                     sourceProducer.send(message);
+                  }
+                  sourceProducer.close();
+                  producerSession.commit();
+               }
+            }
+
+            // it shouldn't have any sender connects still. The retry loop is leaving a few behind.:w
+            assertMemory(checkLeak, 5, SenderImpl.class.getName());

Review Comment:
   false alarm... I had a producer I forgot to close. duh



-- 
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 merged pull request #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375


-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#issuecomment-1450463551

   @gemmellr this PR will sit here until a proton-j release is done


-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1119640420


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
##########
@@ -418,6 +418,17 @@ public void cancel(Object brokerConsumer, Message message, boolean updateCounts)
       }
    }
 
+   /** to be used when credits were drained between handle and actual delivery */
+   public void returnToQueue(Object brokerConsumer, MessageReference reference) {

Review Comment:
   @michaelandrepearce those issues we had were mostly related to closing consumers with a lot of pre-fetch in the queue, which is an anti-pattern. One user reported issues here, and even though we will make sure he changes his usage,I  still wanted to fix these.
   
   Although the Iterator and Journal were issues independent of prefetch or AMQP.



-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#issuecomment-1456310954

   @gemmellr I will update this PR with the upgrade on Proton, and make these tests faster, and remove it from the fast-tests


-- 
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 #4375: ARTEMIS-4175 JournalFileImpl Leaking

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#issuecomment-1435665851

   another win for https://github.com/check-leak/check-leak 
   
   :)


-- 
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] michaelandrepearce commented on a diff in pull request #4375: some tasks around Memory Leaks in the broker

Posted by "michaelandrepearce (via GitHub)" <gi...@apache.org>.
michaelandrepearce commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1119626853


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
##########
@@ -418,6 +418,17 @@ public void cancel(Object brokerConsumer, Message message, boolean updateCounts)
       }
    }
 
+   /** to be used when credits were drained between handle and actual delivery */
+   public void returnToQueue(Object brokerConsumer, MessageReference reference) {

Review Comment:
   Is this issue re proton credits possibly occuring in earlier versions say 2.21? Asking as had odd issue with consumers just pausing like they run out of credits.



-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#discussion_r1119636197


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
##########
@@ -418,6 +418,17 @@ public void cancel(Object brokerConsumer, Message message, boolean updateCounts)
       }
    }
 
+   /** to be used when credits were drained between handle and actual delivery */
+   public void returnToQueue(Object brokerConsumer, MessageReference reference) {

Review Comment:
   This is being tested on ConnectionLeakTest::testCheckIterators*, which is using https://github.com/check-leak/check-leak



-- 
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 #4375: some tasks around Memory Leaks in the broker

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4375:
URL: https://github.com/apache/activemq-artemis/pull/4375#issuecomment-1448893088

   @gemmellr I have organized the PR here. I have the "bad fix" and the test separate here now. If you deal with that leak in Proton we would just drop the "proposed fix to deal with cancellations"


-- 
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