From: Moiz Raja Date: Thu, 28 May 2015 02:22:40 +0000 (-0700) Subject: BUG 3340 : Improve logging X-Git-Tag: release/beryllium~539 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=86e398bb1e1a60f7516b398a0f27268bf232049d BUG 3340 : Improve logging - In SimpleShardDataTreeCohort we log the contents of the tree at debug level. This makes it impossible to turn on debug for the package org.opendaylight.cluster.datastore - At some point we seem to have lost the feature to include the chain number in the transaction identifier. I added that back in. Change-Id: Ifd6cd4c9f5c49790dfb045d2800b5a8beef19b37 Signed-off-by: Moiz Raja (cherry picked from commit afa53553bd476c625ae137e14c7484ff510fbcc4) --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java index bfbfb138a1..2b6b39684a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java @@ -38,7 +38,7 @@ final class SimpleShardDataTreeCohort extends ShardDataTreeCohort { public ListenableFuture canCommit() { try { dataTree.getDataTree().validate(dataTreeModification()); - LOG.debug("Transaction {} validated", transaction); + LOG.trace("Transaction {} validated", transaction); return TRUE_FUTURE; } catch (Exception e) { return Futures.immediateFailedFuture(e); @@ -53,7 +53,7 @@ final class SimpleShardDataTreeCohort extends ShardDataTreeCohort { * FIXME: this is the place where we should be interacting with persistence, specifically by invoking * persist on the candidate (which gives us a Future). */ - LOG.debug("Transaction {} prepared candidate {}", transaction, candidate); + LOG.trace("Transaction {} prepared candidate {}", transaction, candidate); return VOID_FUTURE; } catch (Exception e) { LOG.debug("Transaction {} failed to prepare", transaction, e); @@ -84,7 +84,7 @@ final class SimpleShardDataTreeCohort extends ShardDataTreeCohort { return Futures.immediateFailedFuture(e); } - LOG.debug("Transaction {} committed, proceeding to notify", transaction); + LOG.trace("Transaction {} committed, proceeding to notify", transaction); dataTree.notifyListeners(candidate); return VOID_FUTURE; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java index ff6471caa0..0946b402fd 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java @@ -13,6 +13,7 @@ import com.google.common.base.Preconditions; import java.util.Collection; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import org.opendaylight.controller.cluster.datastore.identifiers.TransactionChainIdentifier; import org.opendaylight.controller.cluster.datastore.identifiers.TransactionIdentifier; import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionChain; import org.opendaylight.controller.cluster.datastore.messages.PrimaryShardInfo; @@ -115,18 +116,19 @@ final class TransactionChainProxy extends AbstractTransactionContextFactory STATE_UPDATER = AtomicReferenceFieldUpdater.newUpdater(TransactionChainProxy.class, State.class, "currentState"); - private final String transactionChainId; + private final TransactionChainIdentifier transactionChainId; private final TransactionContextFactory parent; private volatile State currentState = IDLE_STATE; TransactionChainProxy(final TransactionContextFactory parent) { super(parent.getActorContext()); - transactionChainId = parent.getActorContext().getCurrentMemberName() + "-txn-chain-" + CHAIN_COUNTER.incrementAndGet(); + + transactionChainId = new TransactionChainIdentifier(parent.getActorContext().getCurrentMemberName(), CHAIN_COUNTER.incrementAndGet()); this.parent = parent; } public String getTransactionChainId() { - return transactionChainId; + return transactionChainId.toString(); } @Override @@ -152,7 +154,7 @@ final class TransactionChainProxy extends AbstractTransactionContextFactory returnPromise = akka.dispatch.Futures.promise(); @@ -197,10 +207,11 @@ final class TransactionChainProxy extends AbstractTransactionContextFactory stringRepresentation; - public ChainedTransactionIdentifier(final String memberName, final long counter, final String chainId) { - super(memberName, counter); - this.chainId = Preconditions.checkNotNull(chainId); + public ChainedTransactionIdentifier(final TransactionChainIdentifier chainId, final long txnCounter) { + super(chainId.getMemberName(), txnCounter); + Preconditions.checkNotNull(chainId); + this.chainId = chainId.toString(); + stringRepresentation = Suppliers.memoize(new Supplier() { + @Override + public String get() { + return new StringBuilder(chainId.toString().length() + TX_SEPARATOR.length() + 10). + append(chainId).append(TX_SEPARATOR).append(getCounter()).toString(); + } + }); } + @Override public String getChainId() { return chainId; } + + @Override + public String toString() { + return stringRepresentation.get(); + } + } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifier.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifier.java new file mode 100644 index 0000000000..4b1c096f7b --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifier.java @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2015 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ + +package org.opendaylight.controller.cluster.datastore.identifiers; + +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import java.util.concurrent.atomic.AtomicLong; + +public class TransactionChainIdentifier { + + protected static final String CHAIN_SEPARATOR = "-chn-"; + + private final AtomicLong txnCounter = new AtomicLong(); + private final Supplier stringRepresentation; + private final String memberName; + + public TransactionChainIdentifier(final String memberName, final long counter) { + this.memberName = memberName; + stringRepresentation = Suppliers.memoize(new Supplier() { + @Override + public String get() { + final StringBuilder sb = new StringBuilder(); + sb.append(memberName).append(CHAIN_SEPARATOR); + sb.append(counter); + return sb.toString(); + } + }); + } + @Override + public String toString() { + return stringRepresentation.get(); + } + + public TransactionIdentifier newTransactionIdentifier(){ + return new ChainedTransactionIdentifier(this, txnCounter.incrementAndGet()); + } + + public String getMemberName() { + return memberName; + } +} diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java index 6742b5c7db..5a365f28a3 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java @@ -9,10 +9,17 @@ package org.opendaylight.controller.cluster.datastore.identifiers; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; public class TransactionIdentifier { - private static final String TX_SEPARATOR = "-txn-"; + protected static final String TX_SEPARATOR = "-txn-"; + + protected String getMemberName() { + return memberName; + } + + protected long getCounter() { + return counter; + } private final String memberName; private final long counter; @@ -27,12 +34,8 @@ public class TransactionIdentifier { return ""; } - public static TransactionIdentifier create(String memberName, long counter, String chainId) { - if (Strings.isNullOrEmpty(chainId)) { - return new TransactionIdentifier(memberName, counter); - } else { - return new ChainedTransactionIdentifier(memberName, counter, chainId); - } + public static TransactionIdentifier create(String memberName, long counter) { + return new TransactionIdentifier(memberName, counter); } @Override @@ -72,4 +75,5 @@ public class TransactionIdentifier { return stringRepresentation; } + } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DebugThreePhaseCommitCohortTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DebugThreePhaseCommitCohortTest.java index 4427ee89bf..630e153b2a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DebugThreePhaseCommitCohortTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DebugThreePhaseCommitCohortTest.java @@ -47,7 +47,7 @@ public class DebugThreePhaseCommitCohortTest { List> expCohortFutures = new ArrayList<>(); doReturn(expCohortFutures).when(mockDelegate).getCohortFutures(); - TransactionIdentifier transactionId = TransactionIdentifier.create("1", 1, ""); + TransactionIdentifier transactionId = TransactionIdentifier.create("1", 1); Throwable debugContext = new RuntimeException("mock"); DebugThreePhaseCommitCohort cohort = new DebugThreePhaseCommitCohort(transactionId , mockDelegate , debugContext ); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java index 11ca195311..4ea81f44e3 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java @@ -203,8 +203,7 @@ public class TransactionChainProxyTest extends AbstractTransactionProxyTest { verifyOneBatchedModification(txActorRef1, new WriteModification(TestModel.TEST_PATH, writeNode1), true); - String tx2MemberName = "tx2MemberName"; - doReturn(tx2MemberName).when(mockActorContext).getCurrentMemberName(); + String tx2MemberName = "mock-member"; ActorRef shardActorRef2 = setupActorContextWithoutInitialCreateTransaction(getSystem()); ActorRef txActorRef2 = setupActorContextWithInitialCreateTransaction(getSystem(), READ_WRITE, DataStoreVersions.CURRENT_VERSION, tx2MemberName, shardActorRef2); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java new file mode 100644 index 0000000000..ba3130b5fd --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java @@ -0,0 +1,23 @@ +package org.opendaylight.controller.cluster.datastore.identifiers; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class ChainedTransactionIdentifierTest { + + @Test + public void testToString(){ + TransactionChainIdentifier chainId = new TransactionChainIdentifier("member-1", 99); + ChainedTransactionIdentifier chainedTransactionIdentifier = new ChainedTransactionIdentifier(chainId, 100); + + String txnId = chainedTransactionIdentifier.toString(); + + assertTrue(txnId.contains("member-1")); + assertTrue(txnId.contains("100")); + assertTrue(txnId.contains("99")); + + assertEquals("member-1-chn-99-txn-100", txnId); + } + +} \ No newline at end of file diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java new file mode 100644 index 0000000000..c91fe2bc00 --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java @@ -0,0 +1,29 @@ +package org.opendaylight.controller.cluster.datastore.identifiers; + +import static org.junit.Assert.assertEquals; +import org.junit.Test; + +public class TransactionChainIdentifierTest { + @Test + public void testToString(){ + TransactionChainIdentifier transactionChainIdentifier = new TransactionChainIdentifier("member-1", 99); + + String id = transactionChainIdentifier.toString(); + + assertEquals("member-1-chn-99", id); + } + + @Test + public void testNewTransactionIdentifier(){ + TransactionChainIdentifier transactionChainIdentifier = new TransactionChainIdentifier("member-1", 99); + + TransactionIdentifier txId1 = transactionChainIdentifier.newTransactionIdentifier(); + + assertEquals("member-1-chn-99-txn-1", txId1.toString()); + + TransactionIdentifier txId2 = transactionChainIdentifier.newTransactionIdentifier(); + + assertEquals("member-1-chn-99-txn-2", txId2.toString()); + } + +} \ No newline at end of file