Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16891 )
Change subject: IMPALA-9975 (part 2): Introduce new admission control daemon ...................................................................... Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/16891/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/16891/1/be/src/runtime/exec-env.cc@289 PS1, Line 289: if (FLAGS_is_coordinator && FLAGS_admission_control_service_addr.empty()) { > IIRC the executors do actually need an AdmissionController to report the me You're right. There's a lot more thought that needs to go in to what actually needs to be sent to the statestore when the admission service is in use, and what changes can be made to the admission algorithm now that we're not relying as much on eventually consistent info about cluster load. I think its best to leave this for follow up work, so I'll revert this to the original behavior. I filed IMPALA-10423 to track this http://gerrit.cloudera.org:8080/#/c/16891/1/be/src/scheduling/admissiond-main.cc File be/src/scheduling/admissiond-main.cc: http://gerrit.cloudera.org:8080/#/c/16891/1/be/src/scheduling/admissiond-main.cc@44 PS1, Line 44: ABORT_IF_ERROR(daemon_env.Init(/* init_jvm */ false)); > Does the admission controller need to load the llama-site.xml and fair-sche Done http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py@74 PS1, Line 74: t > flake8: E501 line too long (98 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py@293 PS1, Line 293: def build_admissiond_arg_list(): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py@413 PS1, Line 413: 127.0.0.1 > Maybe should be INTERNAL_LISTEN_HOST. Not sure it matters though. Done. This brought up the good point that as previously written admission_control_service_addr had to be a resolved IP address. I fixed that, and also changed it to just a hostname with the port taken from the admission_service_port flag, to be more consistent with how we handle this for the statestored and catalogd. http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py File tests/common/impala_cluster.py: http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@106 PS1, Line 106: s > flake8: E501 line too long (102 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@236 PS1, Line 236: i > flake8: E501 line too long (101 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@566 PS1, Line 566: class AdmissiondProcess(BaseImpalaProcess): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@571 PS1, Line 571: # > flake8: E265 block comment should start with '# ' Done -- To view, visit http://gerrit.cloudera.org:8080/16891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id677814b31e9193035e8cf0d08aba0ce388a0ad9 Gerrit-Change-Number: 16891 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 08 Jan 2021 22:18:04 +0000 Gerrit-HasComments: Yes
