You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Philip Zeyliger (Code Review)" <ge...@cloudera.org> on 2018/04/11 16:09:50 UTC

[Impala-ASF-CR] Move some test spilling debug actions to exhaustive

Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9976 )

Change subject: Move some test_spilling debug actions to exhaustive
......................................................................


Patch Set 2: Code-Review+2

(2 comments)

Alex Behm pointed out that the metric I was using for the time may have been "cpu seconds" and therefore somewhat wrong. (Python's xdist would have been the semi-culprit here.)

That said, some timestamped logs I have suggest that there are still 14 minutes running these tests on one thread (all the same "gw11"), so I think this will make a noticeable improvement.

As far as the code, please look at my comment on line 51. I'm fine with you carrying the +2 for the one trivial change.


$cat log-test-EE_TEST_PARALLEL.txt  | grep TestSpilling | grep gw11
2018-04-10 11:35:16.338894 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_aggs[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.5', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:40:18.259913 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_aggs[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.9', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:42:58.561880 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_aggs[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:43:07.442799 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_large_rows[exec_option: {'debug_action': None, 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:43:21.291602 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_large_rows[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.1', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:43:37.639672 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_large_rows[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.5', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:43:53.933062 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_large_rows[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.9', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:44:11.333679 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_large_rows[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:45:37.174578 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_naaj[exec_option: {'debug_action': None, 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:47:06.525427 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_naaj[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.1', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]
2018-04-10 11:49:27.954346 [gw11] PASSED query_test/test_spilling.py::TestSpillingDebugActionDimensions::test_spilling_naaj[exec_option: {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.5', 'default_spillable_buffer_size': '256k'} | table_format: parquet/none]

http://gerrit.cloudera.org:8080/#/c/9976/2/tests/query_test/test_spilling.py
File tests/query_test/test_spilling.py:

http://gerrit.cloudera.org:8080/#/c/9976/2/tests/query_test/test_spilling.py@38
PS2, Line 38:                    reason='Queries may not spill on larger clusters')
Not related to current change: would setting memlimits make them spill?


http://gerrit.cloudera.org:8080/#/c/9976/2/tests/query_test/test_spilling.py@51
PS2, Line 51:       debug_action_dims.extend(EXHAUSTIVE_DEBUG_ACTION_DIMS)
nit: I think this mutates CORE_DEBUG_ACTION_DIMS, which isn't your intention.

I recommend:

debug_action_dims = CORE_DEBUG_ACTION_DIMS + EXHAUSTIVE_DEBUG_ACTION_IMS



# example

>>> x = [1]
>>> y = [2]
>>> z = x
>>> z.extend(y)
>>> x
[1, 2]



-- 
To view, visit http://gerrit.cloudera.org:8080/9976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ea9f6c299480f8dfc943635342e4473499cc8ad
Gerrit-Change-Number: 9976
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:09:50 +0000
Gerrit-HasComments: Yes