Lars Volker 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 3: (5 comments) Apologies for being late, I only had some small comments. http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc@571 PS3, Line 571: UnixMillis() - last_report_time_ms_ Maybe clamp this at 0 to make sure we won't return a negative value if someone refreshes the page at the wrong time? http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@83 PS3, Line 83: get_thrift_profile get_thrift_profile takes a timeout, so you can call it once before the loop to wait until the profile shows up. http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@90 PS3, Line 90: 2: Can you make this a constant and add a comment. Otherwise this test will fail if someone adds a new profile node in front of them? Alternatively you could make the search a bit more generic and iterate over all nodes. http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@108 PS3, Line 108: # Minimum NTP poll period is 8 seconds. I found this part tricky to read. Why is it 8 seconds? Can you elaborate in the comment what the intent here is? http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@112 PS3, Line 112: self.client.fetch(query, handle) Should we just call close_query() here? -- 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: 3 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: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Fri, 30 Nov 2018 00:58:20 +0000 Gerrit-HasComments: Yes
