Allow filtering transaction traces with a minimum 52/74652/3
authorMichael Vorburger <vorburger@redhat.com>
Mon, 30 Jul 2018 13:25:05 +0000 (15:25 +0200)
committerStephen Kitt <skitt@redhat.com>
Mon, 30 Jul 2018 13:59:10 +0000 (15:59 +0200)
This adds a "minimum open number of transactions" option to
trace:transactions and print out a clear final message indicating
whether any leaks were found or not.

JIRA: CONTROLLER-1765
Change-Id: Ie9f4ee263ed7defcb84c8b82dc8ec6f1f81ba07d
Signed-off-by: Michael Vorburger <vorburger@redhat.com>
Signed-off-by: Stephen Kitt <skitt@redhat.com>
opendaylight/md-sal/mdsal-trace/api/src/main/java/org/opendaylight/controller/md/sal/trace/api/TracingDOMDataBroker.java
opendaylight/md-sal/mdsal-trace/cli/pom.xml
opendaylight/md-sal/mdsal-trace/cli/src/main/java/org/opendaylight/controller/md/sal/trace/cli/PrintOpenTransactionsCommand.java
opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingBroker.java
opendaylight/md-sal/mdsal-trace/dom-impl/src/test/java/org/opendaylight/controller/md/sal/trace/tests/TracingBrokerTest.java

index 3b62189bd9cdc6d3543c823e1425f444f3660f43..88e2261cae2ede3b8a31cf05cd07f387e99577d1 100644 (file)
@@ -18,8 +18,9 @@ public interface TracingDOMDataBroker extends DOMDataBroker {
     /**
      * Prints a human-readable "report" of all opened but not closed transactions,
      * including transactions chains and transactions opened by them, onto the printStream.
+     * @param minOpenTransactions minimum open number of transactions (leaks with fewer are not printed)
      * @return true if there were any open transactions, false if none
      */
-    boolean printOpenTransactions(PrintStream printStream);
+    boolean printOpenTransactions(PrintStream printStream, int minOpenTransactions);
 
 }
index e337ae0161ba80fac7d39a5ad538afcf9f6413b6..a8c1dec5bb8ad4347a6e73f60441de073f685f33 100644 (file)
@@ -30,8 +30,6 @@ and is available at http://www.eclipse.org/legal/epl-v10.html
     <dependency>
       <groupId>org.apache.karaf.shell</groupId>
       <artifactId>org.apache.karaf.shell.core</artifactId>
-      <!-- TODO remove <version> once https://git.opendaylight.org/gerrit/#/c/62397/ is merged-->
-      <version>4.0.9</version>
       <exclusions>
         <exclusion>
           <groupId>*</groupId>
index a1d79a79f1ad5a3cbfee6a34bf2504f70ae7a0ef..3ebd50657f37f6da03e5ebdb7ec06ed2c675bdd6 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.md.sal.trace.cli;
 
 import java.util.List;
 import org.apache.karaf.shell.api.action.Action;
+import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
 import org.apache.karaf.shell.api.action.lifecycle.Reference;
 import org.apache.karaf.shell.api.action.lifecycle.Service;
@@ -25,6 +26,10 @@ import org.opendaylight.controller.md.sal.trace.api.TracingDOMDataBroker;
     + "if transaction-debug-context-enabled is true in mdsaltrace_config.xml")
 public class PrintOpenTransactionsCommand implements Action {
 
+    @Argument(index = 0, name = "minOpenTransactions", required = false, multiValued = false,
+            description = "Minimum open number of transactions (leaks with fewer are suppressed)")
+    Integer minOpenTransactions = 1;
+
     @Reference
     private List<TracingDOMDataBroker> tracingDOMDataBrokers;
 
@@ -34,10 +39,18 @@ public class PrintOpenTransactionsCommand implements Action {
     @Override
     @SuppressWarnings("checkstyle:RegexpSingleLineJava")
     public Object execute() {
+        boolean hasFound = false;
         for (TracingDOMDataBroker tracingDOMDataBroker : tracingDOMDataBrokers) {
-            tracingDOMDataBroker.printOpenTransactions(System.out);
+            hasFound |= tracingDOMDataBroker.printOpenTransactions(System.out, minOpenTransactions);
+        }
+        if (hasFound) {
+            System.out.println(
+                    "Actually did find real leaks with more than " + minOpenTransactions + " open transactions");
+        } else {
+            System.out.println(
+                    "Did not find any real leaks with more than " + minOpenTransactions + " open transactions");
         }
-        return null;
+        return hasFound;
     }
 
 }
index f66aa25a8a8c964bedce3e9a848a75d2c48eb67c..cdb4c716931c277fe417f86e187a13dcb94fd68b 100644 (file)
@@ -147,7 +147,7 @@ public class TracingBroker implements TracingDOMDataBroker {
             return child.startsWith(parent.substring(parentOffset), childOffset);
         }
 
-        @SuppressWarnings("checkstyle:hiddenField")
+        @SuppressWarnings({ "checkstyle:hiddenField", "hiding" })
         public boolean subtreesOverlap(YangInstanceIdentifier iid, LogicalDatastoreType store) {
             if (this.store != null && !this.store.equals(store)) {
                 return false;
@@ -157,7 +157,7 @@ public class TracingBroker implements TracingDOMDataBroker {
             return isParent(iidString, otherIidString) || isParent(otherIidString, iidString);
         }
 
-        @SuppressWarnings("checkstyle:hiddenField")
+        @SuppressWarnings({ "checkstyle:hiddenField", "hiding" })
         public boolean eventIsOfInterest(YangInstanceIdentifier iid, LogicalDatastoreType store) {
             if (this.store != null && !this.store.equals(store)) {
                 return false;
@@ -348,7 +348,7 @@ public class TracingBroker implements TracingDOMDataBroker {
     }
 
     @Override
-    public boolean printOpenTransactions(PrintStream ps) {
+    public boolean printOpenTransactions(PrintStream ps, int minOpenTXs) {
         if (transactionChainsRegistry.getAllUnique().isEmpty()
             && readOnlyTransactionsRegistry.getAllUnique().isEmpty()
             && writeTransactionsRegistry.getAllUnique().isEmpty()
@@ -363,9 +363,10 @@ public class TracingBroker implements TracingDOMDataBroker {
         ps.println("[NB: If no stack traces are shown below, then "
                  + "enable transaction-debug-context-enabled in mdsaltrace_config.xml]");
         ps.println();
-        printRegistryOpenTransactions(readOnlyTransactionsRegistry, ps, "  ");
-        printRegistryOpenTransactions(writeTransactionsRegistry, ps, "  ");
-        printRegistryOpenTransactions(readWriteTransactionsRegistry, ps, "  ");
+        // Flag to track if we really found any real leaks with more (or equal) to minOpenTXs
+        boolean hasFound = print(readOnlyTransactionsRegistry, ps, "  ", minOpenTXs);
+        hasFound |= print(writeTransactionsRegistry, ps, "  ", minOpenTXs);
+        hasFound |= print(readWriteTransactionsRegistry, ps, "  ", minOpenTXs);
 
         // Now print details for each non-closed TransactionChain
         // incl. in turn each ones own read/Write[Only]TransactionsRegistry
@@ -375,24 +376,27 @@ public class TracingBroker implements TracingDOMDataBroker {
             ps.println("  " + transactionChainsRegistry.getAnchor() + " : "
                     + transactionChainsRegistry.getCreateDescription());
         }
-        entries.forEach(entry -> {
+        for (CloseTrackedRegistryReportEntry<TracingTransactionChain> entry : entries) {
             ps.println("    " + entry.getNumberAddedNotRemoved() + "x TransactionChains opened but not closed here:");
             printStackTraceElements(ps, "      ", entry.getStackTraceElements());
             @SuppressWarnings("resource")
             TracingTransactionChain txChain = (TracingTransactionChain) entry
                 .getExampleCloseTracked().getRealCloseTracked();
-            printRegistryOpenTransactions(txChain.getReadOnlyTransactionsRegistry(), ps, "        ");
-            printRegistryOpenTransactions(txChain.getWriteTransactionsRegistry(), ps, "        ");
-            printRegistryOpenTransactions(txChain.getReadWriteTransactionsRegistry(), ps, "        ");
-        });
+            hasFound |= print(txChain.getReadOnlyTransactionsRegistry(), ps, "        ", minOpenTXs);
+            hasFound |= print(txChain.getWriteTransactionsRegistry(), ps, "        ", minOpenTXs);
+            hasFound |= print(txChain.getReadWriteTransactionsRegistry(), ps, "        ", minOpenTXs);
+        }
         ps.println();
 
-        return true;
+        return hasFound;
     }
 
-    private <T extends CloseTracked<T>> void printRegistryOpenTransactions(
-            CloseTrackedRegistry<T> registry, PrintStream ps, String indent) {
+    private <T extends CloseTracked<T>> boolean print(
+            CloseTrackedRegistry<T> registry, PrintStream ps, String indent, int minOpenTransactions) {
         Set<CloseTrackedRegistryReportEntry<T>> unsorted = registry.getAllUnique();
+        if (unsorted.size() < minOpenTransactions) {
+            return false;
+        }
 
         List<CloseTrackedRegistryReportEntry<T>> entries = new ArrayList<>(unsorted);
         entries.sort((o1, o2) -> Long.compare(o2.getNumberAddedNotRemoved(), o1.getNumberAddedNotRemoved()));
@@ -408,6 +412,7 @@ public class TracingBroker implements TracingDOMDataBroker {
         if (!entries.isEmpty()) {
             ps.println();
         }
+        return true;
     }
 
     private void printStackTraceElements(PrintStream ps, String indent, List<StackTraceElement> stackTraceElements) {
index bcd2d7c923a2ce56f3886cb33c6eaa104f091396..4e24c7d892abfb3cffd40725b7d0d73a3b054739 100644 (file)
@@ -48,7 +48,7 @@ public class TracingBrokerTest {
 
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         PrintStream ps = new PrintStream(baos);
-        boolean printReturnValue = tracingBroker.printOpenTransactions(ps);
+        boolean printReturnValue = tracingBroker.printOpenTransactions(ps, 1);
         String output = new String(baos.toByteArray(), UTF_8);
 
         assertThat(printReturnValue).isTrue();