From e44ef1a032ea6a4e5d4f29c1e3a34f1c65b37d31 Mon Sep 17 00:00:00 2001 From: Michael Musgrove Date: Wed, 3 Apr 2024 18:00:23 +0100 Subject: [PATCH 1/2] JBTM-3859 clear MBean cache before probing --- .../tools/osb/mbean/ObjStoreBrowser.java | 34 ++-------------- .../ts/arjuna/tools/ObjStoreBrowserTest.java | 39 ++++++++++++------- 2 files changed, 28 insertions(+), 45 deletions(-) diff --git a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java index 42cc2c1f90..6de62cfc52 100644 --- a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java +++ b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java @@ -378,34 +378,6 @@ public void setExposeAllRecordsAsMBeans(boolean exposeAllLogs) { this.exposeAllLogs = exposeAllLogs; } - /** - * Update registered MBeans based on the current set of Uids. - * @param allCurrUids any registered MBeans not in this collection will be deregistered - */ - private void unregisterRemovedUids(Map> allCurrUids) { - - for (Map.Entry> e : registeredMBeans.entrySet()) { - String type = e.getKey(); - List registeredBeansOfType = e.getValue(); - Collection currUidsOfType = allCurrUids.get(type); - - if (currUidsOfType != null) { - Iterator iterator = registeredBeansOfType.iterator(); - - while (iterator.hasNext()) { - UidWrapper w = iterator.next(); - - if (!currUidsOfType.contains(w.getUid())) { - w.unregister(); - iterator.remove(); - } - } - } else { - unregisterMBeans(registeredBeansOfType); - } - } - } - /** * See if any new MBeans need to be registered or if any existing MBeans no longer exist * as ObjectStore entries. @@ -414,12 +386,12 @@ private void unregisterRemovedUids(Map> allCurrUids) { public synchronized void probe() throws MBeanException { Map> currUidsForType = new HashMap>(); + // recreate every MBean so clear the cache first + unregisterMBeans(); + for (String type : getTypes()) currUidsForType.put(type, getUids(type)); - // if there are any beans in registeredMBeans that don't appear in new list and unregister them - unregisterRemovedUids(currUidsForType); //unregisterMBeans(); - for (Map.Entry> e : currUidsForType.entrySet()) { String type = e.getKey(); diff --git a/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/tools/ObjStoreBrowserTest.java b/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/tools/ObjStoreBrowserTest.java index 6023cc8b99..e1b93f05fc 100644 --- a/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/tools/ObjStoreBrowserTest.java +++ b/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/tools/ObjStoreBrowserTest.java @@ -38,8 +38,6 @@ import com.arjuna.ats.arjuna.coordinator.RecordType; import com.arjuna.ats.arjuna.coordinator.abstractrecord.RecordTypeManager; import com.arjuna.ats.arjuna.coordinator.abstractrecord.RecordTypeMap; -import com.arjuna.ats.arjuna.recovery.RecoveryDriver; -import com.arjuna.ats.arjuna.recovery.RecoveryManager; import com.arjuna.ats.arjuna.tools.osb.mbean.ActionBean; import com.arjuna.ats.arjuna.tools.osb.mbean.LogRecordWrapper; import com.arjuna.ats.arjuna.tools.osb.mbean.OSEntryBean; @@ -51,6 +49,8 @@ import com.arjuna.ats.internal.arjuna.recovery.RecoveryManagerImple; import com.hp.mwtests.ts.arjuna.resources.CrashRecord; +import java.util.Collection; + /** * @deprecated as of 5.0.5.Final In a subsequent release we will change packages names in order to * provide a better separation between public and internal classes. @@ -182,15 +182,8 @@ public void aaTest(boolean replay) throws Exception { osb.start(); osb.probe(); - // there should be one MBean corresponding to the AtomicAction A - UidWrapper w = osb.findUid(A.get_uid()); - assertNotNull(w); - OSEntryBean ai = w.getMBean(); - assertNotNull(ai); - - // the MBean should wrap an ActionBean - assertTrue(ai instanceof ActionBean); - ActionBean actionBean = (ActionBean) ai; + // there should be one MBean corresponding to the AtomicAction A and the MBean should wrap an ActionBean + ActionBean actionBean = lookupActionBean(osb, A.get_uid()); // and there should be one MBean corresponding to the CrashRecord that got the heuristic: int recCount = 0; @@ -211,6 +204,16 @@ public void aaTest(boolean replay) throws Exception { assertEquals(1, recCount); + // verify that the state on disk is no longer a heuristic + actionBean = lookupActionBean(osb, A.get_uid()); + Collection participants = actionBean.getParticipants(); + + // there should be only the heuristic participant remaining: + assertEquals(1, participants.size()); + LogRecordWrapper wrapper = actionBean.getParticipants().iterator().next(); + // and it should no longer be a heuristic + assertFalse(wrapper.isHeuristic()); + if (!replay) { actionBean.remove(); } else { @@ -228,13 +231,21 @@ public void aaTest(boolean replay) throws Exception { osb.probe(); // look up the MBean and verify that it no longer exists - w = osb.findUid(A.get_uid()); - assertNull(w); + assertNull(osb.findUid(A.get_uid())); - osb.dump(new StringBuilder()); osb.stop(); } + private ActionBean lookupActionBean(ObjStoreBrowser osb, Uid uid) { + UidWrapper w = osb.findUid(uid); + assertNotNull(w); + OSEntryBean ai = w.getMBean(); + assertNotNull(ai); + assertTrue(ai instanceof ActionBean); + + return (ActionBean) ai; + } + // define an MBean interface for use in the next test public interface NotAnotherMBean extends ObjStoreItemMBean {} From f87f49a712863833c37f049348e68a280c1fb6d3 Mon Sep 17 00:00:00 2001 From: Michael Musgrove Date: Wed, 20 Mar 2024 18:39:22 +0000 Subject: [PATCH 2/2] JBTM-3859 MBean for JTA transaction heuristics is imprecise --- .../ats/arjuna/logging/arjunaI18NLogger.java | 9 ++ .../tools/osb/mbean/ObjStoreBrowser.java | 27 +++- .../ts/jta/tools/ObjStoreBrowserTest.java | 124 ++++++++++++++---- 3 files changed, 132 insertions(+), 28 deletions(-) diff --git a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/logging/arjunaI18NLogger.java b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/logging/arjunaI18NLogger.java index 52f21a8b85..f41c1af084 100644 --- a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/logging/arjunaI18NLogger.java +++ b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/logging/arjunaI18NLogger.java @@ -1629,6 +1629,15 @@ public void warn_objectstore_JDBCImple_over_max_image_size(int imageSize, format = MESSAGE_FORMAT) @LogMessage(level = INFO) public void info_maxIO(); + + @Message(id = 12406, value = "Invalid class type in system property ObjStoreBrowserHandlers: ''{0}''", format = MESSAGE_FORMAT) + @LogMessage(level = WARN) + void warn_invalidObjStoreBrowser_type(String type, @Cause Exception e); + + @Message(id = 12407, value = "Skipping handler for bean type: '{0}'", format = MESSAGE_FORMAT) + @LogMessage(level = INFO) + void info_osbSkipHandler(String type); + /* Allocate new messages directly above this notice. - id: use the next id number in numeric sequence. Don't reuse ids. diff --git a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java index 6de62cfc52..5454bde046 100644 --- a/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java +++ b/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/tools/osb/mbean/ObjStoreBrowser.java @@ -79,6 +79,20 @@ public class ObjStoreBrowser implements ObjStoreBrowserMBean { "StateManager/BasicAction/TwoPhaseCoordinator/AtomicAction", null ), + // JTAActionBean overrides ActionBean when the ArjunaJTA module is present, as opposed to just ArjunaCore + // on its own. + // These handlers are iterated over and added to the osbTypeMap map using typeName as the key so, + // to ensure that JTAActionBean is added to the map in JTA mode, please ensure that it appears + // last in this collection so that it overwrites the previous value. An alternative could be to make + // the beanClass field a comma separated list and the first one that is found to be known to the + // class loader would be used, or a completely different design for the instrumentation could be chosen + new OSBTypeHandler( + true, + "com.arjuna.ats.arjuna.AtomicAction", + "com.arjuna.ats.internal.jta.tools.osb.mbean.jta.JTAActionBean", + "StateManager/BasicAction/TwoPhaseCoordinator/AtomicAction", + null + ), }; private static OSBTypeHandler[] defaultJTSOsbTypes = { @@ -266,8 +280,17 @@ private void init(String logDir) { setExposeAllRecordsAsMBeans(arjPropertyManager. getObjectStoreEnvironmentBean().getExposeAllLogRecordsAsMBeans()); - for (OSBTypeHandler osbType : defaultOsbTypes) - osbTypeMap.put(osbType.getTypeName(), osbType); + for (OSBTypeHandler osbType : defaultOsbTypes) { + try { + Class.forName(osbType.getBeanClass(), true, this.getClass().getClassLoader()); + osbTypeMap.put(osbType.getTypeName(), osbType); + } catch (ClassNotFoundException ignore) { + // Some record types (currently only StateManager/BasicAction/TwoPhaseCoordinator/AtomicAction) + // use different handlers depending upon whether ArjunaJTA is present. Ignore classes which + // aren't present and log an info message + tsLogger.i18NLogger.info_osbSkipHandler(osbType.getBeanClass()); + } + } for (OSBTypeHandler osbType : defaultJTSOsbTypes) osbTypeMap.put(osbType.getTypeName(), osbType); diff --git a/ArjunaJTA/jta/tests/classes/com/hp/mwtests/ts/jta/tools/ObjStoreBrowserTest.java b/ArjunaJTA/jta/tests/classes/com/hp/mwtests/ts/jta/tools/ObjStoreBrowserTest.java index c7674b4023..49ea4378b7 100644 --- a/ArjunaJTA/jta/tests/classes/com/hp/mwtests/ts/jta/tools/ObjStoreBrowserTest.java +++ b/ArjunaJTA/jta/tests/classes/com/hp/mwtests/ts/jta/tools/ObjStoreBrowserTest.java @@ -1,12 +1,12 @@ package com.hp.mwtests.ts.jta.tools; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.IOException; import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -14,6 +14,13 @@ import javax.management.MBeanException; import javax.management.MalformedObjectNameException; import javax.management.ObjectName; + +import com.arjuna.ats.arjuna.exceptions.ObjectStoreException; +import com.arjuna.ats.internal.jta.resources.arjunacore.XAResourceRecord; +import org.jboss.tm.XAResourceWrapper; +import com.arjuna.ats.internal.jta.resources.arjunacore.XAResourceRecordWrappingPlugin; +import com.arjuna.ats.jta.common.JTAEnvironmentBean; +import com.arjuna.ats.jta.common.jtaPropertyManager; import javax.transaction.HeuristicMixedException; import javax.transaction.xa.XAException; import javax.transaction.xa.XAResource; @@ -45,8 +52,13 @@ * provide a better separation between public and internal classes. */ @Deprecated // in order to provide a better separation between public and internal classes. -class ExtendedFailureXAResource extends FailureXAResource { +class ExtendedFailureXAResource extends FailureXAResource implements XAResourceWrapper { private boolean forgotten; + private String productName; + + public ExtendedFailureXAResource(FailLocation loc) { + super(loc); + } @Override public void commit(Xid id, boolean onePhase) throws XAException { @@ -59,18 +71,34 @@ public void forget(Xid xid) throws XAException { super.forget(xid); forgotten = true; } -} -public class ObjStoreBrowserTest { - private ObjStoreBrowser osb; + @Override + public XAResource getResource() { + return null; + } - private ObjStoreBrowser createObjStoreBrowser() { - ObjStoreBrowser osb = new ObjStoreBrowser(); + public void setProductName(String productName) { + this.productName = productName; + } - osb.setType("com.arjuna.ats.arjuna.AtomicAction", "com.arjuna.ats.internal.jta.tools.osb.mbean.jta.JTAActionBean"); + @Override + public String getProductName() { + return productName; + } - return osb; - } + @Override + public String getProductVersion() { + return null; + } + + @Override + public String getJndiName() { + return null; + } +} + +public class ObjStoreBrowserTest { + private ObjStoreBrowser osb; @BeforeClass public static void setUp() { @@ -79,7 +107,8 @@ public static void setUp() { @Before public void beforeTest() { FailureXAResource.resetForgetCounts(); - osb = createObjStoreBrowser(); + + osb = new ObjStoreBrowser(); osb.start(); } @@ -106,9 +135,16 @@ public void testCommitMarkableResourceRecordBean() throws Exception { @Test public void testMBeanHeuristic () throws Exception { - FailureXAResource failureXAResource = new FailureXAResource(FailureXAResource.FailLocation.commit); // generates a heuristic on commit + // generates a heuristic on commit + ExtendedFailureXAResource failureXAResource = + new ExtendedFailureXAResource(FailureXAResource.FailLocation.commit); + String productName = "P"; + + failureXAResource.setProductName(productName); + + XAResourceRecordBeanMBean mbean = getHeuristicMBean(osb, new TransactionImple(1000000000), failureXAResource); - getHeuristicMBean(osb, new TransactionImple(1000000000), failureXAResource); + assertEquals(productName, mbean.getEisProductName()); } /** @@ -337,25 +373,61 @@ private JTAActionBean getTransactionBean(ObjStoreBrowser osb, TransactionImple t } private XAResourceRecordBeanMBean getHeuristicMBean(ObjStoreBrowser osb, TransactionImple tx, FailureXAResource failureXAResource) throws Exception { - generateHeuristic(tx, failureXAResource); + // use the wrapper plugin because we'd also like to test resource metadata + XAResourceRecordWrappingPlugin xarWrapperPlugin = new XAResourceRecordWrappingPlugin() { + String productName = ""; + @Override + public void transcribeWrapperData(XAResourceRecord xaResourceRecord) { + final XAResource xaResource = (XAResource) xaResourceRecord.value(); + + if (xaResource instanceof XAResourceWrapper) { + XAResourceWrapper xaResourceWrapper = (XAResourceWrapper) xaResource; + productName = xaResourceWrapper.getProductName(); + + xaResourceRecord.setProductName(xaResourceWrapper.getProductName()); + xaResourceRecord.setProductVersion(xaResourceWrapper.getProductVersion()); + xaResourceRecord.setJndiName(xaResourceWrapper.getJndiName()); + } + } + + @Override + public Integer getEISName(XAResource xaResource) throws IOException, ObjectStoreException { + return 0; + } + + @Override + public String getEISName(Integer eisName) { + return productName; + } + }; - osb.probe(); - // there should be one MBean corresponding to the Transaction - JTAActionBean actionBean = getTransactionBean(osb, tx, true); + JTAEnvironmentBean env = jtaPropertyManager.getJTAEnvironmentBean(); + XAResourceRecordWrappingPlugin prevWrappingPlugin = env.getXAResourceRecordWrappingPlugin(); + + try { + env.setXAResourceRecordWrappingPlugin(xarWrapperPlugin); + generateHeuristic(tx, failureXAResource); + osb.probe(); + // there should be one MBean corresponding to the Transaction + JTAActionBean actionBean = getTransactionBean(osb, tx, true); - assertNotNull(actionBean); + assertNotNull(actionBean); - // and the transaction should contain only one participant (namely the FailureXAResource that generated the heuristic): - Collection participants = actionBean.getParticipants(); + // and the transaction should contain only one participant + // (namely the FailureXAResource that generated the heuristic): + Collection participants = actionBean.getParticipants(); - assertEquals(1, participants.size()); - assertNotNull(failureXAResource.getXid()); + assertEquals(1, participants.size()); + assertNotNull(failureXAResource.getXid()); - LogRecordWrapper participant = participants.iterator().next(); + LogRecordWrapper participant = participants.iterator().next(); - assertTrue(participant.isHeuristic()); - assertTrue(participant instanceof XAResourceRecordBeanMBean); + assertTrue(participant.isHeuristic()); + assertTrue(participant instanceof XAResourceRecordBeanMBean); - return (XAResourceRecordBeanMBean) participant; + return (XAResourceRecordBeanMBean) participant; + } finally { + env.setXAResourceRecordWrappingPlugin(prevWrappingPlugin); + } } }