瀏覽代碼

ZOOKEEPER-2893: very poor choice of logging if client fails to connect to server

'addr' variable is used to identify which server to connect to.
I've made this available for error handling code in order to let it fallback to this address if the remote socket hasn't been initialised yet. This will give us better error messages if the client is unable to connect to server for some reason.

Author: Andor Molnar <andor@cloudera.com>

Reviewers: phunt@apache.org

Closes #430 from anmolnar/ZOOKEEPER-2893 and squashes the following commits:

aa735540 [Andor Molnar] ZOOKEEPER-2893. Use log4j message templates
47a8cf4c [Andor Molnar] ZOOKEEPER-2893. Make serverAddress local variable of run(). Separate SocketExceptions from generic ex handler and log at info level.
6ea4cb21 [Andor Molnar] ZOOKEEPER-2893. Renamed addr to serverAddress, use serverAddress in log message, it's always populated with the correct remote endpoint
fbe4ccde [Andor Molnar] ZOOKEEPER-2893. Make 'addr' variable available for error handling code to give a chance to fallback if the socket hasn't been initialized yet

Change-Id: I22becf9c1f923a28c82f263b604239fde9bc0ce4
Andor Molnar 7 年之前
父節點
當前提交
e129e7a0b6
共有 1 個文件被更改,包括 17 次插入17 次删除
  1. 17 17
      src/java/main/org/apache/zookeeper/ClientCnxn.java

+ 17 - 17
src/java/main/org/apache/zookeeper/ClientCnxn.java

@@ -26,6 +26,7 @@ import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.SocketAddress;
+import java.net.SocketException;
 import java.nio.ByteBuffer;
 import java.util.HashSet;
 import java.util.ArrayList;
@@ -1051,7 +1052,7 @@ public class ClientCnxn {
         // throws a LoginException: see startConnect() below.
         private boolean saslLoginFailed = false;
 
-        private void startConnect() throws IOException {
+        private void startConnect(InetSocketAddress addr) throws IOException {
             // initializing it for new connection
             saslLoginFailed = false;
             if(!isFirstConnect){
@@ -1063,14 +1064,6 @@ public class ClientCnxn {
             }
             state = States.CONNECTING;
 
-            InetSocketAddress addr;
-            if (rwServerAddress != null) {
-                addr = rwServerAddress;
-                rwServerAddress = null;
-            } else {
-                addr = hostProvider.next(1000);
-            }
-
             String hostPort = addr.getHostString() + ":" + addr.getPort();
             MDC.put("myid", hostPort);
             setName(getName().replaceAll("\\(.*\\)", "(" + hostPort + ")"));
@@ -1123,6 +1116,7 @@ public class ClientCnxn {
             int to;
             long lastPingRwServer = Time.currentElapsedTime();
             final int MAX_SEND_PING_INTERVAL = 10000; //10 seconds
+            InetSocketAddress serverAddress = null;
             while (state.isAlive()) {
                 try {
                     if (!clientCnxnSocket.isConnected()) {
@@ -1130,7 +1124,13 @@ public class ClientCnxn {
                         if (closing) {
                             break;
                         }
-                        startConnect();
+                        if (rwServerAddress != null) {
+                            serverAddress = rwServerAddress;
+                            rwServerAddress = null;
+                        } else {
+                            serverAddress = hostProvider.next(1000);
+                        }
+                        startConnect(serverAddress);
                         clientCnxnSocket.updateLastSendAndHeard();
                     }
 
@@ -1231,14 +1231,14 @@ public class ClientCnxn {
                             LOG.info(e.getMessage() + RETRY_CONN_MSG);
                         } else if (e instanceof RWServerFoundException) {
                             LOG.info(e.getMessage());
+                        } else if (e instanceof SocketException) {
+                            LOG.info("Socket error occurred: {}: {}", serverAddress, e.getMessage());
                         } else {
-                            LOG.warn(
-                                    "Session 0x"
-                                            + Long.toHexString(getSessionId())
-                                            + " for server "
-                                            + clientCnxnSocket.getRemoteSocketAddress()
-                                            + ", unexpected error"
-                                            + RETRY_CONN_MSG, e);
+                            LOG.warn("Session 0x{} for server {}, unexpected error{}",
+                                            Long.toHexString(getSessionId()),
+                                            serverAddress,
+                                            RETRY_CONN_MSG,
+                                            e);
                         }
                         // At this point, there might still be new packets appended to outgoingQueue.
                         // they will be handled in next connection or cleared up if closed.