Return-Path: Delivered-To: apmail-geronimo-scm-archive@www.apache.org Received: (qmail 11699 invoked from network); 11 Aug 2010 16:55:48 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 11 Aug 2010 16:55:48 -0000 Received: (qmail 66514 invoked by uid 500); 11 Aug 2010 16:55:48 -0000 Delivered-To: apmail-geronimo-scm-archive@geronimo.apache.org Received: (qmail 66455 invoked by uid 500); 11 Aug 2010 16:55:47 -0000 Mailing-List: contact scm-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list scm@geronimo.apache.org Received: (qmail 66448 invoked by uid 99); 11 Aug 2010 16:55:47 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Aug 2010 16:55:47 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Aug 2010 16:55:45 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id E2CAE23888DD; Wed, 11 Aug 2010 16:54:28 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r984471 - in /geronimo/components/txmanager/trunk: ./ geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/ geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/ Date: Wed, 11 Aug 2010 16:54:28 -0000 To: scm@geronimo.apache.org From: djencks@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100811165428.E2CAE23888DD@eris.apache.org> Author: djencks Date: Wed Aug 11 16:54:28 2010 New Revision: 984471 URL: http://svn.apache.org/viewvc?rev=984471&view=rev Log: GERONIMO-5519 match up the xid with the name during recovery. Add some more tracing. Merge from 2.1 branch rev 984273 Modified: geronimo/components/txmanager/trunk/ (props changed) geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java Propchange: geronimo/components/txmanager/trunk/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Wed Aug 11 16:54:28 2010 @@ -1,2 +1,2 @@ -/geronimo/components/txmanager/branches/geronimo-txmanager-parent-2.1:780438-803185,984259,984262 +/geronimo/components/txmanager/branches/geronimo-txmanager-parent-2.1:780438-803185,984259,984262,984273 /geronimo/components/txmanager/branches/geronimo-txmanager-parent-2.1.1:981270 Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java Wed Aug 11 16:54:28 2010 @@ -43,7 +43,7 @@ public class RecoverTask implements Runn this.recoverableTransactionManager = recoverableTransactionManager; } - @Override +// @Override public void run() { try { NamedXAResource namedXAResource = namedXAResourceFactory.getNamedXAResource(); Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java Wed Aug 11 16:54:28 2010 @@ -74,9 +74,11 @@ public class RecoveryImpl implements Rec } for (XidBranchesPair xidBranchesPair : preparedXids) { Xid xid = xidBranchesPair.getXid(); + log.trace("read prepared global xid from log: " + toString(xid)); if (xidFactory.matchesGlobalId(xid.getGlobalTransactionId())) { ourXids.put(new ByteArrayWrapper(xid.getGlobalTransactionId()), xidBranchesPair); for (TransactionBranchInfo transactionBranchInfo : xidBranchesPair.getBranches()) { + log.trace("read branch from log: " + transactionBranchInfo.toString()); String name = transactionBranchInfo.getResourceName(); Set transactionsForName = nameToOurTxMap.get(name); if (transactionsForName == null) { @@ -86,6 +88,7 @@ public class RecoveryImpl implements Rec transactionsForName.add(xidBranchesPair); } } else { + log.trace("read external prepared xid from log: " + toString(xid)); TransactionImpl externalTx = new ExternalTransaction(xid, txLog, retryScheduler, xidBranchesPair.getBranches()); externalXids.put(xid, externalTx); externalGlobalIdMap.put(xid.getGlobalTransactionId(), externalTx); @@ -99,6 +102,7 @@ public class RecoveryImpl implements Rec Xid[] prepared = xaResource.recover(XAResource.TMSTARTRSCAN + XAResource.TMENDRSCAN); for (int i = 0; prepared != null && i < prepared.length; i++) { Xid xid = prepared[i]; + log.trace("considering recovered xid from\n name: " + xaResource.getName() + "\n " + toString(xid)); ByteArrayWrapper globalIdWrapper = new ByteArrayWrapper(xid.getGlobalTransactionId()); XidBranchesPair xidNamesPair = ourXids.get(globalIdWrapper); @@ -107,7 +111,8 @@ public class RecoveryImpl implements Rec // Only commit if this NamedXAResource was the XAResource for the transaction. // Otherwise, wait for recoverResourceManager to be called for the actual XAResource // This is a bit wasteful, but given our management of XAResources by "name", is about the best we can do. - if (isNameInTransaction(xidNamesPair, name)) { + if (isNameInTransaction(xidNamesPair, name, xid)) { + log.trace("This xid was prepared from this XAResource: committing"); try { xaResource.commit(xid, false); } catch(XAException e) { @@ -115,9 +120,12 @@ public class RecoveryImpl implements Rec log.error("Recovery error", e); } removeNameFromTransaction(xidNamesPair, name, true); + } else { + log.trace("This xid was prepared from another XAResource, ignoring"); } } else if (xidFactory.matchesGlobalId(xid.getGlobalTransactionId())) { //ours, but prepare not logged + log.trace("this xid was initiated from this tm but not prepared: rolling back"); try { xaResource.rollback(xid); } catch (XAException e) { @@ -129,6 +137,7 @@ public class RecoveryImpl implements Rec TransactionImpl externalTx = externalGlobalIdMap.get(xid.getGlobalTransactionId()); if (externalTx == null) { //we did not prepare this branch, rollback. + log.trace("this xid is from an external transaction and was not prepared: rolling back"); try { xaResource.rollback(xid); } catch (XAException e) { @@ -136,6 +145,7 @@ public class RecoveryImpl implements Rec log.error("Could not roll back", e); } } else { + log.trace("this xid is from an external transaction and was prepared in this tm. Waiting for instructions from transaction originator"); //we prepared this branch, must wait for commit/rollback command. externalTx.addBranchXid(xaResource, xid); } @@ -150,15 +160,21 @@ public class RecoveryImpl implements Rec } } - private boolean isNameInTransaction(XidBranchesPair xidBranchesPair, String name) { + private boolean isNameInTransaction(XidBranchesPair xidBranchesPair, String name, Xid xid) { for (TransactionBranchInfo transactionBranchInfo : xidBranchesPair.getBranches()) { - if (name.equals(transactionBranchInfo.getResourceName())) { + if (name.equals(transactionBranchInfo.getResourceName()) && equals(xid, transactionBranchInfo.getBranchXid())) { return true; } } return false; } - + + private boolean equals(Xid xid1, Xid xid2) { + return xid1.getFormatId() == xid1.getFormatId() + && Arrays.equals(xid1.getBranchQualifier(), xid2.getBranchQualifier()) + && Arrays.equals(xid1.getGlobalTransactionId(), xid2.getGlobalTransactionId()); + } + private void removeNameFromTransaction(XidBranchesPair xidBranchesPair, String name, boolean warn) { int removed = 0; for (Iterator branches = xidBranchesPair.getBranches().iterator(); branches.hasNext();) { @@ -206,6 +222,28 @@ public class RecoveryImpl implements Rec return new HashMap(externalXids); } + private static String toString(Xid xid) { + if (xid instanceof XidImpl) { + return xid.toString(); + } + StringBuilder s = new StringBuilder(); + s.append("[Xid:class=").append(xid.getClass().getSimpleName()).append(":globalId="); + byte[] globalId = xid.getGlobalTransactionId(); + for (int i = 0; i < globalId.length; i++) { + s.append(Integer.toHexString(globalId[i])); + } + s.append(",length=").append(globalId.length); + s.append(",branchId="); + byte[] branchId = xid.getBranchQualifier(); + for (int i = 0; i < branchId.length; i++) { + s.append(Integer.toHexString(branchId[i])); + } + s.append(",length="); + s.append(branchId.length); + s.append("]"); + return s.toString(); + } + private static class ByteArrayWrapper { private final byte[] bytes; private final int hashCode; Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java Wed Aug 11 16:54:28 2010 @@ -32,6 +32,8 @@ public class TransactionBranchInfoImpl i private final String resourceName; public TransactionBranchInfoImpl(Xid branchXid, String resourceName) { + if (resourceName == null) throw new NullPointerException("resourceName"); + if (branchXid == null) throw new NullPointerException("branchXid"); this.branchXid = branchXid; this.resourceName = resourceName; } @@ -43,4 +45,16 @@ public class TransactionBranchInfoImpl i public String getResourceName() { return resourceName; } + + @Override + public String toString() { + StringBuilder b = new StringBuilder("[Transaction branch:\n"); + b.append(" name:").append(resourceName); + b.append("\n branchId: "); + for (byte i : branchXid.getBranchQualifier()) { + b.append(Integer.toHexString(i)); + } + b.append("\n]\n"); + return b.toString(); + } } Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java Wed Aug 11 16:54:28 2010 @@ -49,8 +49,8 @@ public abstract class AbstractRecoveryTe public void test2ResOnlineAfterRecoveryStart() throws Exception { Xid[] xids = getXidArray(XID_COUNT); - MockXAResource xares1 = new MockXAResource(RM1, xids); - MockXAResource xares2 = new MockXAResource(RM2, xids); + MockXAResource xares1 = new MockXAResource(RM1); + MockXAResource xares2 = new MockXAResource(RM2); MockTransactionInfo[] txInfos = makeTxInfos(xids); addBranch(txInfos, xares1); addBranch(txInfos, xares2); @@ -89,9 +89,9 @@ public abstract class AbstractRecoveryTe tmp.addAll(xids23List); Xid[] xids3 = (Xid[]) tmp.toArray(new Xid[6]); - MockXAResource xares1 = new MockXAResource(RM1, xids1); - MockXAResource xares2 = new MockXAResource(RM2, xids2); - MockXAResource xares3 = new MockXAResource(RM3, xids3); + MockXAResource xares1 = new MockXAResource(RM1); + MockXAResource xares2 = new MockXAResource(RM2); + MockXAResource xares3 = new MockXAResource(RM3); MockTransactionInfo[] txInfos12 = makeTxInfos(xids12); addBranch(txInfos12, xares1); addBranch(txInfos12, xares2); @@ -140,12 +140,13 @@ public abstract class AbstractRecoveryTe return xids; } - private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) { + private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) throws XAException { for (int i = 0; i < txInfos.length; i++) { MockTransactionInfo txInfo = txInfos[i]; Xid globalXid = txInfo.globalXid; Xid branchXid = xidFactory.createBranch(globalXid, branchCounter++); - txInfo.branches.add(new MockTransactionBranchInfo(xaRes.getName(), branchXid)); + xaRes.start(branchXid, 0); + txInfo.branches.add(new TransactionBranchInfoImpl(branchXid, xaRes.getName())); } } @@ -161,13 +162,12 @@ public abstract class AbstractRecoveryTe private static class MockXAResource implements NamedXAResource { private final String name; - private final Xid[] xids; - private final List committed = new ArrayList(); - private final List rolledBack = new ArrayList(); + private final List xids = new ArrayList(); + private final List committed = new ArrayList(); + private final List rolledBack = new ArrayList(); - public MockXAResource(String name, Xid[] xids) { + public MockXAResource(String name) { this.name = name; - this.xids = xids; } public String getName() { @@ -197,7 +197,7 @@ public abstract class AbstractRecoveryTe } public Xid[] recover(int flag) throws XAException { - return xids; + return xids.toArray(new Xid[xids.size()]); } public void rollback(Xid xid) throws XAException { @@ -209,6 +209,7 @@ public abstract class AbstractRecoveryTe } public void start(Xid xid, int flags) throws XAException { + xids.add(xid); } public List getCommitted() { @@ -232,21 +233,4 @@ public abstract class AbstractRecoveryTe } } - private static class MockTransactionBranchInfo implements TransactionBranchInfo { - private final String name; - private final Xid branchXid; - - public MockTransactionBranchInfo(String name, Xid branchXid) { - this.name = name; - this.branchXid = branchXid; - } - - public String getResourceName() { - return name; - } - - public Xid getBranchXid() { - return branchXid; - } - } } Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java Wed Aug 11 16:54:28 2010 @@ -33,9 +33,9 @@ import javax.transaction.xa.Xid; * */ public class MockLog implements TransactionLog { - final Map prepared = new HashMap(); - final List committed = new ArrayList(); - final List rolledBack = new ArrayList(); + final Map prepared = new HashMap(); + final List committed = new ArrayList(); + final List rolledBack = new ArrayList(); public void begin(Xid xid) throws LogException { } @@ -56,8 +56,8 @@ public class MockLog implements Transact rolledBack.add(xid); } - public Collection recover(XidFactory xidFactory) throws LogException { - Map copy = new HashMap(prepared); + public Collection recover(XidFactory xidFactory) throws LogException { + Map copy = new HashMap(prepared); copy.keySet().removeAll(committed); copy.keySet().removeAll(rolledBack); return copy.values(); Modified: geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java?rev=984471&r1=984470&r2=984471&view=diff ============================================================================== --- geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java (original) +++ geronimo/components/txmanager/trunk/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java Wed Aug 11 16:54:28 2010 @@ -40,6 +40,7 @@ public class RecoveryTest extends TestCa private final String RM1 = "rm1"; private final String RM2 = "rm2"; private final String RM3 = "rm3"; + private int count = 0; public void testCommittedRMToBeRecovered() throws Exception { MockLog mockLog = new MockLog(); @@ -48,7 +49,7 @@ public class RecoveryTest extends TestCa // to recover. This means that the presumed abort protocol has failed // right after the commit of the RM and before the force-write of the // committed log record. - MockXAResource xares1 = new MockXAResource(RM1, new Xid[0]); + MockXAResource xares1 = new MockXAResource(RM1); MockTransactionInfo[] txInfos = makeTxInfos(xids); addBranch(txInfos, xares1); prepareLog(mockLog, txInfos); @@ -58,7 +59,7 @@ public class RecoveryTest extends TestCa assertTrue(recovery.getExternalXids().isEmpty()); assertTrue(!recovery.localRecoveryComplete()); recovery.recoverResourceManager(xares1); - assertEquals(0, xares1.committed.size()); + assertEquals(1, xares1.committed.size()); assertTrue(recovery.localRecoveryComplete()); } @@ -66,8 +67,8 @@ public class RecoveryTest extends TestCa public void test2ResOnlineAfterRecoveryStart() throws Exception { MockLog mockLog = new MockLog(); Xid[] xids = getXidArray(3); - MockXAResource xares1 = new MockXAResource(RM1, xids); - MockXAResource xares2 = new MockXAResource(RM2, xids); + MockXAResource xares1 = new MockXAResource(RM1); + MockXAResource xares2 = new MockXAResource(RM2); MockTransactionInfo[] txInfos = makeTxInfos(xids); addBranch(txInfos, xares1); addBranch(txInfos, xares2); @@ -86,10 +87,12 @@ public class RecoveryTest extends TestCa } - private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) { + private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) throws XAException { for (int i = 0; i < txInfos.length; i++) { MockTransactionInfo txInfo = txInfos[i]; - txInfo.branches.add(new MockTransactionBranchInfo(xaRes.getName())); + Xid xid = xidFactory.createBranch(txInfo.globalXid, count++); + xaRes.start(xid, 0); + txInfo.branches.add(new TransactionBranchInfoImpl(xid, xaRes.getName())); } } @@ -97,7 +100,7 @@ public class RecoveryTest extends TestCa MockTransactionInfo[] txInfos = new MockTransactionInfo[xids.length]; for (int i = 0; i < xids.length; i++) { Xid xid = xids[i]; - txInfos[i] = new MockTransactionInfo(xid, new ArrayList()); + txInfos[i] = new MockTransactionInfo(xid, new ArrayList()); } return txInfos; } @@ -123,9 +126,9 @@ public class RecoveryTest extends TestCa tmp.addAll(xids23List); Xid[] xids3 = (Xid[]) tmp.toArray(new Xid[6]); - MockXAResource xares1 = new MockXAResource(RM1, xids1); - MockXAResource xares2 = new MockXAResource(RM2, xids2); - MockXAResource xares3 = new MockXAResource(RM3, xids3); + MockXAResource xares1 = new MockXAResource(RM1); + MockXAResource xares2 = new MockXAResource(RM2); + MockXAResource xares3 = new MockXAResource(RM3); MockTransactionInfo[] txInfos12 = makeTxInfos(xids12); addBranch(txInfos12, xares1); addBranch(txInfos12, xares2); @@ -174,13 +177,12 @@ public class RecoveryTest extends TestCa private static class MockXAResource implements NamedXAResource { private final String name; - private final Xid[] xids; - private final List committed = new ArrayList(); - private final List rolledBack = new ArrayList(); + private final List xids = new ArrayList(); + private final List committed = new ArrayList(); + private final List rolledBack = new ArrayList(); - public MockXAResource(String name, Xid[] xids) { + public MockXAResource(String name) { this.name = name; - this.xids = xids; } public String getName() { @@ -210,7 +212,7 @@ public class RecoveryTest extends TestCa } public Xid[] recover(int flag) throws XAException { - return xids; + return xids.toArray(new Xid[xids.size()]); } public void rollback(Xid xid) throws XAException { @@ -222,6 +224,7 @@ public class RecoveryTest extends TestCa } public void start(Xid xid, int flags) throws XAException { + xids.add(xid); } public List getCommitted() { @@ -237,27 +240,12 @@ public class RecoveryTest extends TestCa private static class MockTransactionInfo { private Xid globalXid; - private List branches; + private List branches; - public MockTransactionInfo(Xid globalXid, List branches) { + public MockTransactionInfo(Xid globalXid, List branches) { this.globalXid = globalXid; this.branches = branches; } } - private static class MockTransactionBranchInfo implements TransactionBranchInfo { - private String name; - - public MockTransactionBranchInfo(String name) { - this.name = name; - } - - public String getResourceName() { - return name; - } - - public Xid getBranchXid() { - return null; - } - } }