> On July 20, 2017, 11:59 p.m., Aihua Xu wrote: > > service/src/java/org/apache/hive/service/cli/operation/Operation.java > > Lines 269 (patched) > > <https://reviews.apache.org/r/61010/diff/1/?file=1780484#file1780484line269> > > > > We only register the Operation log appender once for all the operation > > logs at the beginning. Now if you stop the appender here, then the > > following queries will not be able to output to the appender any more, > > right? > > > > Can you test your patch by: connect to HS2 from one beeline session, > > disconnect and reconnect. Then see if you still see output from the beeline > > console? > > > > Will we be able to close OutputStream instead? stopQueryAppender() > > should be called when HS2 service gets shutdown.
Nice catch Andrew! One more thing. Could you please do this for the LogDivertAppenderForTest too? It is only used for tests, but it would be good to clean up it too. Thanks, Peter - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61010/#review181083 ----------------------------------------------------------- On July 20, 2017, 10:45 p.m., Andrew Sherman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61010/ > ----------------------------------------------------------- > > (Updated July 20, 2017, 10:45 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > Previously HIVE-16061 and HIVE-16400 changed Operation Logging to use the > Log4j2 > RoutingAppender to automatically output the log for each query into an > individual operation log file. As log4j does not know when a query is > finished > it keeps the OutputStream in the Appender open even when the query completes. > The stream holds a file descriptor and so we leak file descriptors. Note that > we > are already careful to close any streams reading from the operation log file. > To fix this we use a technique described in the comments of LOG4J2-510 which > uses reflection to close the appender. The test in TestOperationLoggingLayout > is > extended to check that the Appender is closed. > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingLayout.java > 1a8337f574bb753e8c3c48a6b477b17700b05256 > ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java > e697b545984555414e27bafe92d7f22829a22687 > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 8d453d5d9153c2ec86c4adc7a68bd3b5dd249743 > > > Diff: https://reviews.apache.org/r/61010/diff/1/ > > > Testing > ------- > > Hand testing to show leak has gone. > The test in TestOperationLoggingLayout is extended to check that the Appender > is closed. > > > Thanks, > > Andrew Sherman > >
