diff --git a/activemq-client/src/main/java/org/apache/activemq/command/XATransactionId.java b/activemq-client/src/main/java/org/apache/activemq/command/XATransactionId.java index 84fea7af98..b7c517ed96 100644 --- a/activemq-client/src/main/java/org/apache/activemq/command/XATransactionId.java +++ b/activemq-client/src/main/java/org/apache/activemq/command/XATransactionId.java @@ -22,6 +22,7 @@ import java.util.Arrays; import javax.transaction.xa.Xid; import org.apache.activemq.util.DataByteArrayInputStream; import org.apache.activemq.util.DataByteArrayOutputStream; +import org.apache.activemq.util.JenkinsHash; /** * @openwire:marshaller code="112" @@ -199,8 +200,9 @@ public class XATransactionId extends TransactionId implements Xid, Comparable { public int hashCode() { if (hash == 0) { hash = formatId; - hash = hash(globalTransactionId, hash); - hash = hash(branchQualifier, hash); + JenkinsHash jh = JenkinsHash.getInstance(); + hash = jh.hash(globalTransactionId, hash); + hash = jh.hash(branchQualifier, hash); if (hash == 0) { hash = 0xaceace; } @@ -208,14 +210,6 @@ public class XATransactionId extends TransactionId implements Xid, Comparable { return hash; } - private static int hash(byte[] bytes, int hash) { - int size = bytes.length; - for (int i = 0; i < size; i++) { - hash ^= bytes[i] << ((i % 4) * 8); - } - return hash; - } - public boolean equals(Object o) { if (o == null || o.getClass() != XATransactionId.class) { return false; diff --git a/activemq-client/src/main/java/org/apache/activemq/util/JenkinsHash.java b/activemq-client/src/main/java/org/apache/activemq/util/JenkinsHash.java new file mode 100644 index 0000000000..d9524cef47 --- /dev/null +++ b/activemq-client/src/main/java/org/apache/activemq/util/JenkinsHash.java @@ -0,0 +1,258 @@ +/** + * 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.util; + +public class JenkinsHash { + + private static final long INT_MASK = 0x00000000ffffffffL; + private static final long BYTE_MASK = 0x00000000000000ffL; + + private static final JenkinsHash _instance = new JenkinsHash(); + + public static JenkinsHash getInstance() { + return _instance; + } + + private static long rot(long val, int pos) { + return ((Integer.rotateLeft((int) (val & INT_MASK), pos)) & INT_MASK); + } + + /** + * Calculate a hash using all bytes from the input argument, and + * a seed of -1. + * @param bytes input bytes + * @return hash value + */ + public int hash(byte[] bytes) { + return hash(bytes, bytes.length, -1); + } + + /** + * Calculate a hash using all bytes from the input argument, and + * a seed of -1. + * @param bytes input bytes + * @return hash value + */ + public int hash(byte[] bytes, int initVal) { + return hash(bytes, bytes.length, initVal); + } + + /** + * taken from hashlittle() -- hash a variable-length key into a 32-bit value + * + * @param key the key (the unaligned variable-length array of bytes) + * @param nbytes number of bytes to include in hash + * @param initval can be any integer value + * @return a 32-bit value. Every bit of the key affects every bit of the + * return value. Two keys differing by one or two bits will have totally + * different hash values. + *
+ *
The best hash table sizes are powers of 2. There is no need to do mod
+ * a prime (mod is sooo slow!). If you need less than 32 bits, use a bitmask.
+ * For example, if you need only 10 bits, do
+ * h = (h & hashmask(10));
+ * In which case, the hash table should have hashsize(10) elements.
+ *
+ *
If you are hashing n strings byte[][] k, do it like this: + * for (int i = 0, h = 0; i < n; ++i) h = hash( k[i], h); + *
+ *
By Bob Jenkins, 2006. bob_jenkins@burtleburtle.net. You may use this + * code any way you wish, private, educational, or commercial. It's free. + *
+ *
Use for hash table lookup, or anything where one collision in 2^^32 is + * acceptable. Do NOT use for cryptographic purposes. + */ + public int hash(byte[] key, int nbytes, int initval) { + int length = nbytes; + long a, b, c; // We use longs because we don't have unsigned ints + a = b = c = (0x00000000deadbeefL + length + initval) & INT_MASK; + int offset = 0; + for (; length > 12; offset += 12, length -= 12) { + a = (a + (key[offset + 0] & BYTE_MASK)) & INT_MASK; + a = (a + (((key[offset + 1] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + a = (a + (((key[offset + 2] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + a = (a + (((key[offset + 3] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + b = (b + (key[offset + 4] & BYTE_MASK)) & INT_MASK; + b = (b + (((key[offset + 5] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + b = (b + (((key[offset + 6] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + b = (b + (((key[offset + 7] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + c = (c + (key[offset + 8] & BYTE_MASK)) & INT_MASK; + c = (c + (((key[offset + 9] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + c = (c + (((key[offset + 10] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + c = (c + (((key[offset + 11] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + + /* + * mix -- mix 3 32-bit values reversibly. + * This is reversible, so any information in (a,b,c) before mix() is + * still in (a,b,c) after mix(). + * + * If four pairs of (a,b,c) inputs are run through mix(), or through + * mix() in reverse, there are at least 32 bits of the output that + * are sometimes the same for one pair and different for another pair. + * + * This was tested for: + * - pairs that differed by one bit, by two bits, in any combination + * of top bits of (a,b,c), or in any combination of bottom bits of + * (a,b,c). + * - "differ" is defined as +, -, ^, or ~^. For + and -, I transformed + * the output delta to a Gray code (a^(a>>1)) so a string of 1's (as + * is commonly produced by subtraction) look like a single 1-bit + * difference. + * - the base values were pseudorandom, all zero but one bit set, or + * all zero plus a counter that starts at zero. + * + * Some k values for my "a-=c; a^=rot(c,k); c+=b;" arrangement that + * satisfy this are + * 4 6 8 16 19 4 + * 9 15 3 18 27 15 + * 14 9 3 7 17 3 + * Well, "9 15 3 18 27 15" didn't quite get 32 bits diffing for + * "differ" defined as + with a one-bit base and a two-bit delta. I + * used http://burtleburtle.net/bob/hash/avalanche.html to choose + * the operations, constants, and arrangements of the variables. + * + * This does not achieve avalanche. There are input bits of (a,b,c) + * that fail to affect some output bits of (a,b,c), especially of a. + * The most thoroughly mixed value is c, but it doesn't really even + * achieve avalanche in c. + * + * This allows some parallelism. Read-after-writes are good at doubling + * the number of bits affected, so the goal of mixing pulls in the + * opposite direction as the goal of parallelism. I did what I could. + * Rotates seem to cost as much as shifts on every machine I could lay + * my hands on, and rotates are much kinder to the top and bottom bits, + * so I used rotates. + * + * #define mix(a,b,c) \ + * { \ + * a -= c; a ^= rot(c, 4); c += b; \ + * b -= a; b ^= rot(a, 6); a += c; \ + * c -= b; c ^= rot(b, 8); b += a; \ + * a -= c; a ^= rot(c,16); c += b; \ + * b -= a; b ^= rot(a,19); a += c; \ + * c -= b; c ^= rot(b, 4); b += a; \ + * } + * + * mix(a,b,c); + */ + a = (a - c) & INT_MASK; + a ^= rot(c, 4); + c = (c + b) & INT_MASK; + b = (b - a) & INT_MASK; + b ^= rot(a, 6); + a = (a + c) & INT_MASK; + c = (c - b) & INT_MASK; + c ^= rot(b, 8); + b = (b + a) & INT_MASK; + a = (a - c) & INT_MASK; + a ^= rot(c, 16); + c = (c + b) & INT_MASK; + b = (b - a) & INT_MASK; + b ^= rot(a, 19); + a = (a + c) & INT_MASK; + c = (c - b) & INT_MASK; + c ^= rot(b, 4); + b = (b + a) & INT_MASK; + } + + //-------------------------------- last block: affect all 32 bits of (c) + switch (length) { // all the case statements fall through + case 12: + c = (c + (((key[offset + 11] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + case 11: + c = (c + (((key[offset + 10] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + case 10: + c = (c + (((key[offset + 9] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + case 9: + c = (c + (key[offset + 8] & BYTE_MASK)) & INT_MASK; + case 8: + b = (b + (((key[offset + 7] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + case 7: + b = (b + (((key[offset + 6] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + case 6: + b = (b + (((key[offset + 5] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + case 5: + b = (b + (key[offset + 4] & BYTE_MASK)) & INT_MASK; + case 4: + a = (a + (((key[offset + 3] & BYTE_MASK) << 24) & INT_MASK)) & INT_MASK; + case 3: + a = (a + (((key[offset + 2] & BYTE_MASK) << 16) & INT_MASK)) & INT_MASK; + case 2: + a = (a + (((key[offset + 1] & BYTE_MASK) << 8) & INT_MASK)) & INT_MASK; + case 1: + a = (a + (key[offset + 0] & BYTE_MASK)) & INT_MASK; + break; + case 0: + return (int) (c & INT_MASK); + } + + /* + * final -- final mixing of 3 32-bit values (a,b,c) into c + * + * Pairs of (a,b,c) values differing in only a few bits will usually + * produce values of c that look totally different. This was tested for + * - pairs that differed by one bit, by two bits, in any combination + * of top bits of (a,b,c), or in any combination of bottom bits of + * (a,b,c). + * + * - "differ" is defined as +, -, ^, or ~^. For + and -, I transformed + * the output delta to a Gray code (a^(a>>1)) so a string of 1's (as + * is commonly produced by subtraction) look like a single 1-bit + * difference. + * + * - the base values were pseudorandom, all zero but one bit set, or + * all zero plus a counter that starts at zero. + * + * These constants passed: + * 14 11 25 16 4 14 24 + * 12 14 25 16 4 14 24 + * and these came close: + * 4 8 15 26 3 22 24 + * 10 8 15 26 3 22 24 + * 11 8 15 26 3 22 24 + * + * #define final(a,b,c) \ + * { + * c ^= b; c -= rot(b,14); \ + * a ^= c; a -= rot(c,11); \ + * b ^= a; b -= rot(a,25); \ + * c ^= b; c -= rot(b,16); \ + * a ^= c; a -= rot(c,4); \ + * b ^= a; b -= rot(a,14); \ + * c ^= b; c -= rot(b,24); \ + * } + * + */ + c ^= b; + c = (c - rot(b, 14)) & INT_MASK; + a ^= c; + a = (a - rot(c, 11)) & INT_MASK; + b ^= a; + b = (b - rot(a, 25)) & INT_MASK; + c ^= b; + c = (c - rot(b, 16)) & INT_MASK; + a ^= c; + a = (a - rot(c, 4)) & INT_MASK; + b ^= a; + b = (b - rot(a, 14)) & INT_MASK; + c ^= b; + c = (c - rot(b, 24)) & INT_MASK; + + return (int) (c & INT_MASK); + } + +} diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapter.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapter.java index c4f480c13f..fa0f7c22b1 100644 --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapter.java +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapter.java @@ -547,6 +547,14 @@ public class KahaDBPersistenceAdapter extends LockableServiceSupport implements letter.setCheckForCorruptJournalFiles(checkForCorruptJournalFiles); } + public boolean isPurgeRecoveredXATransactions() { + return letter.isPurgeRecoveredXATransactions(); + } + + public void setPurgeRecoveredXATransactions(boolean purgeRecoveredXATransactions) { + letter.setPurgeRecoveredXATransactions(purgeRecoveredXATransactions); + } + @Override public void setBrokerService(BrokerService brokerService) { super.setBrokerService(brokerService); diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java index 26e8cd0481..413e137a30 100644 --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java @@ -272,6 +272,7 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe private boolean ignoreMissingJournalfiles = false; private int indexCacheSize = 10000; private boolean checkForCorruptJournalFiles = false; + private boolean purgeRecoveredXATransactions = false; private boolean checksumJournalFiles = true; protected boolean forceRecoverIndex = false; private boolean archiveCorruptedIndex = false; @@ -748,6 +749,13 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe for (TransactionId txId : preparedTransactions.keySet()) { LOG.warn("Recovered prepared XA TX: [{}]", txId); } + + if (purgeRecoveredXATransactions){ + if (!preparedTransactions.isEmpty()){ + LOG.warn("Purging " + preparedTransactions.size() + " recovered prepared XA TXs" ); + preparedTransactions.clear(); + } + } } } finally { @@ -3307,6 +3315,14 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe this.checkForCorruptJournalFiles = checkForCorruptJournalFiles; } + public boolean isPurgeRecoveredXATransactions() { + return purgeRecoveredXATransactions; + } + + public void setPurgeRecoveredXATransactions(boolean purgeRecoveredXATransactions) { + this.purgeRecoveredXATransactions = purgeRecoveredXATransactions; + } + public boolean isChecksumJournalFiles() { return checksumJournalFiles; } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRestartTestSupport.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRestartTestSupport.java index c4e3848803..111494a027 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRestartTestSupport.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRestartTestSupport.java @@ -58,10 +58,13 @@ public class BrokerRestartTestSupport extends BrokerTestSupport { * @throws URISyntaxException */ protected void restartBroker() throws Exception { - broker.stop(); - broker.waitUntilStopped(); - broker = createRestartedBroker(); + stopBroker(); broker.start(); } + protected void stopBroker() throws Exception { + broker.stop(); + broker.waitUntilStopped(); + broker = createRestartedBroker(); + } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/XARecoveryBrokerTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/XARecoveryBrokerTest.java index 6a8b3f41f3..60d3b8b416 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/XARecoveryBrokerTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/XARecoveryBrokerTest.java @@ -267,6 +267,72 @@ public class XARecoveryBrokerTest extends BrokerRestartTestSupport { assertEmptyDLQ(); } + public void testPreparedTransactionRecoveredPurgeOnRestart() throws Exception { + + ActiveMQDestination destination = createDestination(); + + // Setup the producer and send the message. + StubConnection connection = createConnection(); + ConnectionInfo connectionInfo = createConnectionInfo(); + SessionInfo sessionInfo = createSessionInfo(connectionInfo); + ProducerInfo producerInfo = createProducerInfo(sessionInfo); + connection.send(connectionInfo); + connection.send(sessionInfo); + connection.send(producerInfo); + ConsumerInfo consumerInfo = createConsumerInfo(sessionInfo, destination); + connection.send(consumerInfo); + + // Prepare 4 message sends. + for (int i = 0; i < 4; i++) { + // Begin the transaction. + XATransactionId txid = createXATransaction(sessionInfo); + connection.send(createBeginTransaction(connectionInfo, txid)); + + Message message = createMessage(producerInfo, destination); + message.setPersistent(true); + message.setTransactionId(txid); + connection.send(message); + + // Prepare + connection.send(createPrepareTransaction(connectionInfo, txid)); + } + + // Since prepared but not committed.. they should not get delivered. + assertNull(receiveMessage(connection)); + assertNoMessagesLeft(connection); + connection.request(closeConnectionInfo(connectionInfo)); + + // restart the broker. + stopBroker(); + if (broker.getPersistenceAdapter() instanceof KahaDBPersistenceAdapter) { + KahaDBPersistenceAdapter adapter = (KahaDBPersistenceAdapter)broker.getPersistenceAdapter(); + adapter.setPurgeRecoveredXATransactions(true); + LOG.info("Setting purgeRecoveredXATransactions to true on the KahaDBPersistenceAdapter"); + } + broker.start(); + + // Setup the consumer and try receive the message. + connection = createConnection(); + connectionInfo = createConnectionInfo(); + sessionInfo = createSessionInfo(connectionInfo); + connection.send(connectionInfo); + connection.send(sessionInfo); + consumerInfo = createConsumerInfo(sessionInfo, destination); + connection.send(consumerInfo); + + // Since prepared but not committed.. they should not get delivered. + assertNull(receiveMessage(connection)); + assertNoMessagesLeft(connection); + + Response response = connection.request(new TransactionInfo(connectionInfo.getConnectionId(), null, TransactionInfo.RECOVER)); + assertNotNull(response); + DataArrayResponse dar = (DataArrayResponse)response; + + //These should be purged so expect 0 + assertEquals(0, dar.getData().length); + + } + private void assertEmptyDLQ() throws Exception { try { DestinationViewMBean destinationView = getProxyToDestination(new ActiveMQQueue(SharedDeadLetterStrategy.DEFAULT_DEAD_LETTER_QUEUE_NAME)); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ7013Test.java b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ7013Test.java new file mode 100644 index 0000000000..87e21a3004 --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ7013Test.java @@ -0,0 +1,55 @@ +/** + * 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.bugs; + +import org.apache.activemq.command.XATransactionId; +import org.junit.Test; + +import static org.junit.Assert.assertNotEquals; + +public class AMQ7013Test { + + @Test + public void hashTest() throws Exception{ + + byte[] globalId1 = hexStringToByteArray("00000000000000000000ffff0a970616dbbe2c3b5b42f94800002259"); + byte[] branchQualifier1 = hexStringToByteArray("00000000000000000000ffff0a970616dbbe2c3b5b42f94800002259"); + XATransactionId id1 = new XATransactionId(); + id1.setGlobalTransactionId(globalId1); + id1.setBranchQualifier(branchQualifier1); + id1.setFormatId(131077); + + byte[] globalId2 = hexStringToByteArray("00000000000000000000ffff0a970616dbbe2c3b5b42f948000021d2"); + byte[] branchQualifier2 = hexStringToByteArray("00000000000000000000ffff0a970616dbbe2c3b5b42f948000021d2"); + XATransactionId id2 = new XATransactionId(); + id2.setGlobalTransactionId(globalId2); + id2.setBranchQualifier(branchQualifier2); + id2.setFormatId(131077); + + assertNotEquals(id1.hashCode(), id2.hashCode()); + } + + public byte[] hexStringToByteArray(String s) { + int len = s.length(); + byte[] data = new byte[len / 2]; + for (int i = 0; i < len; i += 2) { + data[i / 2] = (byte) ((Character.digit(s.charAt(i), 16) << 4) + + Character.digit(s.charAt(i+1), 16)); + } + return data; + } +} \ No newline at end of file