You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2021/06/09 21:27:37 UTC
[Impala-ASF-CR] [PROTOTYPE] IMPALA-10711: Allow customizing startup's filesystem check
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17570
Change subject: [PROTOTYPE] IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
[PROTOTYPE] IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Hand testing locally
TODO:
- Add tests
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
4 files changed, 44 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/17570/1
--
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: newchange
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17570/3/tests/custom_cluster/test_startup_filesystem_checks.py
File tests/custom_cluster/test_startup_filesystem_checks.py:
http://gerrit.cloudera.org:8080/#/c/17570/3/tests/custom_cluster/test_startup_filesystem_checks.py@45
PS3, Line 45: s
> flake8: E501 line too long (92 > 90 characters)
Done
--
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: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:33:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7213/ DRY_RUN=true
--
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: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:22:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 5:
Thanks for the review!
--
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: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:43:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
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)
> I brought back the minidump_path flag in case any minidumps get written.
Thanks for the explanation! I didn't realize that Impala can generate a core dump if it doesn't start up. I agree with the plan in IMPALA-10148
--
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: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:37:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8876/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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 <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:52:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8874/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:29:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Added test_startup_filesystem_checks.py custom cluster test
- Hand tested locally
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Reviewed-on: http://gerrit.cloudera.org:8080/17570
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A tests/custom_cluster/test_startup_filesystem_checks.py
5 files changed, 169 insertions(+), 6 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
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: merged
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17570
to look at the new patch set (#2).
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Added test_startup_filesystem_checks.py custom cluster test
- Hand tested locally
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A tests/custom_cluster/test_startup_filesystem_checks.py
5 files changed, 168 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/17570/2
--
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: newpatchset
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer 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: Code-Review+1
(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
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
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
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
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
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:
https://github.com/apache/impala/blob/b28da054f3595bb92873433211438306fc22fbc7/tests/custom_cluster/test_query_event_hooks.py#L86
It simply creates a tmpdir and writes the coredumps there.
--
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 <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 11:10:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17570
to look at the new patch set (#4).
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Added test_startup_filesystem_checks.py custom cluster test
- Hand tested locally
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A tests/custom_cluster/test_startup_filesystem_checks.py
5 files changed, 169 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/17570/4
--
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: newpatchset
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8875/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:42:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:18:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7221/ DRY_RUN=false
--
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: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:44:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 3: Verified+1
--
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: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 06:20:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17570
to look at the new patch set (#5).
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Added test_startup_filesystem_checks.py custom cluster test
- Hand tested locally
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A tests/custom_cluster/test_startup_filesystem_checks.py
5 files changed, 169 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/17570/5
--
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: newpatchset
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17570/3/tests/custom_cluster/test_startup_filesystem_checks.py
File tests/custom_cluster/test_startup_filesystem_checks.py:
http://gerrit.cloudera.org:8080/#/c/17570/3/tests/custom_cluster/test_startup_filesystem_checks.py@45
PS3, Line 45: s
flake8: E501 line too long (92 > 90 characters)
--
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: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 00:22:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17570
to look at the new patch set (#3).
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
IMPALA-10711: Allow customizing startup's filesystem check
At startup, Impala runs JniFrontend::checkFileSystem() to verify
that it can list the root directory of the filesystem. Some
deployments would like to restrict Impala to a subdirectory
of the filesystem. In that circumstance, Impala may not have
access to the root of the filesystem and the startup check
will fail.
This adds the startup parameter: startup_filesystem_check_directories
to specify the directories to check in JniFrontend::checkFileSystem()
at startup. For deployments that are restricted to a subdirectory,
startup_filesystem_check_directories=/some/subdirectory would
check /some/subdirectory rather than /. This parameter allows for
a list of directories in case it is useful to verify multiple
locations (such as different s3 buckets). If set to the empty string,
no directories will be checked.
Testing:
- Added test_startup_filesystem_checks.py custom cluster test
- Hand tested locally
Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A tests/custom_cluster/test_startup_filesystem_checks.py
5 files changed, 168 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/17570/3
--
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: newpatchset
Gerrit-Change-Id: I68e6802424f16002c90a37b3ae2490721525e58d
Gerrit-Change-Number: 17570
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8890/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:41:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [PROTOTYPE] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: [PROTOTYPE] IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8872/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jun 2021 21:49:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10711: Allow customizing startup's filesystem check
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17570 )
Change subject: IMPALA-10711: Allow customizing startup's filesystem check
......................................................................
Patch Set 5: Verified+1
--
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: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 11 Jun 2021 02:37:37 +0000
Gerrit-HasComments: No