hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Sherman <asher...@cloudera.com>
Subject Re: Review Request 61010: HIVE-17128 Operation Logging leaks file descriptors as the log4j Appender is never closed
Date Sat, 22 Jul 2017 00:17:00 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61010/
-----------------------------------------------------------

(Updated July 22, 2017, 12:16 a.m.)


Review request for hive.


Repository: hive-git


Description (updated)
-------

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 subordinate 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 subordinate Appender.  We use this
to close the per-query subordinate Appenders from both LogDivertAppender and
LogDivertAppenderForTest.  The test in TestOperationLoggingLayout is extended to
check that the Appenders are closed when a query completes.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/common/LogUtils.java 83f3af7440253bfbcedbc8b21d745fb71c0d7fb9

  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

  ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppenderForTest.java 465844d66b92b371f457fda0d885d75fbfce6805

  service/src/java/org/apache/hive/service/cli/operation/Operation.java 8d453d5d9153c2ec86c4adc7a68bd3b5dd249743



Diff: https://reviews.apache.org/r/61010/diff/2/

Changes: https://reviews.apache.org/r/61010/diff/1-2/


Testing
-------

Hand testing to show leak has gone.
The test in TestOperationLoggingLayout is extended to check that the Appender is closed.


Thanks,

Andrew Sherman


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message