Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7895618F84 for ; Mon, 14 Mar 2016 20:00:15 +0000 (UTC) Received: (qmail 89309 invoked by uid 500); 14 Mar 2016 20:00:11 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 89126 invoked by uid 500); 14 Mar 2016 20:00:11 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 88746 invoked by uid 99); 14 Mar 2016 20:00:11 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 14 Mar 2016 20:00:11 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id F1C7CDFF8E; Mon, 14 Mar 2016 20:00:10 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: arp@apache.org To: common-commits@hadoop.apache.org Date: Mon, 14 Mar 2016 20:00:23 -0000 Message-Id: <7a9da8dd58a9452bb5868d389f85be31@git.apache.org> In-Reply-To: <2e2aa54dde944e4d991a43ef5d28bb8c@git.apache.org> References: <2e2aa54dde944e4d991a43ef5d28bb8c@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [14/20] hadoop git commit: Revert "HADOOP-12672. RPC timeout should not override IPC ping interval (iwasakims)" Revert "HADOOP-12672. RPC timeout should not override IPC ping interval (iwasakims)" This reverts commit 682adc6ba9db3bed94fd4ea3d83761db6abfe695. Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/75429969 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/75429969 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/75429969 Branch: refs/heads/HDFS-1312 Commit: 754299695b778b9b602e46836c35a3ac9474d7f8 Parents: 247a790 Author: Steve Loughran Authored: Fri Mar 11 17:00:17 2016 +0000 Committer: Steve Loughran Committed: Fri Mar 11 17:00:17 2016 +0000 ---------------------------------------------------------------------- .../main/java/org/apache/hadoop/ipc/Client.java | 33 ++++------ .../src/main/resources/core-default.xml | 9 +-- .../java/org/apache/hadoop/ipc/TestRPC.java | 68 -------------------- 3 files changed, 19 insertions(+), 91 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/75429969/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java index 3ae1d67..8d87957 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java @@ -386,7 +386,7 @@ public class Client { private Socket socket = null; // connected socket private DataInputStream in; private DataOutputStream out; - private final int rpcTimeout; + private int rpcTimeout; private int maxIdleTime; //connections will be culled if it was idle for //maxIdleTime msecs private final RetryPolicy connectionRetryPolicy; @@ -394,9 +394,8 @@ public class Client { private int maxRetriesOnSocketTimeouts; private final boolean tcpNoDelay; // if T then disable Nagle's Algorithm private final boolean tcpLowLatency; // if T then use low-delay QoS - private final boolean doPing; //do we need to send ping message - private final int pingInterval; // how often sends ping to the server - private final int soTimeout; // used by ipc ping and rpc timeout + private boolean doPing; //do we need to send ping message + private int pingInterval; // how often sends ping to the server in msecs private ByteArrayOutputStream pingRequest; // ping message // currently active calls @@ -435,9 +434,6 @@ public class Client { pingHeader.writeDelimitedTo(pingRequest); } this.pingInterval = remoteId.getPingInterval(); - this.soTimeout = - (rpcTimeout == 0 || (doPing && pingInterval < rpcTimeout))? - this.pingInterval : this.rpcTimeout; this.serviceClass = serviceClass; if (LOG.isDebugEnabled()) { LOG.debug("The ping interval is " + this.pingInterval + " ms."); @@ -488,12 +484,12 @@ public class Client { /* Process timeout exception * if the connection is not going to be closed or - * the RPC is not timed out yet, send a ping. + * is not configured to have a RPC timeout, send a ping. + * (if rpcTimeout is not set to be 0, then RPC should timeout. + * otherwise, throw the timeout exception. */ - private void handleTimeout(SocketTimeoutException e, int waiting) - throws IOException { - if (shouldCloseConnection.get() || !running.get() || - (0 < rpcTimeout && rpcTimeout <= waiting)) { + private void handleTimeout(SocketTimeoutException e) throws IOException { + if (shouldCloseConnection.get() || !running.get() || rpcTimeout > 0) { throw e; } else { sendPing(); @@ -507,13 +503,11 @@ public class Client { */ @Override public int read() throws IOException { - int waiting = 0; do { try { return super.read(); } catch (SocketTimeoutException e) { - waiting += soTimeout; - handleTimeout(e, waiting); + handleTimeout(e); } } while (true); } @@ -526,13 +520,11 @@ public class Client { */ @Override public int read(byte[] buf, int off, int len) throws IOException { - int waiting = 0; do { try { return super.read(buf, off, len); } catch (SocketTimeoutException e) { - waiting += soTimeout; - handleTimeout(e, waiting); + handleTimeout(e); } } while (true); } @@ -640,7 +632,10 @@ public class Client { } NetUtils.connect(this.socket, server, connectionTimeout); - this.socket.setSoTimeout(soTimeout); + if (rpcTimeout > 0) { + pingInterval = rpcTimeout; // rpcTimeout overwrites pingInterval + } + this.socket.setSoTimeout(pingInterval); return; } catch (ConnectTimeoutException toe) { /* Check for an address change and update the local reference. http://git-wip-us.apache.org/repos/asf/hadoop/blob/75429969/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml index 5037113..187f923 100644 --- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml +++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml @@ -1054,7 +1054,7 @@ true Send a ping to the server when timeout on reading the response, if set to true. If no failure is detected, the client retries until at least - a byte is read or the time given by ipc.client.rpc-timeout.ms is passed. + a byte is read. @@ -1071,9 +1071,10 @@ ipc.client.rpc-timeout.ms 0 Timeout on waiting response from server, in milliseconds. - If ipc.client.ping is set to true and this rpc-timeout is greater than - the value of ipc.ping.interval, the effective value of the rpc-timeout is - rounded up to multiple of ipc.ping.interval. + Currently this timeout works only when ipc.client.ping is set to true + because it uses the same facilities with IPC ping. + The timeout overrides the ipc.ping.interval and client will throw exception + instead of sending ping when the interval is passed. http://git-wip-us.apache.org/repos/asf/hadoop/blob/75429969/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java index 929b82b..99bfc61 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java @@ -1043,74 +1043,6 @@ public class TestRPC extends TestRpcBase { } } - /** - * Test RPC timeout when ipc.client.ping is false. - */ - @Test(timeout=30000) - public void testClientRpcTimeoutWithoutPing() throws Exception { - final Server server = new RPC.Builder(conf) - .setProtocol(TestProtocol.class).setInstance(new TestImpl()) - .setBindAddress(ADDRESS).setPort(0) - .setQueueSizePerHandler(1).setNumHandlers(1).setVerbose(true) - .build(); - server.start(); - - final Configuration conf = new Configuration(); - conf.setBoolean(CommonConfigurationKeys.IPC_CLIENT_PING_KEY, false); - conf.setInt(CommonConfigurationKeys.IPC_CLIENT_RPC_TIMEOUT_KEY, 1000); - final TestProtocol proxy = - RPC.getProxy(TestProtocol.class, TestProtocol.versionID, - NetUtils.getConnectAddress(server), conf); - - try { - proxy.sleep(3000); - fail("RPC should time out."); - } catch (SocketTimeoutException e) { - LOG.info("got expected timeout.", e); - } finally { - server.stop(); - RPC.stopProxy(proxy); - } - } - - /** - * Test RPC timeout greater than ipc.ping.interval. - */ - @Test(timeout=30000) - public void testClientRpcTimeoutGreaterThanPingInterval() throws Exception { - final Server server = new RPC.Builder(conf) - .setProtocol(TestProtocol.class).setInstance(new TestImpl()) - .setBindAddress(ADDRESS).setPort(0) - .setQueueSizePerHandler(1).setNumHandlers(1).setVerbose(true) - .build(); - server.start(); - - final Configuration conf = new Configuration(); - conf.setBoolean(CommonConfigurationKeys.IPC_CLIENT_PING_KEY, true); - conf.setInt(CommonConfigurationKeys.IPC_PING_INTERVAL_KEY, 800); - conf.setInt(CommonConfigurationKeys.IPC_CLIENT_RPC_TIMEOUT_KEY, 1000); - final TestProtocol proxy = - RPC.getProxy(TestProtocol.class, TestProtocol.versionID, - NetUtils.getConnectAddress(server), conf); - - // should not time out. - proxy.sleep(300); - - // should not time out because effective rpc-timeout is - // multiple of ping interval: 1600 (= 800 * (1000 / 800 + 1)) - proxy.sleep(1300); - - try { - proxy.sleep(2000); - fail("RPC should time out."); - } catch (SocketTimeoutException e) { - LOG.info("got expected timeout.", e); - } finally { - server.stop(); - RPC.stopProxy(proxy); - } - } - public static void main(String[] args) throws Exception { new TestRPC().testCallsInternal(conf); }