> On Jan. 11, 2018, 9:51 p.m., Andrew Sherman wrote: > > common/src/java/org/apache/hadoop/hive/common/LogUtils.java > > Lines 259 (patched) > > <https://reviews.apache.org/r/65075/diff/1/?file=1938298#file1938298line259> > > > > Does deleteAppender() call stop() internally? If so then can we delete > > the previous call to stop the subordinateAppender?
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. > 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) > > <https://reviews.apache.org/r/65075/diff/1/?file=1938299#file1938299line163> > > > > 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? 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. - kalyan kumar ----------------------------------------------------------- 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 > >