You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2017/01/24 23:19:37 UTC

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Jim Apple has uploaded a new change for review.

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

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................

IMPALA-4818: Ensure the same number of tests are run every time

This is a partial revert of Change-Id: I04b4d6db508a26a1a2e4b972b...
from January 2014. That commit caused test_cancel_insert to run
different random tests each time it was run; the number of tests run
could vary between 1 and 9. This commit fixes the number at 9, and the
same 9 every time, to avoid test flakiness.

Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
---
M tests/query_test/test_cancellation.py
1 file changed, 0 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/5784/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5784/2/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

PS2, Line 195:     if cls.exploration_strategy() != 'exhaustive':
             :       cls.ImpalaTestMatrix.add_constraint(lambda v: v.get_value('cancel_delay') in [3])
L196 will rarely if ever get executed, because the workload as derived from L60 is "tpch", not "functional-query". Typical exhaustive tests tend only to apply to tests whose workload is functional-query, via "--workload_exploration_strategy functional-query:exhaustive".

I believe this is so because, unlike the functional database, the tpch database will not exist in all the typical exhaustive formats when doing a data load, which means typical "exhaustive vectors" aren't possible with tpch. However, the queries (L35) here come from tpch, hence the value of get_workload().

Some options:

- Remove L195. If tests are quick, this is OK.
or
- Hack up test dimensions and workload to avoid redundant tests but also truly run various cancel delays when workload_exploration_strategy is functional-query:exhaustive .
or
- Remove L195-196 if loss of coverage is only negligible


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5784/2/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

PS2, Line 195:     if cls.exploration_strategy() != 'exhaustive':
             :       cls.ImpalaTestMatrix.add_constraint(lambda v: v.get_value('cancel_delay') in [3])
> L196 will rarely if ever get executed, because the workload as derived from
I don't quite see what you're getting at here. It seems like line 198-199 in HEAD get executed for every "core" run.

In the history of this patch, line 195 was added because tests were too slow. Generally, I'd like to confine this patch to just derandomizing this method and worry about which test should be run later if we see a problem.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/254/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 1:

Dry-run: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/208/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2:

rebase only, carry David's +1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 1:

> Dry-run: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/208/

Failed due to network issues with gerrit.cloudera.org. Re-run: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/214/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Hello David Knupp,

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

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

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

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................

IMPALA-4818: Ensure the same number of tests are run every time

This is a partial revert of Change-Id: I04b4d6db508a26a1a2e4b972b...
from January 2014. That commit caused test_cancel_insert to run
different random tests each time it was run; the number of tests run
could vary between 1 and 9. This commit fixes the number at 9, and the
same 9 every time, to avoid test flakiness.

Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
---
M tests/query_test/test_cancellation.py
1 file changed, 0 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/5784/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


IMPALA-4818: Ensure the same number of tests are run every time

This is a partial revert of Change-Id: I04b4d6db508a26a1a2e4b972b...
from January 2014. That commit caused test_cancel_insert to run
different random tests each time it was run; the number of tests run
could vary between 1 and 9. This commit fixes the number at 9, and the
same 9 every time, to avoid test flakiness.

Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Reviewed-on: http://gerrit.cloudera.org:8080/5784
Reviewed-by: Michael Brown <mi...@cloudera.com>
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/query_test/test_cancellation.py
1 file changed, 0 insertions(+), 3 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Michael Brown: Looks good to me, but someone else must approve
  Alex Behm: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4818: Ensure the same number of tests are run every time

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: IMPALA-4818: Ensure the same number of tests are run every time
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

Maybe this is a candidate for upvoting?

http://gerrit.cloudera.org:8080/#/c/5784/2/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

PS2, Line 195:     if cls.exploration_strategy() != 'exhaustive':
             :       cls.ImpalaTestMatrix.add_constraint(lambda v: v.get_value('cancel_delay') in [3])
> I don't quite see what you're getting at here. It seems like line 198-199 i
This ends up falling under IMPALA-3947. Sorry for the noise.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I22cecfbe7c9a102f788d01eb80aa188579ef6d7e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes