Apache
/
activemq-artemis
mirror of https://github.com/apache/activemq-artemis.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ARTEMIS-4786 Avoid ConcurrentModificationException while queue.destroy in page 

We observed this assert error in the netty collection used for acks:

java.lang.AssertionError: null is not a legitimate internal value. Concurrent Modification?
at io.netty.util.collection.IntObjectHashMap.toExternal(IntObjectHashMap.java:103) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap.access$900(IntObjectHashMap.java:37) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap$PrimitiveIterator.value(IntObjectHashMap.java:650) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap$2$1.next(IntObjectHashMap.java:234) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]

this will avoid the cleanup and rely on GC for the cleanup.

PagingLeakTest is being added to make sure the cleanup is actually not needed.
This commit is contained in:
Clebert Suconic 2024-05-29 16:59:11 -04:00 committed by clebertsuconic
parent 6b5f78bfc4
commit 032597dba3
3 changed files with 212 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
View File

@ -488,7 +488,6 @@ public class PageCursorProviderImpl implements PageCursorProvider {
}
} catch (Exception ex) {
ActiveMQServerLogger.LOGGER.problemCleaningPageAddress(pagingStore.getAddress(), ex);
return;
}
}

4
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java
View File

@ -340,9 +340,6 @@ public final class PageSubscriptionImpl implements PageSubscription {
}
}
}
infoPG.acks.clear();
infoPG.removedReferences.clear();
}
tx.addOperation(new TransactionOperationAbstract() {
@ -751,7 +748,6 @@ public final class PageSubscriptionImpl implements PageSubscription {
}
}
}
info.acks.clear();
}
deletedPage.usageExhaust();
}

212
tests/leak-tests/src/test/java/org/apache/activemq/artemis/tests/leak/PagingLeakTest.java Normal file
View File

@ -0,0 +1,212 @@
/*
* 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.BytesMessage;
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.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import io.github.checkleak.core.CheckLeak;
import org.apache.activemq.artemis.api.core.QueueConfiguration;
import org.apache.activemq.artemis.api.core.RoutingType;
import org.apache.activemq.artemis.core.paging.cursor.impl.PagePositionImpl;
import org.apache.activemq.artemis.core.paging.cursor.impl.PageSubscriptionImpl;
import org.apache.activemq.artemis.core.paging.impl.PageTransactionInfoImpl;
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.Queue;
import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl;
import org.apache.activemq.artemis.core.server.impl.AddressInfo;
import org.apache.activemq.artemis.core.server.impl.ServerStatus;
import org.apache.activemq.artemis.tests.util.CFUtil;
import org.apache.activemq.artemis.utils.Wait;
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;
/* at the time this test was written JournalFileImpl was leaking through JournalFileImpl::negative creating a linked list (or leaked-list, pun intended) */
public class PagingLeakTest extends AbstractLeakTest {
private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
ActiveMQServer server;
@BeforeClass
public static void beforeClass() throws Exception {
Assume.assumeTrue(CheckLeak.isLoaded());
}
@After
public void validateServer() throws Exception {
CheckLeak checkLeak = new CheckLeak();
// I am doing this check here because the test method might hold a client connection
// so this check has to be done after the test, and before the server is stopped
assertMemory(checkLeak, 0, RemotingConnectionImpl.class.getName());
server.stop();
server = null;
clearServers();
ServerStatus.clear();
assertMemory(checkLeak, 0, ActiveMQServerImpl.class.getName());
}
@Override
@Before
public void setUp() throws Exception {
server = createServer(true, createDefaultConfig(1, true));
server.getConfiguration().setJournalPoolFiles(4).setJournalMinFiles(2);
server.start();
}
@Test
public void testAMQP() throws Exception {
doTest("AMQP");
}
@Test
public void testCore() throws Exception {
doTest("CORE");
}
@Test
public void testOpenWire() throws Exception {
doTest("OPENWIRE");
}
private void doTest(String protocol) throws Exception {
int MESSAGES = 100;
int MESSAGE_SIZE = 104;
int COMMIT_INTERVAL = 10;
basicMemoryAsserts();
ExecutorService executorService = Executors.newFixedThreadPool(2);
runAfter(executorService::shutdownNow);
ConnectionFactory cf = CFUtil.createConnectionFactory(protocol, "tcp://localhost:61616");
server.addAddressInfo(new AddressInfo(getName()).addRoutingType(RoutingType.ANYCAST));
server.createQueue(new QueueConfiguration(getName()).setAddress(getName()).setRoutingType(RoutingType.ANYCAST).setDurable(true));
Queue serverQueue = server.locateQueue(getName());
serverQueue.getPagingStore().startPaging();
try (Connection connection = cf.createConnection()) {
Session session = connection.createSession(true, Session.SESSION_TRANSACTED);
MessageProducer producer = session.createProducer(session.createQueue(getName()));
connection.start();
for (int i = 0; i < MESSAGES; i++) {
BytesMessage message = session.createBytesMessage();
message.writeBytes(new byte[MESSAGE_SIZE]);
message.setIntProperty("i", i);
producer.send(message);
if (i > 0 && i % COMMIT_INTERVAL == 0) {
session.commit();
serverQueue.getPagingStore().forceAnotherPage();
}
}
session.commit();
}
Wait.assertEquals(MESSAGES, serverQueue::getMessageCount, 5000);
CheckLeak checkLeak = new CheckLeak();
// no acks done, no PagePosition recorded
Assert.assertEquals(0, checkLeak.getAllObjects(PagePositionImpl.class).length);
serverQueue.getPagingStore().disableCleanup();
try (Connection connection = cf.createConnection()) {
Session session = connection.createSession(true, Session.SESSION_TRANSACTED);
MessageConsumer consumer = session.createConsumer(session.createQueue(getName()));
connection.start();
for (int i = 0; i < MESSAGES; i++) {
Message message = consumer.receive(5000);
Assert.assertNotNull(message);
if (i > 0 && i % COMMIT_INTERVAL == 0) {
session.commit();
}
}
session.commit();
}
// We forced paged, there should be one PageCursorInfo per page since they are all acked
Wait.assertEquals(MESSAGES / COMMIT_INTERVAL, () -> checkLeak.getAllObjects(PageSubscriptionImpl.PageCursorInfo.class).length, 5_0000, 500);
serverQueue.getPagingStore().enableCleanup();
// There should be only one holding the next page in place
Wait.assertEquals(1, () -> checkLeak.getAllObjects(PagePositionImpl.class).length, 5_000, 500);
// Everything is acked, we should not have any pageTransactions
Wait.assertEquals(0, () -> checkLeak.getAllObjects(PageTransactionInfoImpl.class).length, 5_0000, 500);
serverQueue.getPagingStore().startPaging();
try (Connection connection = cf.createConnection()) {
Session session = connection.createSession(true, Session.SESSION_TRANSACTED);
MessageProducer producer = session.createProducer(session.createQueue(getName()));
connection.start();
for (int i = 0; i < MESSAGES; i++) {
BytesMessage message = session.createBytesMessage();
message.writeBytes(new byte[MESSAGE_SIZE]);
message.setIntProperty("i", i);
producer.send(message);
if (i > 0 && i % COMMIT_INTERVAL == 0) {
session.commit();
serverQueue.getPagingStore().forceAnotherPage();
}
}
session.commit();
}
// there should be MESSAGES / COMMIT_INTERVAL in the page TX
Wait.assertEquals(MESSAGES / COMMIT_INTERVAL, () -> checkLeak.getAllObjects(PageTransactionInfoImpl.class).length, 5_0000, 500);
serverQueue.deleteQueue();
// setting it to null to make sure checkLeak will not count anything else
serverQueue = null;
// Nothing else being held after queue is removed
Wait.assertEquals(0, () -> checkLeak.getAllObjects(PageSubscriptionImpl.PageCursorInfo.class).length);
Wait.assertEquals(0, () -> checkLeak.getAllObjects(PagePositionImpl.class).length, 5_000, 500);
Wait.assertEquals(0, () -> checkLeak.getAllObjects(PageTransactionInfoImpl.class).length, 5_0000, 500);
}
}