Tim Armstrong 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: (5 comments) 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? Oops, yep had it reversed. 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@1037 PS2, Line 1037: self.parent = None The binary search caught a pretty bad bug - we don't set runner.parent when doing the binary search. I reran it successfully. http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1144 PS2, Line 1144: canel > cancel Done 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 Good point. I tried to clarify it with some comments, lmk if it could be further improved. 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 I think the naming of the variable is a little misleading since it includes queries that failed or were cancelled. I made the name longer and more verbose, LMK if it makes it clearer (I think it probably does). -- 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 23:10:08 +0000 Gerrit-HasComments: Yes
