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 <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang <[email protected]>
Gerrit-HasComments: Yes

Reply via email to