Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@903 PS4, Line 903: BackendConfig.INSTANCE.getStartupFilesystemCheckDirectories(); > nit: +2 indentation Done http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@916 PS4, Line 916: path_string + " does not exist."; > nit: +2 indentation Done http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@920 PS4, Line 920: path_string + " is not a directory."; > nit: +2 indentation Done http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@926 PS4, Line 926: path_string + ". Error was: \n" + e.getMessage(); > nit: +2 indentation Done http://gerrit.cloudera.org:8080/#/c/17570/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@929 PS4, Line 929: ". Error was: \n" + e.getMessage(); > nit: +2 indentation Done http://gerrit.cloudera.org:8080/#/c/17570/4/tests/custom_cluster/test_startup_filesystem_checks.py File tests/custom_cluster/test_startup_filesystem_checks.py: http://gerrit.cloudera.org:8080/#/c/17570/4/tests/custom_cluster/test_startup_filesystem_checks.py@112 PS4, Line 112: # The core dumps expected to be generated by this test should be cleaned up : possible_cores = find_all_files('*core*') : post_test_cores = set([f for f in possible_cores if is_core_dump(f)]) : : for f in (post_test_cores - self.pre_test_cores): : LOG.info("Cleaned up {core} created by tests".format(core=f)) : os.remove(f) > Another similar test uses a simpler method IMO: I brought back the minidump_path flag in case any minidumps get written. Core dumps are separate and go somewhere else. test_query_event_hooks.py has an existing issue where it leaves core dumps around: IMPALA-10148 That doesn't cause any test failure, but at some point we should fix it. I based this test on authorization/test_provider.py::TestAuthorizationProvider, which had code to cleanup the core dumps. -- To view, visit http://gerrit.cloudera.org:8080/17570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d Gerrit-Change-Number: 17570 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Thu, 10 Jun 2021 20:18:59 +0000 Gerrit-HasComments: Yes
