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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9976 )

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

Move some test_spilling debug actions to exhaustive

Phil Z pointed out that these tests take a long time to run.
Based on experience, we only need the always/never variants
to catch the vast majority of spilling-related bugs, so
relegating the other variants to exhaustive should not cause any major
problems.

People making spilling-related changes to the spilling code should run
this test with the exhaustive dimension to flush out any problems in
advance, e.g.:

  impala-py.test tests/query_test/test_spilling.py -n2 --verbose \
--workload_exploration_strategy=functional-query:exhaustive

Change-Id: I9ea9f6c299480f8dfc943635342e4473499cc8ad
Reviewed-on: http://gerrit.cloudera.org:8080/9976
Reviewed-by: Tim Armstrong 
Reviewed-by: Philip Zeyliger 
Tested-by: Impala Public Jenkins 
---
M tests/query_test/test_spilling.py
1 file changed, 13 insertions(+), 4 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Philip Zeyliger: Looks good to me, approved
  Impala Public Jenkins: Verified

--
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: merged
Gerrit-Change-Id: I9ea9f6c299480f8dfc943635342e4473499cc8ad
Gerrit-Change-Number: 9976
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3: Verified+1


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 12 Apr 2018 01:25:35 +
Gerrit-HasComments: No


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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2286/


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 21:08:33 +
Gerrit-HasComments: No


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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2281/


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 20:38:44 +
Gerrit-HasComments: No


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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2281/


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:45:44 +
Gerrit-HasComments: No


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

2018-04-11 Thread Tim Armstrong (Code Review)
Tim Armstrong 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:

(1 comment)

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')
> set num_nodes=3?
num_nodes=1 and num_nodes=0 are the only values that do anything.



--
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 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:33:39 +
Gerrit-HasComments: Yes


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

2018-04-11 Thread Philip Zeyliger (Code Review)
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 3: Code-Review+2

(1 comment)

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')
> We already set buffer_pool_limit to force all these queries to spill, the p
set num_nodes=3?



-- 
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:28:59 +
Gerrit-HasComments: Yes


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

2018-04-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3:

And yeah, these tests are definitely time consuming - I've run them locally 
enough times :).


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:22:32 +
Gerrit-HasComments: No


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

2018-04-11 Thread Tim Armstrong (Code Review)
Tim Armstrong 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:

(2 comments)

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?
We already set buffer_pool_limit to force all these queries to spill, the 
problem is that the amount of data per impalad changes. I expect it would work 
fine on a 3 node remote cluster, but that probably needs testing.


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 intentio
Oops.



--
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 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:21:18 +
Gerrit-HasComments: Yes


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

2018-04-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3: Code-Review+2

carry +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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:21:27 +
Gerrit-HasComments: No


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

2018-04-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9976 )

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


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2280/


--
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: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:21:39 +
Gerrit-HasComments: No


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

2018-04-11 Thread Tim Armstrong (Code Review)
Hello Philip Zeyliger,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9976

to look at the new patch set (#3).

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

Move some test_spilling debug actions to exhaustive

Phil Z pointed out that these tests take a long time to run.
Based on experience, we only need the always/never variants
to catch the vast majority of spilling-related bugs, so
relegating the other variants to exhaustive should not cause any major
problems.

People making spilling-related changes to the spilling code should run
this test with the exhaustive dimension to flush out any problems in
advance, e.g.:

  impala-py.test tests/query_test/test_spilling.py -n2 --verbose \
--workload_exploration_strategy=functional-query:exhaustive

Change-Id: I9ea9f6c299480f8dfc943635342e4473499cc8ad
---
M tests/query_test/test_spilling.py
1 file changed, 13 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/9976/3
--
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: newpatchset
Gerrit-Change-Id: I9ea9f6c299480f8dfc943635342e4473499cc8ad
Gerrit-Change-Number: 9976
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Philip Zeyliger 


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

2018-04-11 Thread Philip Zeyliger (Code Review)
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 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 11 Apr 2018 16:09:50 +