From f8103c69eaaa23fe7fff4d8413240063b9ec56d2 Mon Sep 17 00:00:00 2001 From: David Mollitor Date: Wed, 20 May 2020 09:26:31 -0400 Subject: [PATCH] THRIFT-5202: TNonblockingMultiFetchClient Use SLF4J Parameterized Logging Client: java Patch: David Mollitor This closes #2137 Use SLF4J API to log full Exception details. Use SLF4J parameterized logging instead of String format. --- .../thrift/TNonblockingMultiFetchClient.java | 58 +++++++------------ 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/lib/java/src/org/apache/thrift/TNonblockingMultiFetchClient.java b/lib/java/src/org/apache/thrift/TNonblockingMultiFetchClient.java index 382d978db..9cf873cac 100644 --- a/lib/java/src/org/apache/thrift/TNonblockingMultiFetchClient.java +++ b/lib/java/src/org/apache/thrift/TNonblockingMultiFetchClient.java @@ -77,8 +77,7 @@ import java.util.concurrent.TimeoutException; public class TNonblockingMultiFetchClient { private static final Logger LOGGER = LoggerFactory.getLogger( - TNonblockingMultiFetchClient.class.getName() - ); + TNonblockingMultiFetchClient.class); // if the size of the response msg exceeds this limit (in byte), we will // not read the msg @@ -163,18 +162,18 @@ public class TNonblockingMultiFetchClient { executor.execute(task); try { task.get(fetchTimeoutSeconds, TimeUnit.SECONDS); - } catch(InterruptedException ie) { + } catch (InterruptedException ie) { // attempt to cancel execution of the task. task.cancel(true); - LOGGER.error("interrupted during fetch: "+ie.toString()); - } catch(ExecutionException ee) { + LOGGER.error("Interrupted during fetch", ie); + } catch (ExecutionException ee) { // attempt to cancel execution of the task. task.cancel(true); - LOGGER.error("exception during fetch: "+ee.toString()); - } catch(TimeoutException te) { + LOGGER.error("Exception during fetch", ee); + } catch (TimeoutException te) { // attempt to cancel execution of the task. task.cancel(true); - LOGGER.error("timeout for fetch: "+te.toString()); + LOGGER.error("Timeout for fetch", te); } executor.shutdownNow(); @@ -215,8 +214,8 @@ public class TNonblockingMultiFetchClient { try { selector = Selector.open(); - } catch (IOException e) { - LOGGER.error("selector opens error: "+e.toString()); + } catch (IOException ioe) { + LOGGER.error("Selector opens error", ioe); return; } @@ -239,10 +238,8 @@ public class TNonblockingMultiFetchClient { // attach index of the key key.attach(i); } catch (Exception e) { - stats.incNumConnectErrorServers(); - String err = String.format("set up socket to server %s error: %s", - server.toString(), e.toString()); - LOGGER.error(err); + stats.incNumConnectErrorServers(); + LOGGER.error("Set up socket to server {} error", server, e); // free resource if (s != null) { try {s.close();} catch (Exception ex) {} @@ -264,7 +261,7 @@ public class TNonblockingMultiFetchClient { try{ selector.select(); } catch (Exception e) { - LOGGER.error("selector selects error: "+e.toString()); + LOGGER.error("Selector selects error", e); continue; } @@ -284,10 +281,7 @@ public class TNonblockingMultiFetchClient { sChannel.finishConnect(); } catch (Exception e) { stats.incNumConnectErrorServers(); - String err = String.format("socket %d connects to server %s " + - "error: %s", - index, servers.get(index).toString(), e.toString()); - LOGGER.error(err); + LOGGER.error("Socket {} connects to server {} error", index, servers.get(index), e); } } @@ -299,10 +293,7 @@ public class TNonblockingMultiFetchClient { SocketChannel sChannel = (SocketChannel)selKey.channel(); sChannel.write(sendBuf[index]); } catch (Exception e) { - String err = String.format("socket %d writes to server %s " + - "error: %s", - index, servers.get(index).toString(), e.toString()); - LOGGER.error(err); + LOGGER.error("Socket {} writes to server {} error", index, servers.get(index), e); } } } @@ -325,10 +316,8 @@ public class TNonblockingMultiFetchClient { if (frameSize[index] <= 0) { stats.incNumInvalidFrameSize(); - String err = String.format("Read an invalid frame size %d" - + " from %s. Does the server use TFramedTransport? ", - frameSize[index], servers.get(index).toString()); - LOGGER.error(err); + LOGGER.error("Read an invalid frame size {} from {}. Does the server use TFramedTransport?", + frameSize[index], servers.get(index)); sChannel.close(); continue; } @@ -339,11 +328,8 @@ public class TNonblockingMultiFetchClient { if (frameSize[index] + 4 > maxRecvBufBytesPerServer) { stats.incNumOverflowedRecvBuf(); - String err = String.format("Read frame size %d from %s," - + " total buffer size would exceed limit %d", - frameSize[index], servers.get(index).toString(), - maxRecvBufBytesPerServer); - LOGGER.error(err); + LOGGER.error("Read frame size {} from {}, total buffer size would exceed limit {}", + frameSize[index], servers.get(index), maxRecvBufBytesPerServer); sChannel.close(); continue; } @@ -366,10 +352,8 @@ public class TNonblockingMultiFetchClient { } } } catch (Exception e) { - String err = String.format("socket %d reads from server %s " + - "error: %s", - index, servers.get(index).toString(), e.toString()); - LOGGER.error(err); + LOGGER.error("Socket {} reads from server {} error", + index, servers.get(index), e); } } } @@ -392,7 +376,7 @@ public class TNonblockingMultiFetchClient { selector.close(); } } catch (IOException e) { - LOGGER.error("free resource error: "+e.toString()); + LOGGER.error("Free resource error", e); } } }