From reviews-return-1045164-archive-asf-public=cust-asf.ponee.io@spark.apache.org Fri Feb 21 19:26:03 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id ACEB0180675 for ; Fri, 21 Feb 2020 20:26:02 +0100 (CET) Received: (qmail 15767 invoked by uid 500); 21 Feb 2020 19:26:02 -0000 Mailing-List: contact reviews-help@spark.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list reviews@spark.apache.org Received: (qmail 15751 invoked by uid 99); 21 Feb 2020 19:26:02 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 21 Feb 2020 19:26:02 +0000 From: GitBox To: reviews@spark.apache.org Subject: [GitHub] [spark] otterc commented on a change in pull request #27665: [SPARK-24355][Core][FOLLOWUP] Add flag for fetching chunks in async mode Message-ID: <158231316202.11031.16952200472328388644.gitbox@gitbox.apache.org> References: In-Reply-To: Date: Fri, 21 Feb 2020 19:26:02 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit otterc commented on a change in pull request #27665: [SPARK-24355][Core][FOLLOWUP] Add flag for fetching chunks in async mode URL: https://github.com/apache/spark/pull/27665#discussion_r382763278 ########## File path: common/network-common/src/main/java/org/apache/spark/network/server/ChunkFetchRequestHandler.java ########## @@ -124,7 +127,13 @@ private ChannelFuture respond( final Channel channel, final Encodable result) throws InterruptedException { final SocketAddress remoteAddress = channel.remoteAddress(); - return channel.writeAndFlush(result).await().addListener((ChannelFutureListener) future -> { + ChannelFuture channelFuture = null; + if (chunkFetchAsyncModeEnabled) { + channelFuture = channel.writeAndFlush(result); Review comment: When we remove `await`, we see more SASL requests timing out. I mentioned this in the comment here: https://github.com/apache/spark/pull/22173#issuecomment-578347705 Wouldn't making the `asyncMode` default, make this problem worse? In aysnc mode, how do you plan to tackle increased number of SASL failures? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org For additional commands, e-mail: reviews-help@spark.apache.org