ARTEMIS-4115 ArrayIndexOutOfBoundsException when dup cache size is 0

Allow setting id-cache-size to 0 from broker.xml and ensure the broker
handles this gracefully. Previously you could only set the cache size to
0 via broker properties or programmatically and it would throw an
ArrayIndexOutOfBoundsException when adding an item to the cache.
This commit is contained in:
Justin Bertram 2022-12-15 09:31:49 -06:00
parent 635ca1fbd6
commit cbd9f63a9d
6 changed files with 194 additions and 7 deletions

View File

@ -421,7 +421,7 @@ public final class FileConfigurationParser extends XMLConfigurationUtil {
config.setAddressQueueScanPeriod(getLong(e, "address-queue-scan-period", config.getAddressQueueScanPeriod(), Validators.MINUS_ONE_OR_GT_ZERO));
config.setIDCacheSize(getInteger(e, "id-cache-size", config.getIDCacheSize(), Validators.GT_ZERO));
config.setIDCacheSize(getInteger(e, "id-cache-size", config.getIDCacheSize(), Validators.GE_ZERO));
config.setPersistIDCache(getBoolean(e, "persist-id-cache", config.isPersistIDCache()));

View File

@ -29,11 +29,18 @@ public final class DuplicateIDCaches {
public static DuplicateIDCache persistent(final SimpleString address,
final int size,
final StorageManager storageManager) {
return new PersistentDuplicateIDCache(address, size, storageManager);
if (size == 0) {
return new NoOpDuplicateIDCache();
} else {
return new PersistentDuplicateIDCache(address, size, storageManager);
}
}
public static DuplicateIDCache inMemory(final SimpleString address, final int size) {
return new InMemoryDuplicateIDCache(address, size);
if (size == 0) {
return new NoOpDuplicateIDCache();
} else {
return new InMemoryDuplicateIDCache(address, size);
}
}
}

View File

@ -0,0 +1,77 @@
/*
* 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.core.postoffice.impl;
import java.util.Collections;
import java.util.List;
import org.apache.activemq.artemis.api.core.Pair;
import org.apache.activemq.artemis.core.postoffice.DuplicateIDCache;
import org.apache.activemq.artemis.core.transaction.Transaction;
public final class NoOpDuplicateIDCache implements DuplicateIDCache {
@Override
public boolean contains(byte[] duplicateID) {
return false;
}
@Override
public boolean atomicVerify(byte[] duplID, Transaction tx) throws Exception {
return false;
}
@Override
public void addToCache(byte[] duplicateID) throws Exception {
}
@Override
public void addToCache(byte[] duplicateID, Transaction tx) throws Exception {
}
@Override
public void addToCache(byte[] duplicateID, Transaction tx, boolean instantAdd) throws Exception {
}
@Override
public void deleteFromCache(byte[] duplicateID) throws Exception {
}
@Override
public void load(List<Pair<byte[], Long>> ids) throws Exception {
}
@Override
public void load(Transaction tx, byte[] duplID) {
}
@Override
public void clear() throws Exception {
}
@Override
public List<Pair<byte[], Long>> getMap() {
return Collections.emptyList();
}
}

View File

@ -121,6 +121,18 @@ public class FileConfigurationParserTest extends ActiveMQTestBase {
Assert.assertEquals(333, config.getClusterConfigurations().get(0).getRetryInterval());
}
@Test
public void testParsingZeroIDCacheSize() throws Exception {
FileConfigurationParser parser = new FileConfigurationParser();
String configStr = firstPart + "<id-cache-size>0</id-cache-size>" + lastPart;
ByteArrayInputStream input = new ByteArrayInputStream(configStr.getBytes(StandardCharsets.UTF_8));
Configuration config = parser.parseMainConfig(input);
Assert.assertEquals(0, config.getIDCacheSize());
}
@Test
public void testWildcardConfiguration() throws Exception {
FileConfigurationParser parser = new FileConfigurationParser();

View File

@ -19,7 +19,7 @@ package org.apache.activemq.artemis.tests.integration;
import javax.transaction.xa.XAException;
import javax.transaction.xa.XAResource;
import javax.transaction.xa.Xid;
import java.lang.invoke.MethodHandles;
import java.util.Arrays;
import java.util.Collection;
@ -37,7 +37,6 @@ import org.apache.activemq.artemis.api.core.client.ClientSession;
import org.apache.activemq.artemis.api.core.client.ClientSessionFactory;
import org.apache.activemq.artemis.api.core.client.ServerLocator;
import org.apache.activemq.artemis.core.config.Configuration;
import org.apache.activemq.artemis.core.postoffice.impl.PostOfficeImpl;
import org.apache.activemq.artemis.core.server.ActiveMQServer;
import org.apache.activemq.artemis.core.transaction.impl.XidImpl;
@ -51,7 +50,6 @@ import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.lang.invoke.MethodHandles;
@RunWith(Parameterized.class)
public class DuplicateDetectionTest extends ActiveMQTestBase {
@ -136,6 +134,81 @@ public class DuplicateDetectionTest extends ActiveMQTestBase {
Assert.assertNull(message2);
}
@Test
public void testDisabledDuplicateDetection() throws Exception {
server.stop();
config = createDefaultInVMConfig().setIDCacheSize(0);
server = createServer(config);
server.start();
sf = createSessionFactory(locator);
ClientSession session = sf.createSession(false, true, true);
session.start();
final SimpleString queueName = new SimpleString("DuplicateDetectionTestQueue");
session.createQueue(new QueueConfiguration(queueName).setDurable(false));
ClientProducer producer = session.createProducer(queueName);
ClientConsumer consumer = session.createConsumer(queueName);
ClientMessage message = createMessage(session, 0);
producer.send(message);
ClientMessage message2 = consumer.receive(1000);
Assert.assertEquals(0, message2.getObjectProperty(propKey));
message = createMessage(session, 1);
SimpleString dupID = new SimpleString("abcdefg");
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID.getData());
producer.send(message);
message2 = consumer.receive(1000);
Assert.assertEquals(1, message2.getObjectProperty(propKey));
message = createMessage(session, 2);
SimpleString dupID1 = new SimpleString("abcdefg1");
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID1.getData());
producer.send(message);
message2 = consumer.receive(1000);
Assert.assertEquals(2, message2.getObjectProperty(propKey));
message = createMessage(session, 3);
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID.getData());
producer.send(message);
message2 = consumer.receiveImmediate();
Assert.assertEquals(3, message2.getObjectProperty(propKey));
message = createMessage(session, 4);
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID.getData());
producer.send(message);
message2 = consumer.receiveImmediate();
Assert.assertEquals(4, message2.getObjectProperty(propKey));
message = createMessage(session, 5);
SimpleString dupID2 = new SimpleString("hijklmnop");
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID2.getData());
producer.send(message);
message2 = consumer.receive(1000);
Assert.assertEquals(5, message2.getObjectProperty(propKey));
message = createMessage(session, 6);
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID2.getData());
producer.send(message);
message2 = consumer.receiveImmediate();
Assert.assertEquals(6, message2.getObjectProperty(propKey));
message = createMessage(session, 7);
message.putBytesProperty(Message.HDR_DUPLICATE_DETECTION_ID, dupID.getData());
producer.send(message);
message2 = consumer.receiveImmediate();
Assert.assertEquals(7, message2.getObjectProperty(propKey));
}
@Test
public void testDuplicateIDCacheMemoryRetentionForNonTemporaryQueues() throws Exception {
testDuplicateIDCacheMemoryRetention(false);

View File

@ -128,4 +128,22 @@ public class DuplicateCacheTest extends StorageManagerTestBase {
cache.clear();
}
@Test
public void testDisabledPersistentCache() throws Exception {
createStorage();
DuplicateIDCache cache = DuplicateIDCaches.persistent(new SimpleString("test"), 0, journal);
byte[] bytes = RandomUtil.randomBytes();
// Previously this would throw an ArrayIndexOutOfBoundsException
cache.addToCache(bytes);
}
@Test
public void testDisabledInMemoryCache() throws Exception {
createStorage();
DuplicateIDCache cache = DuplicateIDCaches.inMemory(new SimpleString("test"), 0);
byte[] bytes = RandomUtil.randomBytes();
// Previously this would throw an ArrayIndexOutOfBoundsException
cache.addToCache(bytes);
}
}