impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Date Thu, 20 Oct 2016 22:05:58 GMT
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-3552: make incremental stats max serialized size configurable

Patch Set 1:

Commit Message:

PS1, Line 7: make
nit: Make

PS1, Line 16: impala
File be/src/common/

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?
File fe/src/main/java/org/apache/impala/analysis/

Please replace with --incremental_stats_size_<unit>
File fe/src/main/java/org/apache/impala/catalog/

- 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
To unsubscribe, visit

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

View raw message