zabetak commented on a change in pull request #3048:
URL: https://github.com/apache/hive/pull/3048#discussion_r813760610



##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/log/HushableRandomAccessFileAppender.java
##########
@@ -51,17 +47,6 @@
 public final class HushableRandomAccessFileAppender extends
     AbstractOutputStreamAppender<RandomAccessFileManager> {
 
-  private static final LoadingCache<String, String> CLOSED_FILES =

Review comment:
       The `TestOperationLoggingAPIWithTez` verifies that certain messages 
appear in the operation logs so it is a good guard against regressions. Apart 
from adding more log messages to assert in `TestOperationLoggingAPIWithTez` I 
don't have better ideas for ensuring that it will not break in the future.
   
   I guess HIVE-22753 didn't add any tests cause it was targeting a memory leak 
and these stuff are not always easy to unit test. In that case I guess it would 
require executing hundreds or thousands of queries till the problem shows up.
   
   If you have ideas on how we could improve these tests, I would be happy to 
make any changes.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to