Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11587 )
Change subject: IMPALA-7643: report # queries actually executing in stress test ...................................................................... Patch Set 2: (4 comments) Can you also do a quick binary search? It's fine if you do that just with TPCH or something. Sometimes that can regress. http://gerrit.cloudera.org:8080/#/c/11587/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11587/2//COMMIT_MSG@17 PS2, Line 17: 2 1? http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py File tests/stress/concurrent_select.py: http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1144 PS2, Line 1144: canel cancel http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1153 PS2, Line 1153: if (not started_running and query_state not in ('PENDING_STATE', : 'INITIALIZED_STATE')): : started_running = True : self.parent.increment_num_queries_started_running() : : if query_state not in ('PENDING_STATE', 'INITIALIZED_STATE', 'RUNNING_STATE'): : return True Please leave a comment on the logic here. While it makes sense if you look at common/thrift/hive-1-api/TCLIService.thrift, it makes less sense if you're just trying to read the code in this file. In particular, the confusing part is that something called "increment_num_queries_started_running()" (with the word "running" in it) gets called if query_state = 'RUNNING_STATE', but we're still not ready to return True L1159 due to L1158. A bit of commenting makes the logic more clear for readers. http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1164 PS2, Line 1164: self._cancel(cursor, report) : if not started_running: : self.parent.increment_num_queries_started_running() I'm trying to understand the reasoning for the increment here. I think that means a comment is necessary. :) -- To view, visit http://gerrit.cloudera.org:8080/11587 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5692e8e5ba3224becefc24437197bf5a5b450335 Gerrit-Change-Number: 11587 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 05 Oct 2018 17:57:22 +0000 Gerrit-HasComments: Yes
