From 77024aada928af9dc69de24ab4c1223b5f11ae6a Mon Sep 17 00:00:00 2001 From: Tsuyoshi Ozawa Date: Wed, 25 Mar 2015 16:36:10 +0900 Subject: [PATCH] HADOOP-11741. Add LOG.isDebugEnabled() guard for some LOG.debug(). Contributed by Walter Su. (cherry picked from commit 5582b0f1d469e7c98811a341c4b4c78eaa64ede5) --- .../hadoop-common/CHANGES.txt | 3 + .../org/apache/hadoop/conf/Configuration.java | 4 +- .../crypto/key/JavaKeyStoreProvider.java | 14 ++-- .../hadoop/ha/ActiveStandbyElector.java | 65 +++++++++++++------ .../main/java/org/apache/hadoop/ipc/RPC.java | 9 ++- .../java/org/apache/hadoop/ipc/Server.java | 8 ++- .../metrics/ganglia/GangliaContext31.java | 6 +- .../ssl/FileBasedKeyStoresFactory.java | 22 +++++-- 8 files changed, 90 insertions(+), 41 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 9474f020b83..8724db7e187 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -29,6 +29,9 @@ Release 2.8.0 - UNRELEASED HADOOP-11737. mockito's version in hadoop-nfs’ pom.xml shouldn't be specified. (Kengo Seki via ozawa) + HADOOP-11741. Add LOG.isDebugEnabled() guard for some LOG.debug(). + (Walter Su via ozawa) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index 4cea6a48529..2ccd89a5887 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -2460,7 +2460,9 @@ public class Configuration implements Iterable>, private Document parse(DocumentBuilder builder, URL url) throws IOException, SAXException { if (!quietmode) { - LOG.debug("parsing URL " + url); + if (LOG.isDebugEnabled()) { + LOG.debug("parsing URL " + url); + } } if (url == null) { return null; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java index c0d510d51f9..091cab57f46 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java @@ -214,9 +214,11 @@ public class JavaKeyStoreProvider extends KeyProvider { renameOrFail(path, new Path(path.toString() + "_CORRUPTED_" + System.currentTimeMillis())); renameOrFail(backupPath, path); - LOG.debug(String.format( - "KeyStore loaded successfully from '%s' since '%s'" - + "was corrupted !!", backupPath, path)); + if (LOG.isDebugEnabled()) { + LOG.debug(String.format( + "KeyStore loaded successfully from '%s' since '%s'" + + "was corrupted !!", backupPath, path)); + } } else { throw ioe; } @@ -265,8 +267,10 @@ public class JavaKeyStoreProvider extends KeyProvider { try { perm = loadFromPath(pathToLoad, password); renameOrFail(pathToLoad, path); - LOG.debug(String.format("KeyStore loaded successfully from '%s'!!", - pathToLoad)); + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("KeyStore loaded successfully from '%s'!!", + pathToLoad)); + } if (fs.exists(pathToDelete)) { fs.delete(pathToDelete, true); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ActiveStandbyElector.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ActiveStandbyElector.java index 947baa93e1e..e520a165c82 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ActiveStandbyElector.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ActiveStandbyElector.java @@ -256,7 +256,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { appData = new byte[data.length]; System.arraycopy(data, 0, appData, 0, data.length); - LOG.debug("Attempting active election for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Attempting active election for " + this); + } joinElectionInternal(); } @@ -406,9 +408,11 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { public synchronized void processResult(int rc, String path, Object ctx, String name) { if (isStaleClient(ctx)) return; - LOG.debug("CreateNode result: " + rc + " for path: " + path - + " connectionState: " + zkConnectionState + - " for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("CreateNode result: " + rc + " for path: " + path + + " connectionState: " + zkConnectionState + + " for " + this); + } Code code = Code.get(rc); if (isSuccess(code)) { @@ -467,10 +471,11 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { assert wantToBeInElection : "Got a StatNode result after quitting election"; - - LOG.debug("StatNode result: " + rc + " for path: " + path - + " connectionState: " + zkConnectionState + " for " + this); - + + if (LOG.isDebugEnabled()) { + LOG.debug("StatNode result: " + rc + " for path: " + path + + " connectionState: " + zkConnectionState + " for " + this); + } Code code = Code.get(rc); if (isSuccess(code)) { @@ -535,10 +540,12 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { synchronized void processWatchEvent(ZooKeeper zk, WatchedEvent event) { Event.EventType eventType = event.getType(); if (isStaleClient(zk)) return; - LOG.debug("Watcher event type: " + eventType + " with state:" - + event.getState() + " for path:" + event.getPath() - + " connectionState: " + zkConnectionState - + " for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Watcher event type: " + eventType + " with state:" + + event.getState() + " for path:" + event.getPath() + + " connectionState: " + zkConnectionState + + " for " + this); + } if (eventType == Event.EventType.None) { // the connection state has changed @@ -597,7 +604,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { monitorActiveStatus(); break; default: - LOG.debug("Unexpected node event: " + eventType + " for path: " + path); + if (LOG.isDebugEnabled()) { + LOG.debug("Unexpected node event: " + eventType + " for path: " + path); + } monitorActiveStatus(); } @@ -646,7 +655,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { private void monitorActiveStatus() { assert wantToBeInElection; - LOG.debug("Monitoring active leader for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Monitoring active leader for " + this); + } statRetryCount = 0; monitorLockNodeAsync(); } @@ -737,7 +748,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { int connectionRetryCount = 0; boolean success = false; while(!success && connectionRetryCount < maxRetryNum) { - LOG.debug("Establishing zookeeper connection for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Establishing zookeeper connection for " + this); + } try { createConnection(); success = true; @@ -765,7 +778,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { watcher = null; } zkClient = getNewZooKeeper(); - LOG.debug("Created new connection for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Created new connection for " + this); + } } @InterfaceAudience.Private @@ -773,7 +788,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { if (zkClient == null) { return; } - LOG.debug("Terminating ZK connection for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Terminating ZK connection for " + this); + } ZooKeeper tempZk = zkClient; zkClient = null; watcher = null; @@ -800,8 +817,10 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { try { Stat oldBreadcrumbStat = fenceOldActive(); writeBreadCrumbNode(oldBreadcrumbStat); - - LOG.debug("Becoming active for " + this); + + if (LOG.isDebugEnabled()) { + LOG.debug("Becoming active for " + this); + } appClient.becomeActive(); state = State.ACTIVE; return true; @@ -906,7 +925,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { private void becomeStandby() { if (state != State.STANDBY) { - LOG.debug("Becoming standby for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Becoming standby for " + this); + } state = State.STANDBY; appClient.becomeStandby(); } @@ -914,7 +935,9 @@ public class ActiveStandbyElector implements StatCallback, StringCallback { private void enterNeutralMode() { if (state != State.NEUTRAL) { - LOG.debug("Entering neutral mode for " + this); + if (LOG.isDebugEnabled()) { + LOG.debug("Entering neutral mode for " + this); + } state = State.NEUTRAL; appClient.enterNeutralMode(); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java index 797b719c8f2..ab52113e9de 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java @@ -872,9 +872,12 @@ public class RPC { getProtocolImplMap(rpcKind).put(new ProtoNameVer(protocolName, version), new ProtoClassProtoImpl(protocolClass, protocolImpl)); - LOG.debug("RpcKind = " + rpcKind + " Protocol Name = " + protocolName + " version=" + version + - " ProtocolImpl=" + protocolImpl.getClass().getName() + - " protocolClass=" + protocolClass.getName()); + if (LOG.isDebugEnabled()) { + LOG.debug("RpcKind = " + rpcKind + " Protocol Name = " + protocolName + + " version=" + version + + " ProtocolImpl=" + protocolImpl.getClass().getName() + + " protocolClass=" + protocolClass.getName()); + } } static class VerProtocolImpl { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java index 66fefddc5e9..a5a90d330a4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java @@ -231,9 +231,11 @@ public abstract class Server { throw new IllegalArgumentException("ReRegistration of rpcKind: " + rpcKind); } - LOG.debug("rpcKind=" + rpcKind + - ", rpcRequestWrapperClass=" + rpcRequestWrapperClass + - ", rpcInvoker=" + rpcInvoker); + if (LOG.isDebugEnabled()) { + LOG.debug("rpcKind=" + rpcKind + + ", rpcRequestWrapperClass=" + rpcRequestWrapperClass + + ", rpcInvoker=" + rpcInvoker); + } } public Class getRpcRequestWrapper( diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext31.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext31.java index 2baa180fea7..4c95ee95ace 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext31.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext31.java @@ -80,8 +80,10 @@ public class GangliaContext31 extends GangliaContext { return; } - LOG.debug("Emitting metric " + name + ", type " + type + ", value " + - value + " from hostname" + hostName); + if (LOG.isDebugEnabled()) { + LOG.debug("Emitting metric " + name + ", type " + type + ", value " + + value + " from hostname" + hostName); + } String units = getUnits(name); int slope = getSlope(name); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/FileBasedKeyStoresFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/FileBasedKeyStoresFactory.java index 609c71f5f56..41634a8f445 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/FileBasedKeyStoresFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/FileBasedKeyStoresFactory.java @@ -164,7 +164,9 @@ public class FileBasedKeyStoresFactory implements KeyStoresFactory { // configuration property for key password. keystoreKeyPassword = getPassword( conf, keyPasswordProperty, keystorePassword); - LOG.debug(mode.toString() + " KeyStore: " + keystoreLocation); + if (LOG.isDebugEnabled()) { + LOG.debug(mode.toString() + " KeyStore: " + keystoreLocation); + } InputStream is = new FileInputStream(keystoreLocation); try { @@ -172,7 +174,9 @@ public class FileBasedKeyStoresFactory implements KeyStoresFactory { } finally { is.close(); } - LOG.debug(mode.toString() + " Loaded KeyStore: " + keystoreLocation); + if (LOG.isDebugEnabled()) { + LOG.debug(mode.toString() + " Loaded KeyStore: " + keystoreLocation); + } } else { keystore.load(null, null); } @@ -204,18 +208,24 @@ public class FileBasedKeyStoresFactory implements KeyStoresFactory { resolvePropertyName(mode, SSL_TRUSTSTORE_RELOAD_INTERVAL_TPL_KEY), DEFAULT_SSL_TRUSTSTORE_RELOAD_INTERVAL); - LOG.debug(mode.toString() + " TrustStore: " + truststoreLocation); + if (LOG.isDebugEnabled()) { + LOG.debug(mode.toString() + " TrustStore: " + truststoreLocation); + } trustManager = new ReloadingX509TrustManager(truststoreType, truststoreLocation, truststorePassword, truststoreReloadInterval); trustManager.init(); - LOG.debug(mode.toString() + " Loaded TrustStore: " + truststoreLocation); + if (LOG.isDebugEnabled()) { + LOG.debug(mode.toString() + " Loaded TrustStore: " + truststoreLocation); + } trustManagers = new TrustManager[]{trustManager}; } else { - LOG.debug("The property '" + locationProperty + "' has not been set, " + - "no TrustStore will be loaded"); + if (LOG.isDebugEnabled()) { + LOG.debug("The property '" + locationProperty + "' has not been set, " + + "no TrustStore will be loaded"); + } trustManagers = null; } }