Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12407 )
Change subject: IMPALA-5043: diagnostics for topic staleness in AC ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/12407/3/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/12407/3/be/src/scheduling/admission-controller.cc@49 PS3, Line 49: stale > nit: repeated Done http://gerrit.cloudera.org:8080/#/c/12407/3/be/src/scheduling/admission-controller.cc@292 PS3, Line 292: } else { : last_topic_update_time_ms_ = MonotonicMillis(); > this might be a bit misleading since this does not signify that the topic u Made that change. The update time reported is not quite right in that case but I think we can live with it (there's not really a more reasonable value to pick). http://gerrit.cloudera.org:8080/#/c/12407/3/be/src/scheduling/admission-controller.cc@1127 PS3, Line 1127: // Include a space before the sentence because we're appending to an existing : // sentence. > nit: 'stale' comment? :P Done http://gerrit.cloudera.org:8080/#/c/12407/3/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/12407/3/tests/custom_cluster/test_admission_controller.py@861 PS3, Line 861: STALE_TOPIC_THRESHOLD_MS > nit: maybe move this to the top with other constants? (feel free to ignore Done http://gerrit.cloudera.org:8080/#/c/12407/3/tests/custom_cluster/test_admission_controller.py@919 PS3, Line 919: assert num_admitted_immediately == 1 : assert num_rejected == 1 : assert num_queued == NUM_QUERIES - 2 > based on the AC config: Done -- To view, visit http://gerrit.cloudera.org:8080/12407 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9e26adb6419589ccf7625e423356df45bee4ac9 Gerrit-Change-Number: 12407 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 08 Feb 2019 21:58:05 +0000 Gerrit-HasComments: Yes
