[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. This maintains the current behavior for this implementation. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Reviewed-on: http://gerrit.cloudera.org:8080/10629 Tested-by: Impala Public Jenkins Reviewed-by: Vuk Ercegovac --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 241 insertions(+), 64 deletions(-) Approvals: Impala Public Jenkins: Verified Vuk Ercegovac: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 18 Jun 2018 05:13:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 21:37:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2686/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 18:14:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10629 to look at the new patch set (#7). Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. This maintains the current behavior for this implementation. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 241 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/7 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 10:03:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2683/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 06:31:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 14 Jun 2018 20:08:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 14 Jun 2018 03:47:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2664/ -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 14 Jun 2018 00:11:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 13 Jun 2018 16:28:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10629 to look at the new patch set (#5). Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. This maintains the current behavior for this implementation. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 241 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/5 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10629 )
Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10629/4/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/10629/4/be/src/runtime/exec-env.cc@85
PS4, Line 85: DEFINE_bool(use_local_catalog, false,
> suffixing this with "_hidden" will suppress this from help. useful while th
Done
http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@197
PS2, Line 197: new AuthorizationPo
> I see, you were trying to re-use L224. Pls add a comment: "initializes auth
Done
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java@163
PS4, Line 163: //TODO: Make the reload interval configurable.
> nit: add a space.
Done
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java@232
PS4, Line 232: public FeCatalog getCatalog() {
> nit: one line
Done
--
To view, visit http://gerrit.cloudera.org:8080/10629
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764
Gerrit-Change-Number: 10629
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 13 Jun 2018 00:14:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/10629 )
Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10629/4/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/10629/4/be/src/runtime/exec-env.cc@85
PS4, Line 85: DEFINE_bool(use_local_catalog, false,
suffixing this with "_hidden" will suppress this from help. useful while the
new functionality rolls out. for example, the flag controlling krpc was in this
state until it was made the default.
http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@197
PS2, Line 197: new AuthorizationPo
> I agree this is a little goofy, though I was trying to maintain the exact o
I see, you were trying to re-use L224. Pls add a comment: "initializes
authzChecker"
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java@163
PS4, Line 163: //TODO: Make the reload interval configurable.
nit: add a space.
http://gerrit.cloudera.org:8080/#/c/10629/4/fe/src/main/java/org/apache/impala/service/Frontend.java@232
PS4, Line 232: public FeCatalog getCatalog() {
nit: one line
--
To view, visit http://gerrit.cloudera.org:8080/10629
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764
Gerrit-Change-Number: 10629
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 12 Jun 2018 21:46:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10629 to look at the new patch set (#3). Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. This maintains the current behavior for this implementation. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 238 insertions(+), 63 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/3 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/10629/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10629/2//COMMIT_MSG@21 PS2, Line 21: except when a full catalog update is received. > just to clarify, state that this is the behavior today. Done http://gerrit.cloudera.org:8080/#/c/10629/2/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/10629/2/be/src/runtime/exec-env.cc@87 PS2, Line 87: required > nit: used it's also not required to be started. I'll clarify. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java File fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java@167 PS2, Line 167: name_.toLowerCase() > there have been a number of bugs with inconsistent case handling. just wond in the prior patch in this series I added preconditions to ensure that name_ is always lower case, so I"ll drop this. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java File fe/src/main/java/org/apache/impala/service/FeCatalogManager.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@30 PS2, Line 30: Managers > nit: Manages Done http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@30 PS2, Line 30: implementations > implementation? I assume its only one at a time. Done http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@126 PS2, Line 126: } > nit: add a newline for consistency Done http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@168 PS2, Line 168: authzChecker_ > where is this set/used? in the constructor, by the policy reader task. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@187 PS2, Line 187: /** :* C'tor used by tests to pass in a custom ImpaladCatalog. :*/ > stale comment? this looks like the core constructor so should it be public? yea, you're right, this can be private since the above two constructors are the ones meant for external consumption. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@197 PS2, Line 197: policyReaderTask.run(); > why is this run here? I would think scheduling on L204 would do this. if ne I agree this is a little goofy, though I was trying to maintain the exact old behavior. In the old code, the constructor created the AuthorizationChecker on like 187 with code that duplicated the body of 'AuthorizationPolicyReader.run'. Instead of duplicating it I figured it would be easier to just call 'run' here, which takes care of setting the authzChecker_ value. The scheduling of the task sets a randomized initial delay to reload, but I think we always need to do one load right at startup to ensure that we have the policy loaded before beginning service, right? I was also confused about why it has the check 'if authzConfig_.isEnabled()' below before scheduling the reloader task, but unconditionally does the initial load. -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 12 Jun 2018 00:25:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/10629/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10629/2//COMMIT_MSG@21 PS2, Line 21: except when a full catalog update is received. just to clarify, state that this is the behavior today. http://gerrit.cloudera.org:8080/#/c/10629/2/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/10629/2/be/src/runtime/exec-env.cc@87 PS2, Line 87: required nit: used http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java File fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java@167 PS2, Line 167: name_.toLowerCase() there have been a number of bugs with inconsistent case handling. just wondering why this tolower was used and whether we can make the choice more explicit. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java File fe/src/main/java/org/apache/impala/service/FeCatalogManager.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@30 PS2, Line 30: implementations implementation? I assume its only one at a time. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@30 PS2, Line 30: Managers nit: Manages http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java@126 PS2, Line 126: } nit: add a newline for consistency http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@168 PS2, Line 168: authzChecker_ where is this set/used? http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@187 PS2, Line 187: /** :* C'tor used by tests to pass in a custom ImpaladCatalog. :*/ stale comment? this looks like the core constructor so should it be public? looks like the standard entry point is L173 whereas for tests, we should use L182. I don't think this was clear before... the construction in JniFrontend used L174 and then used this "test" constructor. http://gerrit.cloudera.org:8080/#/c/10629/2/fe/src/main/java/org/apache/impala/service/Frontend.java@197 PS2, Line 197: policyReaderTask.run(); why is this run here? I would think scheduling on L204 would do this. if needed, perhaps it can be run in an else clause of L199. also, since authzChecker_ is not set here, something looks off. -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 11 Jun 2018 18:03:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10629 to look at the new patch set (#2). Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 232 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/2 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2614/ -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 07 Jun 2018 03:57:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10629 ) Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2614/ -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 07 Jun 2018 00:36:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7137. Support configuring Frontend to use LocalCatalog
Hello Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10629 to review the following change. Change subject: IMPALA-7137. Support configuring Frontend to use LocalCatalog .. IMPALA-7137. Support configuring Frontend to use LocalCatalog This adds a new flag -use_local_catalog which is passed through to the frontend and causes it to use LocalCatalog instead of ImpaladCatalog. Additionally, when this flag is configured, the impalad does not subscribe to catalog topic updates from the statestore. The patch is slightly more complex than simply picking which class to instantiate, because the lifecycle is designed a bit differently between the two implementations: - LocalCatalog is instantiated once per query/request. - ImpaladCatalog is instantiated once and stateful across queries, except when a full catalog update is received. In order to abstract this difference in lifecycle from the frontend, I introduced a new FeCatalogManager class with different implementations for the two lifecycles. I also had to add a simple test implementation since some tests rely on directly injecting a Catalog implementation into the Frontend. This patch also includes a few small changes to the local catalog implementation objects to enable the impalad to start and accept connections. With this patch, I was able to manually test as follows: I started just the statestore and the impalad in the new mode: - ./bin/start-statestored.sh - ./bin/start-impalad.sh --use_local_catalog I connected with impala-shell as usual and was able to run the most simple queries: - SHOW DATABASES; - USE functional; - SHOW TABLES; All other functionality results in error messages due to the various TODOs in the current skeleton implementation. Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 --- M be/src/runtime/exec-env.cc M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java A fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java 11 files changed, 232 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10629/1 -- To view, visit http://gerrit.cloudera.org:8080/10629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8c9665bd031d23608740b23eef301970af9aa764 Gerrit-Change-Number: 10629 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
