Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21616 )

Change subject: IMPALA-12345: Add user quotas to Admission Control
......................................................................


Patch Set 3:

(3 comments)

A few minor comments. I still have more to review, but won't get to it today.

http://gerrit.cloudera.org:8080/#/c/21616/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21616/3//COMMIT_MSG@24
PS3, Line 24: two reasons: (1) as queries can only be admitted from the front 
of the
nit: "as" is redundant at the beginning of the statement.


http://gerrit.cloudera.org:8080/#/c/21616/3/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/21616/3/be/src/common/global-flags.cc@193
PS3, Line 193:
nit: why add whitespace?


http://gerrit.cloudera.org:8080/#/c/21616/3/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:

http://gerrit.cloudera.org:8080/#/c/21616/3/be/src/scheduling/admission-controller-test.cc@404
PS3, Line 404:     DCHECK(false) << "unknown outcome";
This should be EXPECT_FALSE. clang-tidy flags this because it doesn't exit, and 
release build will likely fail to compile.



--
To view, visit http://gerrit.cloudera.org:8080/21616
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c33f3f2427db57fb9b6c593a4b22d5029549b41
Gerrit-Change-Number: 21616
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Mon, 05 Aug 2024 19:50:15 +0000
Gerrit-HasComments: Yes

Reply via email to