[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change introduces a TBackendGflags class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Reviewed-on: http://gerrit.cloudera.org:8080/4867 Reviewed-by: Bharath Vissapragada Tested-by: Internal Jenkins --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/util/CMakeLists.txt A be/src/util/backend-gflag-util.cc A be/src/util/backend-gflag-util.h A common/thrift/BackendGflags.thrift M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 24 files changed, 280 insertions(+), 199 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Internal Jenkins has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 12: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 12: Code-Review+2 Carrying +2. Attempting to GVO. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 11: Code-Review+2 +2 for FE. You can carry Dan's +2. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Hello Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4867 to look at the new patch set (#11). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change introduces a TBackendGflags class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/util/CMakeLists.txt A be/src/util/backend-gflag-util.cc A be/src/util/backend-gflag-util.h A common/thrift/BackendGflags.thrift M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 24 files changed, 280 insertions(+), 199 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/11 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 10: (5 comments) http://gerrit.cloudera.org:8080/#/c/4867/10//COMMIT_MSG Commit Message: PS10, Line 21: amends the TBackendConfig > introduces a TBackendGflags Done http://gerrit.cloudera.org:8080/#/c/4867/10/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: Line 76: } > it's too bad we can't autogenerate this file (and the thrift structure) so Yep, we should probably do some kind of scripting work for that. May we can do it as a follow up patch sometime. http://gerrit.cloudera.org:8080/#/c/4867/10/common/thrift/BackendGflags.thrift File common/thrift/BackendGflags.thrift: PS10, Line 22: Attributes without comments correspond to gflags > I'd prefer them to be 1:1 with gflags. Bharath, do you think that's a doabl Made them 1:1. PS10, Line 22: Attributes without comments correspond to gflags > If these aren't 1:1 with gflags, let's call the struct TBackendFlags. Made them 1:1 as per Alex's comment. http://gerrit.cloudera.org:8080/#/c/4867/10/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: PS10, Line 42: Preconditions.checkState(INSTANCE == null); @Alex, I removed this Preconditions check since the 'be' tests instantiate multiple Frontend classes and this check doesn't hold true after the first initializaion. We could ideally do something like Preconditions.checkState(isTestEnv() || INSTANCE == null) but I think its fine even if we overwrite it the second time. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/4867/10/common/thrift/BackendGflags.thrift File common/thrift/BackendGflags.thrift: PS10, Line 22: Attributes without comments correspond to gflags > If these aren't 1:1 with gflags, let's call the struct TBackendFlags. I'd prefer them to be 1:1 with gflags. Bharath, do you think that's a doable change? -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Henry Robinson has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/4867/10/common/thrift/BackendGflags.thrift File common/thrift/BackendGflags.thrift: PS10, Line 22: Attributes without comments correspond to gflags If these aren't 1:1 with gflags, let's call the struct TBackendFlags. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Dan Hecht has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 10: Code-Review+2 (2 comments) Please hold off on committing until the blocker broken builds are fixed though. http://gerrit.cloudera.org:8080/#/c/4867/10//COMMIT_MSG Commit Message: PS10, Line 21: amends the TBackendConfig introduces a TBackendGflags http://gerrit.cloudera.org:8080/#/c/4867/10/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: Line 76: } it's too bad we can't autogenerate this file (and the thrift structure) so that new options are available to the FE automatically, but this is okay for now and definitely and improvement. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 10: Code-Review+1 Carrying +1 from Alex. Dan, can you please do a +2 review for be. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Hello Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4867 to look at the new patch set (#10). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change amends the TBackendConfig class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/util/CMakeLists.txt A be/src/util/backend-gflag-util.cc A be/src/util/backend-gflag-util.h A common/thrift/BackendGflags.thrift M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 24 files changed, 278 insertions(+), 199 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/10 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4867/9/be/src/service/frontend.cc
File be/src/service/frontend.cc:
Line 32: DEFINE_bool(load_catalog_at_startup, false, "if true, load all catalog
data at startup");
> move to catalog.cc
Actually this is used no where else. Removed.
http://gerrit.cloudera.org:8080/#/c/4867/9/be/src/util/backend-gflag-util.cc
File be/src/util/backend-gflag-util.cc:
Line 50: Status GetThriftBackendConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) {
> GetThriftBackendGflags()
Done
Line 51: TBackendStartupParams cfg;
> TBackendGflags
Done
http://gerrit.cloudera.org:8080/#/c/4867/9/common/thrift/BackendConfig.thrift
File common/thrift/BackendConfig.thrift:
Line 1: // Licensed to the Apache Software Foundation (ASF) under one
> BackendGflags.thrift
Done
http://gerrit.cloudera.org:8080/#/c/4867/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 53: // Inits the auth_to_local configuration in static KerberosName
class.
> the static KerberosName
Done
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 9: Code-Review+1
(5 comments)
+2 for FE, +1 for BE after the minor renames
http://gerrit.cloudera.org:8080/#/c/4867/9/be/src/service/frontend.cc
File be/src/service/frontend.cc:
Line 32: DEFINE_bool(load_catalog_at_startup, false, "if true, load all catalog
data at startup");
move to catalog.cc
http://gerrit.cloudera.org:8080/#/c/4867/9/be/src/util/backend-gflag-util.cc
File be/src/util/backend-gflag-util.cc:
Line 50: Status GetThriftBackendConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) {
GetThriftBackendGflags()
Line 51: TBackendStartupParams cfg;
TBackendGflags
http://gerrit.cloudera.org:8080/#/c/4867/9/common/thrift/BackendConfig.thrift
File common/thrift/BackendConfig.thrift:
Line 1: // Licensed to the Apache Software Foundation (ASF) under one
BackendGflags.thrift
http://gerrit.cloudera.org:8080/#/c/4867/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 53: // Inits the auth_to_local configuration in static KerberosName
class.
the static KerberosName
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has uploaded a new patch set (#9). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change amends the TBackendConfig class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/util/CMakeLists.txt A be/src/util/backend-gflag-util.cc A be/src/util/backend-gflag-util.h A common/thrift/BackendConfig.thrift M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 23 files changed, 278 insertions(+), 197 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/9 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4867/8/be/src/util/backend-config-util.cc
File be/src/util/backend-config-util.cc:
Line 38: DEFINE_bool(load_catalog_at_startup, false, "if true, load all catalog
data at startup");
> Where are these DEFINEs currently? Shouldn't these all be DECLARE, otherwis
Oops. I moved the DEFINEs too by mistake. I'm moving them to their respective
components (frontend/catalog) and just have DECLAREs here.
http://gerrit.cloudera.org:8080/#/c/4867/8/be/src/util/backend-config-util.h
File be/src/util/backend-config-util.h:
Line 27: class BackendConfigUtil {
> no need for a class, just use a function
Done. Also renamed this to backend-gflag-util.cc as we have another
./be/src/scheduling/backend-config.cc.
http://gerrit.cloudera.org:8080/#/c/4867/8/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 43: public long getReadSize() {
> single line where it fits
Done
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4867/8/be/src/util/backend-config-util.cc
File be/src/util/backend-config-util.cc:
Line 38: DEFINE_bool(load_catalog_at_startup, false, "if true, load all catalog
data at startup");
Where are these DEFINEs currently? Shouldn't these all be DECLARE, otherwise
we'll have two definitions.
http://gerrit.cloudera.org:8080/#/c/4867/8/be/src/util/backend-config-util.h
File be/src/util/backend-config-util.h:
Line 27: class BackendConfigUtil {
no need for a class, just use a function
http://gerrit.cloudera.org:8080/#/c/4867/8/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 43: public long getReadSize() {
single line where it fits
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has uploaded a new patch set (#8). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change amends the TBackendConfig class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/util/CMakeLists.txt A be/src/util/backend-config-util.cc A be/src/util/backend-config-util.h M common/thrift/Frontend.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 22 files changed, 270 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/8 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 151: Status Catalog::GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const {
> Or maybe even add a new file for this like backend-config.h in util.
Tried adding to ExecEnv but its polluted with all the DECLAREs from gflags. So
I moved them to a separate util file. Please check the next PS.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 151: Status Catalog::GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const {
> Makes sense, but since impalad/catalogd are the same binary they can see al
Or maybe even add a new file for this like backend-config.h in util.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 151: Status Catalog::GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const {
> It was created this way since Fe and Catalog can have their own set of conf
Makes sense, but since impalad/catalogd are the same binary they can see all
gflags, even if some of them only make sense for one or the other. In the FE we
only have a single BackendConfig, which makes sense to me.
Maybe we can make this a static function in ExecEnv()?
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 151: Status Catalog::GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const {
> Can we refactor the code to only have one GetBackendConfigBytes() or are th
It was created this way since Fe and Catalog can have their own set of configs.
For example --load_catalog_in_background is specific to Catalog. Maintaining a
GetBackendConfigBytes() sounds like a good idea but its unclear where to put
that. May be a static method in a util class somewhere where we DEFINE all the
gflags?
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:
Line 106: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const;
> GetBackendConfigBytes()?
Same comment as in frontend.h.
http://gerrit.cloudera.org:8080/#/c/4867/5/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 249: 6: optional bool compute_lineage
> Let's continue down this path for now and see how it goes.
Sure.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 151: Status Catalog::GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const {
Can we refactor the code to only have one GetBackendConfigBytes() or are there
any differences between this one and the one used for the FE?
http://gerrit.cloudera.org:8080/#/c/4867/7/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:
Line 106: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const;
GetBackendConfigBytes()?
http://gerrit.cloudera.org:8080/#/c/4867/5/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 249: 6: optional bool compute_lineage
> Isn't it better if we let the backend drive the decision here and just pass
Let's continue down this path for now and see how it goes.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has uploaded a new patch set (#7). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change amends the TBackendConfig class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 21 files changed, 195 insertions(+), 163 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/7 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has uploaded a new patch set (#6). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change amends the TBackendConfig class to pass the gflags from backend to the Frontend and the Catalog via thrift. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 21 files changed, 196 insertions(+), 160 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/6 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 5:
(15 comments)
I've taken over this. Please check the next PS.
http://gerrit.cloudera.org:8080/#/c/4867/5//COMMIT_MSG
Commit Message:
PS5, Line 21: set of
: gflags, to frontend,
> nit: replace with "to the"
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 150: const {
> move into line above (90 chars per line max)
Done
Line 152: cfg.load_catalog_in_background = FLAGS_load_catalog_in_background;
> use the thrift setter functions
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:
Line 104: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const;
> Document a line or two on what this does.
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/service/frontend.h
File be/src/service/frontend.h:
Line 176: Status GetFrontendConfigBytes(JNIEnv* jni_env, jbyteArray*
cfg_bytes) const;
> Document.
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 236: 1: required string sentry_config
> Why are some of these required and some optional? I couldn't make out a pat
Made everything optional.
Line 242: 3: required i32 other_log_lvl
> non_impala_java_vlog
Done
Line 246: 5: required i64 inc_stats_size_limit_bytes
> your gflag is a uint64, I suggest making the gflag signed as well
Done
Line 249: 6: required bool compute_lineage
> if possible, I'd prefer to pass the gflags directly, i.e., instead of a boo
Isn't it better if we let the backend drive the decision here and just pass the
flags the fe? I'm fine either way but if you feel strongly about this, I'll
make the change. Please let me know.
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
Line 1601: (statsSizeEstimate <
BackendConfig.INSTANCE.getIncStatMaxSize());
> getIncStatsMaxSize() (Stats vs. Stat)
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
Line 30: public class RuntimeEnv {
> much better!
:)
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 41: }
> How about adding initializing the singleton this way:
Done
Line 43: public void setBackendConfig(TBackendConfig cfg) {
> indentation
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
Line 83: public JniCatalog(byte[] thriftBEConfig) throws InternalException,
> thriftBackendConfig or thriftBeConfig
Done
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
Line 120: public JniFrontend(byte[] thriftBEConfig) throws InternalException,
> remove InternalException because that's already covered by ImpalaException
Done
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Alex Behm has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 5:
(12 comments)
Nice cleanup
http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
Line 150: const {
move into line above (90 chars per line max)
Line 152: cfg.load_catalog_in_background = FLAGS_load_catalog_in_background;
use the thrift setter functions
http://gerrit.cloudera.org:8080/#/c/4867/5/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 236: 1: required string sentry_config
Why are some of these required and some optional? I couldn't make out a pattern.
Line 242: 3: required i32 other_log_lvl
non_impala_java_vlog
Line 246: 5: required i64 inc_stats_size_limit_bytes
your gflag is a uint64, I suggest making the gflag signed as well
Line 249: 6: required bool compute_lineage
if possible, I'd prefer to pass the gflags directly, i.e., instead of a bool
here, pass the lineage_event_log_dir, and then the FE BackendConfig can
implement a function computeLineage() based on that
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
Line 1601: (statsSizeEstimate <
BackendConfig.INSTANCE.getIncStatMaxSize());
getIncStatsMaxSize() (Stats vs. Stat)
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
Line 30: public class RuntimeEnv {
much better!
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 41: }
How about adding initializing the singleton this way:
public static void create(TBackendConfig cfg) {
Preconditions.checkNotNull(cfg);
Preconditions.checkState(INSTANCE == null);
INSTANCE = new TBackendConfig(cfg);
}
Line 43: public void setBackendConfig(TBackendConfig cfg) {
indentation
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
Line 83: public JniCatalog(byte[] thriftBEConfig) throws InternalException,
thriftBackendConfig or thriftBeConfig
http://gerrit.cloudera.org:8080/#/c/4867/5/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
Line 120: public JniFrontend(byte[] thriftBEConfig) throws InternalException,
remove InternalException because that's already covered by ImpalaException
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 5:
(1 comment)
agreed w/ bharath. Types.thrift is not good place for tbackendconfig. please
propose other place. unless, let's live w/ it for now.
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS2, Line 73: ABORT_IF_ERROR(GetCatalogConfigBytes(jni_env, &cfg_bytes));
:
: jobject catalog = jni_env->NewObject(catalog_class_,
catalog_ctor_, cfg_bytes);
: EXIT_IF_EXC(jni_env);
: ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, catalog,
&catalog_));
: }
:
: Status Catalog::GetCatalogObject(const TCatalogObject& req,
: TCatalogObject* resp) {
: return JniUtil::CallJniMethod(catalog_, get_cata
> Can you factor all this into a method, GetBackendConfig() or something simi
Done
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 5: Alex, I think we need to find a better place to add TBackendConfig. Right now it is in Types.thrift which doesn't seem to be the ideal one. Any suggestions for that? -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. Patch Set 5: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/4867/5//COMMIT_MSG Commit Message: PS5, Line 21: set of : gflags, to frontend, nit: replace with "to the" http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/catalog/catalog.h File be/src/catalog/catalog.h: Line 104: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray* cfg_bytes) const; Document a line or two on what this does. http://gerrit.cloudera.org:8080/#/c/4867/5/be/src/service/frontend.h File be/src/service/frontend.h: Line 176: Status GetFrontendConfigBytes(JNIEnv* jni_env, jbyteArray* cfg_bytes) const; Document. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/4867/4//COMMIT_MSG
Commit Message:
PS4, Line 21: , set of
: gflags, to frontend, JniCatalog and JniFrontend
> to Catalog and Frontend.
Done
PS4, Line 23: entertain
> use?
Done
http://gerrit.cloudera.org:8080/#/c/4867/4/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:
Line 104: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray&
cfg_bytes) const;
> nit: We generally use the return objects to be pointers and pass others by
Done
http://gerrit.cloudera.org:8080/#/c/4867/4/be/src/service/frontend.h
File be/src/service/frontend.h:
Line 176: Status GetFrontendConfigBytes(JNIEnv* jni_env, jbyteArray&
cfg_bytes) const;
> nit: same as the comment in catalog.h
Done
http://gerrit.cloudera.org:8080/#/c/4867/4/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
Line 39: private boolean computeLineage_;
> Remove.
Done
PS4, Line 46: computeLineage_ = BackendConfig.INSTANCE.getComputeLineage();
> Remove. Update the callers to use BackendConfig.
Done
Line 60: public boolean computeLineage() { return computeLineage_; }
> Remove.
Done
http://gerrit.cloudera.org:8080/#/c/4867/4/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 38: private boolean allowAuthToLocalRules_ = false;
> Remove.
Done
Line 45: private long incStatMaxSize_ = 0;
> Directly have a reference to TBackendConfig object here.
Done
PS4, Line 63: public long getIncStatMaxSize() { return incStatMaxSize_; }
: public boolean isAuthToLocalEnabled() { return
allowAuthToLocalRule
> return tBackendConfigObj. directly. No need to have separate variab
Done
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new patch set (#5). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change includes TBackendConfig to pass backend configs, set of gflags, to frontend, Catalog and Frontend. This also revamps existing query options to use the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 19 files changed, 155 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/5 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 4:
(10 comments)
Looks pretty good. Just a bunch of small changes. I can give a +1 once those
are done.
http://gerrit.cloudera.org:8080/#/c/4867/4//COMMIT_MSG
Commit Message:
PS4, Line 21: , set of
: gflags, to frontend, JniCatalog and JniFrontend
to Catalog and Frontend.
PS4, Line 23: entertain
use?
http://gerrit.cloudera.org:8080/#/c/4867/4/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:
Line 104: Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray&
cfg_bytes) const;
nit: We generally use the return objects to be pointers and pass others by
reference (see other methods above), something like
Status GetCatalogConfigBytes(JNIEnv* jni_env, jbyteArray* cfg_bytes) const;
http://gerrit.cloudera.org:8080/#/c/4867/4/be/src/service/frontend.h
File be/src/service/frontend.h:
Line 176: Status GetFrontendConfigBytes(JNIEnv* jni_env, jbyteArray&
cfg_bytes) const;
nit: same as the comment in catalog.h
http://gerrit.cloudera.org:8080/#/c/4867/4/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
Line 39: private boolean computeLineage_;
Remove.
PS4, Line 46: computeLineage_ = BackendConfig.INSTANCE.getComputeLineage();
Remove. Update the callers to use BackendConfig.
Line 60: public boolean computeLineage() { return computeLineage_; }
Remove.
http://gerrit.cloudera.org:8080/#/c/4867/4/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
Line 38: private boolean allowAuthToLocalRules_ = false;
Remove.
Line 45: private long incStatMaxSize_ = 0;
Directly have a reference to TBackendConfig object here.
PS4, Line 63: public long getIncStatMaxSize() { return incStatMaxSize_; }
: public boolean isAuthToLocalEnabled() { return
allowAuthToLocalRule
return tBackendConfigObj. directly. No need to have separate variables
for each config.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new patch set (#4). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change includes TBackendConfig to pass backend configs, set of gflags, to frontend, JniCatalog and JniFrontend. This also revamps existing query options to entertain the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/service/fe-support.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/authorization/User.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 16 files changed, 163 insertions(+), 120 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/4 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change.
Change subject: IMPALA-3552: Make incremental stats max serialized size
configurable
..
Patch Set 2:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/4867/2//COMMIT_MSG
Commit Message:
PS2, Line 7: make
> nit: Make
Done
PS2, Line 9: fox
> typo
Done
PS2, Line 11: are experiencing
: regressions especially when upgrading.
> Please expand a little on what the regression is.
Done
Line 19: The change also includes TBackendConfig to pass backend config
> Please add a note on revamp of existing query options to use the new TBacke
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS2, Line 73: cfg.load_catalog_in_background =
FLAGS_load_catalog_in_background;
: cfg.num_metadata_loading_threads =
FLAGS_num_metadata_loading_threads;
: cfg.sentry_config.assign(FLAGS_sentry_config);
: // auth_to_local rules are read if --load_auth_to_local_rules
is set to true
: // and impala is kerberized.
: cfg.auth_to_local = FLAGS_load_auth_to_local_rules &&
!FLAGS_principal.empty();
: cfg.principal.assign(FLAGS_principal);
: cfg.impala_log_lvl = FlagToTLogLevel(FLAGS_v);
: cfg.other_log_lvl = FlagToTLogLevel(FLAGS_non_impala_java_vlog);
: cfg.inc_stat_max_size = FLAGS_inc_stat_max_size;
> Can you factor all this into a method, GetBackendConfig() or something simi
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
Line 119: DEFINE_uint64(inc_stat_max_size, 200 * (1LL<<20), "Maximum size (in
bytes) of incremental "
> long line.
Done
PS2, Line 119: inc_stat_max_size
> Rename to incremental_stats_size_limit_bytes or something? That way its mor
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:
PS2, Line 104: TBackendConfig cfg;
:
cfg.authorization_policy_file.assign(FLAGS_authorization_policy_file);
: cfg.server_name.assign(FLAGS_server_name);
: cfg.sentry_config.assign(FLAGS_sentry_config);
: // auth_to_local rules are read if --load_auth_to_local_rules
is set to true
: // and impala is kerberized.
: cfg.auth_to_local = FLAGS_load_auth_to_local_rules &&
!FLAGS_principal.empty();
:
cfg.authorization_policy_provider_class.assign(FLAGS_authorization_policy_provider_class);
: cfg.kudu_master_hosts.assign(FLAGS_kudu_master_hosts);
: cfg.impala_log_lvl = FlagToTLogLevel(FLAGS_v);
: cfg.other_log_lvl =
FlagToTLogLevel(FLAGS_non_impala_java_vlog);
: cfg.inc_stat_max_size = FLAGS_inc_stat_max_size;
:
: jbyteArray cfg_bytes;
: JniLocalFrame jni_frame;
: ABORT_IF_ERROR(jni_frame.push(jni_env));
: ABORT_IF_ERROR(SerializeThriftMsg(jni_env, &cfg, &cfg_bytes));
> Same as catalog.cc, please factor this into a method.
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 233: struct TBackendConfig {
> Please document the usage of this thrift struct and how it is shared betwee
Done
PS2, Line 236: required
> Some of these are only used by Catalog and not fe (and vice versa). Shouldn
Done
Line 245: 12: required string kudu_master_hosts
> While you are at this, can you merge TStartupOptions (Frontend.thrift) with
Let's do this as a separate commit
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
PS2, Line 288: incStatMaxSize in bytes
> --incremental_stats_size_limit_bytes
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
PS2, Line 1591: MAX_INCREMENTAL_STATS_SIZE_BYTES
> update
Done
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
PS2, Line 33:
: // This is overriden by JniFrontend/JniCatalog classes with
user set configuration.
: // TODO: Read this from backend instead of using static
variables.
: private static boolean allowAuthToLocalRules_ = false;
:
: // Maximum size (in bytes) of incremental stats the catalog is
: // allowed to serialize per table. This limit is set as a safety
: // check, to prevent the JVM from hitting a maximum array limit
of
: // 1GB (or OOM) while building the thrift objects
[Impala-ASF-CR] IMPALA-3552: Make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new patch set (#3). Change subject: IMPALA-3552: Make incremental stats max serialized size configurable .. IMPALA-3552: Make incremental stats max serialized size configurable The fix "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when they upgrade impala and the serialized size goes beyond 200MB. To mitigate the issue, the change introduces a new gflag, 'inc_stats_size_limit_bytes' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stats_size_limit_bytes is 200MB. The change includes TBackendConfig to pass backend configs, set of gflags, to frontend, JniCatalog and JniFrontend. This also revamps existing query options to entertain the TBackendConfig. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 11 files changed, 144 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/3 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: make incremental stats max serialized size
configurable
..
Patch Set 2:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/4867/2//COMMIT_MSG
Commit Message:
PS2, Line 7: make
nit: Make
PS2, Line 9: fox
typo
PS2, Line 11: are experiencing
: regressions especially when upgrading.
Please expand a little on what the regression is.
Line 19: The change also includes TBackendConfig to pass backend config
Please add a note on revamp of existing query options to use the new
TBackendConfig method.
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS2, Line 73: cfg.load_catalog_in_background =
FLAGS_load_catalog_in_background;
: cfg.num_metadata_loading_threads =
FLAGS_num_metadata_loading_threads;
: cfg.sentry_config.assign(FLAGS_sentry_config);
: // auth_to_local rules are read if --load_auth_to_local_rules
is set to true
: // and impala is kerberized.
: cfg.auth_to_local = FLAGS_load_auth_to_local_rules &&
!FLAGS_principal.empty();
: cfg.principal.assign(FLAGS_principal);
: cfg.impala_log_lvl = FlagToTLogLevel(FLAGS_v);
: cfg.other_log_lvl = FlagToTLogLevel(FLAGS_non_impala_java_vlog);
: cfg.inc_stat_max_size = FLAGS_inc_stat_max_size;
Can you factor all this into a method, GetBackendConfig() or something similar?
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
PS2, Line 119: inc_stat_max_size
Rename to incremental_stats_size_limit_bytes or something? That way its more
self explanatory to the users.
Line 119: DEFINE_uint64(inc_stat_max_size, 200 * (1LL<<20), "Maximum size (in
bytes) of incremental "
long line.
http://gerrit.cloudera.org:8080/#/c/4867/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:
PS2, Line 104: TBackendConfig cfg;
:
cfg.authorization_policy_file.assign(FLAGS_authorization_policy_file);
: cfg.server_name.assign(FLAGS_server_name);
: cfg.sentry_config.assign(FLAGS_sentry_config);
: // auth_to_local rules are read if --load_auth_to_local_rules
is set to true
: // and impala is kerberized.
: cfg.auth_to_local = FLAGS_load_auth_to_local_rules &&
!FLAGS_principal.empty();
:
cfg.authorization_policy_provider_class.assign(FLAGS_authorization_policy_provider_class);
: cfg.kudu_master_hosts.assign(FLAGS_kudu_master_hosts);
: cfg.impala_log_lvl = FlagToTLogLevel(FLAGS_v);
: cfg.other_log_lvl =
FlagToTLogLevel(FLAGS_non_impala_java_vlog);
: cfg.inc_stat_max_size = FLAGS_inc_stat_max_size;
:
: jbyteArray cfg_bytes;
: JniLocalFrame jni_frame;
: ABORT_IF_ERROR(jni_frame.push(jni_env));
: ABORT_IF_ERROR(SerializeThriftMsg(jni_env, &cfg, &cfg_bytes));
Same as catalog.cc, please factor this into a method.
http://gerrit.cloudera.org:8080/#/c/4867/2/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 233: struct TBackendConfig {
Please document the usage of this thrift struct and how it is shared between fe
and be and how the Java side is expected to read the configs. Also use one line
spacing between members like elsewhere in this file.
PS2, Line 236: required
Some of these are only used by Catalog and not fe (and vice versa). Shouldn't
these be optional?
Line 245: 12: required string kudu_master_hosts
While you are at this, can you merge TStartupOptions (Frontend.thrift) with
this this and remove its references from RuntimeEnv and elsewhere ? You might
have to make it optional since its only used in fe and not catalog.
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
PS2, Line 288: incStatMaxSize in bytes
--incremental_stats_size_limit_bytes
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
PS2, Line 1591: MAX_INCREMENTAL_STATS_SIZE_BYTES
update
http://gerrit.cloudera.org:8080/#/c/4867/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
PS2, Line 33:
: // This is overriden by JniFrontend/JniCatalog classes with
user set configuration.
: // TODO: Read this from backend instead of using static
variables.
: private static boolean allowAuthToLocalRules_ = false;
:
: // Maximum size (in bytes) of incremental sta
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new patch set (#2). Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. IMPALA-3552: make incremental stats max serialized size configurable The fix fox "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when upgrading. To mitigate the issue, the change introduces a new gflag, 'inc_stat_max_size' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stat_max_size is 200MB. The change also includes TBackendConfig to pass backend config to frontend. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/frontend.cc M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 9 files changed, 109 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/2 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > No for both. Then, let's include it here. :) http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: PS1, Line 83: boolean loadInBackground, int numMetadataLoadingThreads, : String sentryServiceConfig, int impalaLogLevel, int otherLogLevel, : boolean allowAuthToLocal, String kerberosPrincipal > Its a good ramp-up task and I think we should do it here. This clean-up has Agreed. However, I think current fix contains enough changes. I will create a new jira ticket and take care of the clean up right after this commit. I want to minimize the scope of changes for one commit. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > Passing a TBackendConfig to Catalog::Catalog is a good idea. However, two q No for both. http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift File common/thrift/Types.thrift: PS1, Line 233: TBackendConfig > I also think this would not be the best place. please recommend better one. Lets leave it here for now and may be move it a suitable place in further rounds of reviews based on others' suggestions. http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: PS1, Line 83: boolean loadInBackground, int numMetadataLoadingThreads, : String sentryServiceConfig, int impalaLogLevel, int otherLogLevel, : boolean allowAuthToLocal, String kerberosPrincipal > Agreed. However, Let's do that in a separate commit. Its a good ramp-up task and I think we should do it here. This clean-up has been pending for a while and its nice to have it fixed. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > Since most of the code is shared between Catalog and Frontend, I think its Passing a TBackendConfig to Catalog::Catalog is a good idea. However, two questions. 1) are there any cases where two catalogs get instantiated? 2) do we want to change config even after catalog gets instantiated? http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift File common/thrift/Types.thrift: PS1, Line 233: TBackendConfig > I think it was named this way to mean the "configs passed to the Backend". I also think this would not be the best place. please recommend better one. unless, let's keep this here for now. For the name, I think 'TBackendConfig' is good for now, because we use BE/FE and BE takes a config options. http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: PS1, Line 83: boolean loadInBackground, int numMetadataLoadingThreads, : String sentryServiceConfig, int impalaLogLevel, int otherLogLevel, : boolean allowAuthToLocal, String kerberosPrincipal > How about moving all these into TBackendConfig? Agreed. However, Let's do that in a separate commit. http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: PS1, Line 120: boolean lazy, String serverName, String authorizationPolicyFile, : String sentryConfigFile, String authPolicyProviderClass, int impalaLogLevel, : int otherLogLevel, boolean allowAuthToLocal, String defaultKuduMasterHosts > Same, how about we make TBackendConfig more generic and refactor all the co Sure. Let's do that in a separate commit. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > I remember you asked for a generic (config) for all jvms, since some of the Since most of the code is shared between Catalog and Frontend, I think its fine to have a common place to source the configs. Callers make sure to use only the configs they need. http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift File common/thrift/Types.thrift: PS1, Line 233: TBackendConfig > This is not a backend config. it is more like a jvm config.. or config from I think it was named this way to mean the "configs passed to the Backend". It could be a little confusing. May be we can call it "TStartupOptions" or something. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > How about passing a TBackendConfig directly here. Basically make the constr I remember you asked for a generic (config) for all jvms, since some of these are catalog specific, I think the choice he has is to have a jvm config with separate sub field for different services. http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift File common/thrift/Types.thrift: PS1, Line 233: TBackendConfig This is not a backend config. it is more like a jvm config.. or config from backend? I think technically there is no "backend" outside of impalad. e.g. you cannot say statestore and catalog are backends. Just my two cents as this already becomes overly complicated. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-3552: make incremental stats max serialized size
configurable
..
Patch Set 1:
(4 comments)
Haven't gone through the whole patch, but I put some design comments. Let me
know what you think.
http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS1, Line 78: catalog_class_, catalog_ctor_,
: load_in_background, num_metadata_loading_threads,
sentry_config,
: FlagToTLogLevel(FLAGS_v),
FlagToTLogLevel(FLAGS_non_impala_java_vlog),
: auth_to_local, principal
How about passing a TBackendConfig directly here. Basically make the
constructor's signature accept a TBackendConfig instead of passing so many
variables.
http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 233: struct TBackendConfig {
This doesn't seem to be the ideal place but I don't see any good place to
include this. May be Alex/Dimitris knows.
http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
PS1, Line 83: boolean loadInBackground, int numMetadataLoadingThreads,
: String sentryServiceConfig, int impalaLogLevel, int
otherLogLevel,
: boolean allowAuthToLocal, String kerberosPrincipal
How about moving all these into TBackendConfig?
http://gerrit.cloudera.org:8080/#/c/4867/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
PS1, Line 120: boolean lazy, String serverName, String authorizationPolicyFile,
: String sentryConfigFile, String authPolicyProviderClass,
int impalaLogLevel,
: int otherLogLevel, boolean allowAuthToLocal, String
defaultKuduMasterHosts
Same, how about we make TBackendConfig more generic and refactor all the
configs so far? Also look for RuntimeEnv and TStartupOptions, that can be
merged with this as well.
--
To view, visit http://gerrit.cloudera.org:8080/4867
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: I still have two questions on the fix. 1. currently, tbackendconfig is introduced in types.thrift. is there a better place? (i.e., create a new file solely for this?) 2. tbackendconfig is passed in Catalog::Catalog and ExecEnv::ExecEnv. is it a good place? Please let me have feedbacks on those two. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has abandoned this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Abandoned after the 1st review, major change was made for the fix. hence, a new review started for this issue. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new change for review. http://gerrit.cloudera.org:8080/4867 Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. IMPALA-3552: make incremental stats max serialized size configurable The fix fox "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when upgrading. To mitigate the issue, the change introduces a new gflag, 'inc_stat_max_size' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stat_max_size is 200MB. The change also includes TBackendConfig to pass backend config to frontend. Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/runtime/exec-env.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 13 files changed, 77 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/4867/1 -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: @Yonghyun I had a chat with Alex on the best way to move forward. He seems to agree with using BackendConfig to pass the params. As per our discussion, its better if we create a thrift struct (ex: TBackendConfig or some such similar), serialize it and pass it using JNI, unpack it in the JniCatalog/Frontend and populate it in a map in the static BackendConfig class. We could have getter methods that return the appropriated type cast'ed value corresponding to a given config key. You could clean up all the existing config arguments too using this. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: PS1, Line 169: MAX_INCREMENTAL_STATS_SIZE_BYTES > I think he can try to use runtimeEnv as well. or have this in catalog not i To my best understanding, MAX_INC_*_BYTES is used in both of impalad and catalogd. So, RuntimeEnv solely could not be used. (libfesupport.so would complain during runtime.) Initially, I also thought BackendConfig is the best option for this because we can remove MAX_INC_*_BYTES from HdfsTable and consolidate BE options in one place. :) Alex/Huaisi/Bharath, if we decide to use BackendConfig, I will abandon this change and re-do the work, which would take few days. Please let me have your inputs. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: PS1, Line 169: MAX_INCREMENTAL_STATS_SIZE_BYTES > - Actually implementing this way seems to be little confusing to me. Reason I think he can try to use runtimeEnv as well. or have this in catalog not in a table. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4772/1//COMMIT_MSG Commit Message: PS1, Line 7: make nit: Make PS1, Line 16: impala Impala http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/common/global-flags.cc File be/src/common/global-flags.cc: PS1, Line 119: inc_stat_max_size whats the unit here? May be rename it to incremental_stats_max_size_mb or incremental_stats_max_size_bytes to make it more self explanatory? http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java: PS1, Line 289: MAX_INCREMENTAL_STATS_SIZE_BYTES, RuntimeEnv.INSTANCE.incStatMaxSize() Please replace with --incremental_stats_size_ http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: PS1, Line 169: MAX_INCREMENTAL_STATS_SIZE_BYTES - Actually implementing this way seems to be little confusing to me. Reason being, this variable is set only set in the Catalog code and remains un-initialized in the frontend code. Given we share classes between the Catalog and fe code, I think its confusing to the readers why we source it from RuntimeEnv at one place and MAX_INCREMENTAL_STATS_SIZE_BYTES in the other. - Initially when you asked my suggestion on the best way to pass this flag, I thought its only being passed to the Catalog and I suggested not to use the BackendConfig way. Now that I read the patch, I realized this is being used in the ComputeStats as well. How about falling back to the BackendConfig way to do this? Although its hacky, its easier to read and it can be set from both JniCatalog and JniFrontend (Look for --load_auth_to_local flag which does the same). - Sorry for the back and forth on this but after reading the patch, I think the hacky BackendConfig is more readable. Alex/Huaisi/Yonghyun thoughts? -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: actually this can this be checked when we load the table, and this should be evicted once we load the incremental stats. take a look at hdfsPartition.java::getFilteredHmsParameters(), it returns a new map. This makes no sense since we will never return the stats in the life time of catalog unless some commands lower the size. I have not looked at the whole process but we should not keep them in memory as possible as we can(not breaking other things). Suggest you manually filter the keys in place. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: Yonghyun, one minor comment on the code review the process. You don't need to respond and say "will take care of it". Typically, we address the comments and before sending a new patch we send the responses that indicate if something was "Done" or asking for clarifications. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 33: inc_stat_max_size > thanks. I will add the unit in option description. (in DEFINE_uint64(...) Sorry. I meant from the name we should know which unit it is. http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS1, Line 176: 0 > Actually, HdfsTable.MAX_INCREMENTAL_STATS_SIZE_BYTES is set in CatalogServi that is set by you right? I think a static member should not be set like this anyway. http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java: PS1, Line 71: incStatMaxSize > Sure. :) Do you recommend me to update computeLineage() as well? do not care about the rest.. :-) -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change.
Change subject: IMPALA-3552: make incremental stats max serialized size
configurable
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS1, Line 33: inc_stat_max_size
> could you be explicit on the unit?
thanks. I will add the unit in option description. (in DEFINE_uint64(...)
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 164: SentryConfig sentryConfig, TUniqueId catalogServiceId, String
kerberosPrincipal, long incStatMaxSize) {
> long line
I will take care of this.
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
PS1, Line 176: 0
> sorry I meant should not be a public static..
Actually, HdfsTable.MAX_INCREMENTAL_STATS_SIZE_BYTES is set in
CatalogServiceCatalog. Let me make this as a private var and create a method to
set it.
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
PS1, Line 71: incStatMaxSize
> getIncStatMaxSize
Sure. :) Do you recommend me to update computeLineage() as well?
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
Line 84: boolean allowAuthToLocal, String kerberosPrincipal, long
incStatMaxSize) throws InternalException {
> long line
will take care of it.
Line 101: numMetadataLoadingThreads, sentryConfig, getServiceId(),
kerberosPrincipal, incStatMaxSize);
> long
will take care of it.
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
File fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java:
Line 34: super(loadInBackground, numLoadingThreads, sentryConfig,
catalogServiceId, null, 200*1024*1024);
> long
will take care of it. thanks. :)
--
To view, visit http://gerrit.cloudera.org:8080/4772
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS1, Line 176: 0 > This should not be a static variable since no one outside this class is usi sorry I meant should not be a public static.. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-3552: make incremental stats max serialized size
configurable
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS1, Line 33: inc_stat_max_size
could you be explicit on the unit?
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 164: SentryConfig sentryConfig, TUniqueId catalogServiceId, String
kerberosPrincipal, long incStatMaxSize) {
long line
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
PS1, Line 176: 0
This should not be a static variable since no one outside this class is using
this anymore.
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
PS1, Line 71: incStatMaxSize
getIncStatMaxSize
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
Line 84: boolean allowAuthToLocal, String kerberosPrincipal, long
incStatMaxSize) throws InternalException {
long line
Line 101: numMetadataLoadingThreads, sentryConfig, getServiceId(),
kerberosPrincipal, incStatMaxSize);
long
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
File fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java:
Line 34: super(loadInBackground, numLoadingThreads, sentryConfig,
catalogServiceId, null, 200*1024*1024);
long
--
To view, visit http://gerrit.cloudera.org:8080/4772
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: local tests as well as jenkins are passing. http://sandbox.jenkins.cloudera.com/job/impala-private-build-and-test/4447/ -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Yonghyun Hwang has uploaded a new change for review. http://gerrit.cloudera.org:8080/4772 Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. IMPALA-3552: make incremental stats max serialized size configurable The fix fox "IMPALA-2648/IMPALA-2664" introduced a conservative limitation on the maximum serialized size of incremental stats. As a side effect, some users with very large tables are experiencing regressions especially when upgrading. To mitigate the issue, the change introduces a new gflag, 'inc_stat_max_size' to make the max serialized size configurable, which allows impala users to specify their own maximum serialized size. Default value for inc_stat_max_size is 200MB. Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 --- M be/src/catalog/catalog.cc M be/src/common/global-flags.cc M be/src/service/fe-support.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java 10 files changed, 33 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/4772/1 -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang
