[
https://issues.apache.org/jira/browse/SENTRY-1377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15398861#comment-15398861
]
Hadoop QA commented on SENTRY-1377:
-----------------------------------
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12820893/SENTRY-1377.003.patch
against master.
{color:red}Overall:{color} -1 due to 2 errors
{color:red}ERROR:{color} mvn test exited 1
{color:red}ERROR:{color} Failed: org.apache.sentry.hdfs.TestHMSPathsFullDump
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/1928/console
This message is automatically generated.
> improve handling of failures, both in tests and after-test cleanup, in
> TestHDFSIntegration.java
> -----------------------------------------------------------------------------------------------
>
> Key: SENTRY-1377
> URL: https://issues.apache.org/jira/browse/SENTRY-1377
> Project: Sentry
> Issue Type: Test
> Components: Sentry
> Affects Versions: 1.8.0
> Reporter: Vadim Spector
> Assignee: Vadim Spector
> Priority: Minor
> Fix For: 1.8.0
>
> Attachments: SENTRY-1377.001.patch, SENTRY-1377.002.patch,
> SENTRY-1377.003.patch
>
>
> 1. TestHDFSIntegration.java should provide best-attempt cleanup in
> cleanAfterTest() method. Currently, if any cleanup operation fails, the rest
> of cleanup code is not executed. Cleanup logic would not normally fail if
> corresponding test succeeds. But if some do not, short-circuiting some
> cleanup logic tends to cascade secondary failures which complicate
> troubleshooting.
> 2. Test methods would short-circuit closing database Connection's and
> Statement's if the test logic fails. Moreover, some test methods have
> multiple initializations of Connection ans Statement, but only one
> stmt.close(); conn.close(); pair at the end. Correct pattern for each
> distinct {Connection,Statement} pair would be:
> conn = null;
> stmt = null;
> try {
> ..... test logic here, including conn and stmt initialization ...
> } finally {
> safeClose(conn, stmt);
> }
> where
> private static safeClose(Connection conn, Statement stmt) {
> if (stmt != null) try { stmt.close(); } catch (Exception ignore) {}
> if (conn != null) try { conn.close(); } catch (Exception ignore) {}
> }
> Testing jdbc driver implementation is not in the scope of HDFS integration
> tests, so ignoring Connection.close() and Statement.close() failures should
> not be a concern for this test class.
> 3. Retry logic uses recursion in some places, as in startHiveServer2(),
> verifyQuery(), verifyOnAllSubDirs. Better to implement it via straightforward
> retry loop. Exception stack trace is more confusing than it needs to be in
> case of recursive calls. Plus, with NUM_RETRIES == 10, at least
> theoretically, it creates running out of stack as an unnecessary failure
> mechanism.
> 4. startHiveServer2() ignores hiveServer2.start() call failure, only logging
> info message.
> 5. Starting hiveServer2 and Hive metastore in separate threads and then
> keeping those threads alive seems unnecessary, since both servers' start()
> methods create servers running on their own threads anyway. It effectively
> leads to ignoring the start() method failure for both servers. Also, it
> leaves no guarantee that hiveServer2 will be started after the Hive metastore
> - both start from independent threads with no cross-thread coordination
> mechanism in place.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
