Transactions, when recovered, need to be recovered in prepare order:

a)  order determined by an unsorted map iterator might confuse people
b) brokerSequence can get out of step for messages recovered then committed - e.g. 
recovery on startup

git-svn-id: https://svn.apache.org/repos/asf/incubator/activemq/trunk@490795 13f79535-47bb-0310-9956-ffa450edef68

activemq-core/src/main/java/org/apache/activemq/store/journal/JournalTransactionStore.java

@@ -21,6 +21,9 @@ package org.apache.activemq.store.journal;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 
 import javax.transaction.xa.XAException;
 
@@ -34,15 +37,14 @@ import org.apache.activemq.command.XATransactionId;
 import org.apache.activemq.store.TransactionRecoveryListener;
 import org.apache.activemq.store.TransactionStore;
 
-import java.util.concurrent.ConcurrentHashMap;
 
 /**
  */
 public class JournalTransactionStore implements TransactionStore {
 
     private final JournalPersistenceAdapter peristenceAdapter;
-    ConcurrentHashMap inflightTransactions = new ConcurrentHashMap();
+    Map inflightTransactions = new LinkedHashMap();
-    ConcurrentHashMap preparedTransactions = new ConcurrentHashMap();
+    Map preparedTransactions = new LinkedHashMap();
     private boolean doingRecover;
 
     
@@ -129,26 +131,39 @@ public class JournalTransactionStore implements TransactionStore {
      * @see org.apache.activemq.store.TransactionStore#prepare(TransactionId)
      */
     public void prepare(TransactionId txid) throws IOException{
-        Tx tx = (Tx) inflightTransactions.remove(txid);
+        Tx tx=null;
+        synchronized(inflightTransactions){
+            tx=(Tx)inflightTransactions.remove(txid);
+        }
         if(tx==null)
             return;
         peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.XA_PREPARE,txid,false),true);
+        synchronized(preparedTransactions){
             preparedTransactions.put(txid,tx);
         }
+    }
     
     /**
      * @throws IOException
      * @see org.apache.activemq.store.TransactionStore#prepare(TransactionId)
      */
     public void replayPrepare(TransactionId txid) throws IOException{
-        Tx tx = (Tx) inflightTransactions.remove(txid);
+        Tx tx=null;
+        synchronized(inflightTransactions){
+            tx=(Tx)inflightTransactions.remove(txid);
+        }
         if(tx==null)
             return;
+        synchronized(preparedTransactions){
             preparedTransactions.put(txid,tx);
         }
+    }
 
     public Tx getTx(Object txid,RecordLocation location){
-        Tx tx = (Tx) inflightTransactions.get(txid);
+        Tx tx=null;
+        synchronized(inflightTransactions){
+            tx=(Tx)inflightTransactions.get(txid);
+        }
         if(tx==null){
             tx=new Tx(location);
             inflightTransactions.put(txid,tx);
@@ -163,17 +178,18 @@ public class JournalTransactionStore implements TransactionStore {
     public void commit(TransactionId txid,boolean wasPrepared) throws IOException{
         Tx tx;
         if(wasPrepared){
+            synchronized(preparedTransactions){
                 tx=(Tx)preparedTransactions.remove(txid);
+            }
         }else{
+            synchronized(inflightTransactions){
                 tx=(Tx)inflightTransactions.remove(txid);
             }
-
+        }
         if(tx==null)
             return;
-
         if(txid.isXATransaction()){
-            peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.XA_COMMIT, txid, wasPrepared),
+            peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.XA_COMMIT,txid,wasPrepared),true);
-                    true);
         }else{
             peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.LOCAL_COMMIT,txid,wasPrepared),
                     true);
@@ -186,32 +202,37 @@ public class JournalTransactionStore implements TransactionStore {
      */
     public Tx replayCommit(TransactionId txid,boolean wasPrepared) throws IOException{
         if(wasPrepared){
+            synchronized(preparedTransactions){
                 return (Tx)preparedTransactions.remove(txid);
+            }
         }else{
+            synchronized(inflightTransactions){
                 return (Tx)inflightTransactions.remove(txid);
             }
         }
+    }
 
     /**
      * @throws IOException
      * @see org.apache.activemq.store.TransactionStore#rollback(TransactionId)
      */
     public void rollback(TransactionId txid) throws IOException{
-
+        Tx tx=null;
-        Tx tx = (Tx) inflightTransactions.remove(txid);
+        synchronized(inflightTransactions){
+            tx=(Tx)inflightTransactions.remove(txid);
+        }
         if(tx!=null)
+            synchronized(preparedTransactions){
                 tx=(Tx)preparedTransactions.remove(txid);
-
+            }
         if(tx!=null){
             if(txid.isXATransaction()){
-                peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.XA_ROLLBACK, txid, false),
+                peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.XA_ROLLBACK,txid,false),true);
-                        true);
             }else{
                 peristenceAdapter.writeCommand(new JournalTransaction(JournalTransaction.LOCAL_ROLLBACK,txid,false),
                         true);
             }
         }
-
     }
 
     /**
@@ -219,9 +240,16 @@ public class JournalTransactionStore implements TransactionStore {
      * @see org.apache.activemq.store.TransactionStore#rollback(TransactionId)
      */
     public void replayRollback(TransactionId txid) throws IOException{
-        if (inflightTransactions.remove(txid) != null)
+        boolean inflight=false;
+        synchronized(inflightTransactions){
+            inflight=inflightTransactions.remove(txid)!=null;
+        }
+        if(inflight){
+            synchronized(preparedTransactions){
                 preparedTransactions.remove(txid);
             }
+        }
+    }
     
     public void start() throws Exception {
     }
@@ -231,12 +259,18 @@ public class JournalTransactionStore implements TransactionStore {
     
     synchronized public void recover(TransactionRecoveryListener listener) throws IOException{
         // All the in-flight transactions get rolled back..
+        synchronized(inflightTransactions){
             inflightTransactions.clear();
+        }
         this.doingRecover=true;
         try{
-            for (Iterator iter = preparedTransactions.keySet().iterator(); iter.hasNext();) {
+            Map txs=null;
+            synchronized(preparedTransactions){
+                txs=new LinkedHashMap(preparedTransactions);
+            }
+            for(Iterator iter=txs.keySet().iterator();iter.hasNext();){
                 Object txid=(Object)iter.next();
-                Tx tx = (Tx) preparedTransactions.get(txid);
+                Tx tx=(Tx)txs.get(txid);
                 listener.recover((XATransactionId)txid,tx.getMessages(),tx.getAcks());
             }
         }finally{
@@ -270,14 +304,13 @@ public class JournalTransactionStore implements TransactionStore {
 
 
     public RecordLocation checkpoint() throws IOException{
-        
         // Nothing really to checkpoint.. since, we don't
         // checkpoint tx operations in to long term store until they are committed.
-
         // But we keep track of the first location of an operation
         // that was associated with an active tx. The journal can not
         // roll over active tx records.
         RecordLocation rc=null;
+        synchronized(inflightTransactions){
             for(Iterator iter=inflightTransactions.values().iterator();iter.hasNext();){
                 Tx tx=(Tx)iter.next();
                 RecordLocation location=tx.location;
@@ -285,6 +318,8 @@ public class JournalTransactionStore implements TransactionStore {
                     rc=location;
                 }
             }
+        }
+        synchronized(preparedTransactions){
             for(Iterator iter=preparedTransactions.values().iterator();iter.hasNext();){
                 Tx tx=(Tx)iter.next();
                 RecordLocation location=tx.location;
@@ -294,6 +329,7 @@ public class JournalTransactionStore implements TransactionStore {
             }
             return rc;
         }
+    }
 
     public boolean isDoingRecover() {
         return doingRecover;