From dev-return-145593-archive-asf-public=cust-asf.ponee.io@hive.apache.org Fri Jan 12 20:40:20 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id 9BAA1180621 for ; Fri, 12 Jan 2018 20:40:20 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 8B9BE160C33; Fri, 12 Jan 2018 19:40:20 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id D2348160C20 for ; Fri, 12 Jan 2018 20:40:19 +0100 (CET) Received: (qmail 40214 invoked by uid 500); 12 Jan 2018 19:40:18 -0000 Mailing-List: contact dev-help@hive.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hive.apache.org Delivered-To: mailing list dev@hive.apache.org Received: (qmail 40203 invoked by uid 99); 12 Jan 2018 19:40:18 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 12 Jan 2018 19:40:18 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id DA387C2AD5; Fri, 12 Jan 2018 19:40:17 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.29 X-Spam-Level: ** X-Spam-Status: No, score=2.29 tagged_above=-999 required=6.31 tests=[HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_LOW=-0.7, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id bfPpDxa-yZnK; Fri, 12 Jan 2018 19:40:16 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTP id E156A5F2EE; Fri, 12 Jan 2018 19:40:15 +0000 (UTC) Received: from reviews.apache.org (unknown [10.41.0.12]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 34DC5E0373; Fri, 12 Jan 2018 19:40:15 +0000 (UTC) Received: from reviews-vm2.apache.org (localhost [IPv6:::1]) by reviews.apache.org (ASF Mail Server at reviews-vm2.apache.org) with ESMTP id DBA5FC40471; Fri, 12 Jan 2018 19:40:13 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7064365557420827920==" MIME-Version: 1.0 Subject: Re: Review Request 65075: HIVE-18426: Memory leak in RoutingAppender for every hive operation From: Andrew Sherman via Review Board To: Sergio Pena , Aihua Xu , Andrew Sherman Cc: kalyan kumar kalvagadda , hive Date: Fri, 12 Jan 2018 19:40:12 -0000 Message-ID: <20180112194012.9621.48037@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Andrew Sherman X-ReviewGroup: hive X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/65075/ X-Sender: Andrew Sherman References: <20180111215100.41418.720@reviews-vm2.apache.org> In-Reply-To: <20180111215100.41418.720@reviews-vm2.apache.org> Reply-To: Andrew Sherman X-ReviewRequest-Repository: hive-git --===============7064365557420827920== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Jan. 11, 2018, 9:51 p.m., Andrew Sherman wrote: > > common/src/java/org/apache/hadoop/hive/common/LogUtils.java > > Lines 259 (patched) > > > > > > Does deleteAppender() call stop() internally? If so then can we delete the previous call to stop the subordinateAppender? > > kalyan kumar kalvagadda wrote: > You are right. deleteAppender internallu calls stop() on the appender. we need to explicitly look-up and stop it. I will update this in my next patch. Thanks > On Jan. 11, 2018, 9:51 p.m., Andrew Sherman wrote: > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingLayout.java > > Line 162 (original), 163 (patched) > > > > > > This code was to check the case described in https://issues.apache.org/jira/browse/HIVE-17826?focusedCommentId=16208636&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16208636 > > I think this is no longer tested. Is that OK? > > kalyan kumar kalvagadda wrote: > I have couple of observations with the limited knowledge that I have. Correct me if i'm wrong here. > > 1. HIVE-17128: Code changes done as part of this jira makes sure that log4j Appender is closed when operation is closed to avoid file descriptors. > 2. HIVE-17826: Added HushableRandomAccessFileAppender which is a copy of RandomAccessFileAppender but has a explicit check in append() method to see if the appender is closed. > > I think issue reported in HIVE-17826 is seen only because the RoutingAppender is not properly cleaned-up when the operation is stopped. If we have the patch i submitted we may not see the issue reported in HIVE-17826 even with out the fix of adding HushableRandomAccessFileAppender. OK - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65075/#review195255 ----------------------------------------------------------- On Jan. 11, 2018, 2:11 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65075/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2018, 2:11 p.m.) > > > Review request for hive, Aihua Xu, Andrew Sherman, and Sergio Pena. > > > Bugs: HIVE-18426 > https://issues.apache.org/jira/browse/HIVE-18426 > > > Repository: hive-git > > > Description > ------- > > Each new operation creates new entry in the ConcurrentMap in RoutingAppender but when the operation ends, AppenderControl stored in the map is retrieved and stopped but the entry in ConcurrentMap is never cleaned up. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c72011951b6b1543352308bd51233c847fb > itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingLayout.java 8febe3e79ff892c54b696b6c6ef92f7026c46033 > > > Diff: https://reviews.apache.org/r/65075/diff/1/ > > > Testing > ------- > > Made sure that the new tests updated to verify this change are passing. > > > Thanks, > > kalyan kumar kalvagadda > > --===============7064365557420827920==--