Repository: incubator-tephra Updated Branches: refs/heads/master b3370b662 -> a9d811922
(TEPHRA-258) Improve logging in thrift client providers This closes #56 from GitHub. Signed-off-by: anew <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-tephra/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-tephra/commit/641e915f Tree: http://git-wip-us.apache.org/repos/asf/incubator-tephra/tree/641e915f Diff: http://git-wip-us.apache.org/repos/asf/incubator-tephra/diff/641e915f Branch: refs/heads/master Commit: 641e915f252d664369c163914a121b97cc7c2d74 Parents: 7cfe061 Author: anew <[email protected]> Authored: Mon Sep 11 18:15:30 2017 -0700 Committer: anew <[email protected]> Committed: Mon Sep 11 22:57:38 2017 -0700 ---------------------------------------------------------------------- .../distributed/AbstractClientProvider.java | 27 +++++++------------- .../distributed/PooledClientProvider.java | 17 +++++------- .../distributed/SingleUseClientProvider.java | 7 +---- .../distributed/ThreadLocalClientProvider.java | 10 ++------ 4 files changed, 18 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-tephra/blob/641e915f/tephra-core/src/main/java/org/apache/tephra/distributed/AbstractClientProvider.java ---------------------------------------------------------------------- diff --git a/tephra-core/src/main/java/org/apache/tephra/distributed/AbstractClientProvider.java b/tephra-core/src/main/java/org/apache/tephra/distributed/AbstractClientProvider.java index 3abdafa..44be459 100644 --- a/tephra-core/src/main/java/org/apache/tephra/distributed/AbstractClientProvider.java +++ b/tephra-core/src/main/java/org/apache/tephra/distributed/AbstractClientProvider.java @@ -24,7 +24,6 @@ import org.apache.thrift.TException; import org.apache.thrift.transport.TFramedTransport; import org.apache.thrift.transport.TSocket; import org.apache.thrift.transport.TTransport; -import org.apache.thrift.transport.TTransportException; import org.apache.twill.discovery.Discoverable; import org.apache.twill.discovery.DiscoveryServiceClient; import org.slf4j.Logger; @@ -93,21 +92,20 @@ public abstract class AbstractClientProvider implements ThriftClientProvider { if (endpointStrategy == null) { // if there is no discovery service, try to read host and port directly // from the configuration - LOG.info("Reading address and port from configuration."); + LOG.debug("Reading transaction service address and port from configuration."); address = configuration.get(TxConstants.Service.CFG_DATA_TX_BIND_ADDRESS, TxConstants.Service.DEFAULT_DATA_TX_BIND_ADDRESS); port = configuration.getInt(TxConstants.Service.CFG_DATA_TX_BIND_PORT, TxConstants.Service.DEFAULT_DATA_TX_BIND_PORT); - LOG.info("Service assumed at " + address + ":" + port); + LOG.debug("Transaction service configured at {}:{}.", address, port); } else { Discoverable endpoint = endpointStrategy.pick(); if (endpoint == null) { - LOG.error("Unable to discover tx service."); - throw new TException("Unable to discover tx service."); + throw new TException("Unable to discover transaction service."); } address = endpoint.getSocketAddress().getHostName(); port = endpoint.getSocketAddress().getPort(); - LOG.info("Service discovered at " + address + ":" + port); + LOG.debug("Transaction service discovered at {}:{}.", address, port); } // now we have an address and port, try to connect a client @@ -115,22 +113,15 @@ public abstract class AbstractClientProvider implements ThriftClientProvider { timeout = configuration.getInt(TxConstants.Service.CFG_DATA_TX_CLIENT_TIMEOUT, TxConstants.Service.DEFAULT_DATA_TX_CLIENT_TIMEOUT_MS); } - LOG.info("Attempting to connect to tx service at " + - address + ":" + port + " with timeout " + timeout + " ms."); + LOG.debug("Attempting to connect to transaction service at {}:{} with RPC timeout of {} ms." + + address, port, timeout); // thrift transport layer - TTransport transport = - new TFramedTransport(new TSocket(address, port, timeout)); - try { - transport.open(); - } catch (TTransportException e) { - LOG.error("Unable to connect to tx service: " + e.getMessage()); - throw e; - } + TTransport transport = new TFramedTransport(new TSocket(address, port, timeout)); + transport.open(); // and create a thrift client TransactionServiceThriftClient newClient = new TransactionServiceThriftClient(transport); - LOG.info("Connected to tx service at " + - address + ":" + port); + LOG.debug("Connected to transaction service at {}:{}.", address, port); return newClient; } http://git-wip-us.apache.org/repos/asf/incubator-tephra/blob/641e915f/tephra-core/src/main/java/org/apache/tephra/distributed/PooledClientProvider.java ---------------------------------------------------------------------- diff --git a/tephra-core/src/main/java/org/apache/tephra/distributed/PooledClientProvider.java b/tephra-core/src/main/java/org/apache/tephra/distributed/PooledClientProvider.java index 9df1441..95fc8ad 100644 --- a/tephra-core/src/main/java/org/apache/tephra/distributed/PooledClientProvider.java +++ b/tephra-core/src/main/java/org/apache/tephra/distributed/PooledClientProvider.java @@ -18,7 +18,6 @@ package org.apache.tephra.distributed; -import com.google.common.base.Throwables; import org.apache.hadoop.conf.Configuration; import org.apache.tephra.TxConstants; import org.apache.thrift.TException; @@ -78,9 +77,8 @@ public class PooledClientProvider extends AbstractClientProvider { maxClients = configuration.getInt(TxConstants.Service.CFG_DATA_TX_CLIENT_COUNT, TxConstants.Service.DEFAULT_DATA_TX_CLIENT_COUNT); if (maxClients < 1) { - LOG.warn("Configuration of " + TxConstants.Service.CFG_DATA_TX_CLIENT_COUNT + - " is invalid: value is " + maxClients + " but must be at least 1. " + - "Using 1 as a fallback. "); + LOG.warn("Configuration of {} is invalid: Value is {} but must be at least 1. Using 1 as a fallback.", + TxConstants.Service.CFG_DATA_TX_CLIENT_COUNT, maxClients); maxClients = 1; } @@ -88,9 +86,8 @@ public class PooledClientProvider extends AbstractClientProvider { configuration.getLong(TxConstants.Service.CFG_DATA_TX_CLIENT_OBTAIN_TIMEOUT_MS, TxConstants.Service.DEFAULT_DATA_TX_CLIENT_OBTAIN_TIMEOUT_MS); if (obtainClientTimeoutMs < 0) { - LOG.warn("Configuration of " + TxConstants.Service.CFG_DATA_TX_CLIENT_COUNT + - " is invalid: value is " + obtainClientTimeoutMs + " but must be at least 0. " + - "Using 0 as a fallback. "); + LOG.warn("Configuration of {} is invalid: Value is {} but must be at least 0. Using 0 as a fallback.", + TxConstants.Service.CFG_DATA_TX_CLIENT_COUNT, obtainClientTimeoutMs); obtainClientTimeoutMs = 0; } this.clients = new TxClientPool(maxClients); @@ -109,8 +106,7 @@ public class PooledClientProvider extends AbstractClientProvider { @Override public String toString() { - return "Elastic pool of size " + this.maxClients + - ", with timeout (in milliseconds): " + this.obtainClientTimeoutMs; + return String.format("Elastic pool of size %d with timeout %d ms", maxClients, obtainClientTimeoutMs); } private TxClientPool getClientPool() { @@ -123,8 +119,7 @@ public class PooledClientProvider extends AbstractClientProvider { try { initializePool(); } catch (TException e) { - LOG.error("Failed to initialize Tx client provider", e); - throw Throwables.propagate(e); + throw new RuntimeException("Failed to initialize transaction client provider: " + this, e); } } } http://git-wip-us.apache.org/repos/asf/incubator-tephra/blob/641e915f/tephra-core/src/main/java/org/apache/tephra/distributed/SingleUseClientProvider.java ---------------------------------------------------------------------- diff --git a/tephra-core/src/main/java/org/apache/tephra/distributed/SingleUseClientProvider.java b/tephra-core/src/main/java/org/apache/tephra/distributed/SingleUseClientProvider.java index d55da5e..0cee60c 100644 --- a/tephra-core/src/main/java/org/apache/tephra/distributed/SingleUseClientProvider.java +++ b/tephra-core/src/main/java/org/apache/tephra/distributed/SingleUseClientProvider.java @@ -43,12 +43,7 @@ public class SingleUseClientProvider extends AbstractClientProvider { @Override public CloseableThriftClient getCloseableClient() throws TException, TimeoutException, InterruptedException { - try { - return new CloseableThriftClient(this, newClient(timeout)); - } catch (TException e) { - LOG.error("Unable to create new tx client: " + e.getMessage()); - throw e; - } + return new CloseableThriftClient(this, newClient(timeout)); } @Override http://git-wip-us.apache.org/repos/asf/incubator-tephra/blob/641e915f/tephra-core/src/main/java/org/apache/tephra/distributed/ThreadLocalClientProvider.java ---------------------------------------------------------------------- diff --git a/tephra-core/src/main/java/org/apache/tephra/distributed/ThreadLocalClientProvider.java b/tephra-core/src/main/java/org/apache/tephra/distributed/ThreadLocalClientProvider.java index dedaffe..a067c99 100644 --- a/tephra-core/src/main/java/org/apache/tephra/distributed/ThreadLocalClientProvider.java +++ b/tephra-core/src/main/java/org/apache/tephra/distributed/ThreadLocalClientProvider.java @@ -45,14 +45,8 @@ public class ThreadLocalClientProvider extends AbstractClientProvider { public CloseableThriftClient getCloseableClient() throws TException, TimeoutException, InterruptedException { TransactionServiceThriftClient client = this.clients.get(); if (client == null) { - try { - client = this.newClient(); - clients.set(client); - } catch (TException e) { - LOG.error("Unable to create new tx client for thread: " - + e.getMessage()); - throw e; - } + client = this.newClient(); + clients.set(client); } return new CloseableThriftClient(this, client); }
