Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12000 )
Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc@47 PS1, Line 47: "Last report received time"; > I am nit-picking somewhat, but this is really 'Last received time' from the How about "Last report received time". "received time" by itself may not be very clear on what was received. http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@95 PS1, Line 95: if report_time < report_time_dict.get(node.name, datetime.min): > What happens here the first time through, when node.name does not yet exist The second argument (datetime.min) is the default value if the key doesn't exist. http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@111 PS1, Line 111: assert num_time_backward <= ceil(elapsed_time / MIN_NTP_POLL_PERIOD) > Should this be num_time_backward? time_backward is a boolean, and shouldn't Nice catch. Fixed. http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@112 PS1, Line 112: s > flake8: F841 local variable 'results' is assigned to but never used Done -- To view, visit http://gerrit.cloudera.org:8080/12000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa Gerrit-Change-Number: 12000 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Balazs Jeszenszky <jes...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Thu, 29 Nov 2018 00:04:39 +0000 Gerrit-HasComments: Yes
