You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2016/06/08 17:13:12 UTC

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

Lars Volker has uploaded a new change for review.

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................

IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

The breakpad tests were writing core files when triggering minidump
writes. This was actually not needed and interfered with test execution
and artifact collection. Most notably processes would take a long time
to terminate while writing core files (IMPALA-3684). The core files
would also be wrongly collected by Jenkins (IMPALA-3693).

This change adds code to stop test clusters reliably, making
test_breakpad independent from calling setup-impala-cluster.py via
os.system. It also disables core dumps for the duration of the test and
re-enables them afterwards.

Change-Id: If592339632aa662b59be09d911229566d5772321
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 32 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 1:

(2 comments)

Thanks for the review. Tests pass on Jenkins here: http://sandbox.jenkins.cloudera.com/job/impala-umbrella-build-and-test/1576/

01:55:26.660 custom_cluster/test_breakpad.py .....

http://gerrit.cloudera.org:8080/#/c/3339/1/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS1, Line 57:       return
> Can you make this something like:
Done


PS1, Line 63:     if cls.exploration_strategy() != 'exhaustive':
            :       return
> If you use pytest.skip() as I suggested L57, this block is not needed.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3339/2/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS2, Line 186:     self.kill_processes(self.cluster.impalads[:1], SIGSEGV)
> I'm curious: why did you choose [:1] instead of [0]?
the method takes a list, so I thought this was more clear than [self.c.i[0]] . Should I change it to the latter?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

The breakpad tests were writing core files when triggering minidump
writes. This was actually not needed and interfered with test execution
and artifact collection. Most notably processes would take a long time
to terminate while writing core files (IMPALA-3684). The core files
would also be wrongly collected by Jenkins (IMPALA-3693).

This change adds code to stop test clusters reliably, making
test_breakpad independent from calling setup-impala-cluster.py via
os.system. It also disables core dumps for the duration of the test and
re-enables them afterwards.

Change-Id: If592339632aa662b59be09d911229566d5772321
Reviewed-on: http://gerrit.cloudera.org:8080/3339
Reviewed-by: Michael Brown <mi...@cloudera.com>
Reviewed-by: Silvius Rus <sr...@cloudera.com>
Tested-by: Lars Volker <lv...@cloudera.com>
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 34 insertions(+), 13 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  Lars Volker: Verified
  Silvius Rus: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Silvius Rus <sr...@cloudera.com>

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Is this passing in Jenkins? Thanks.

http://gerrit.cloudera.org:8080/#/c/3339/1/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS1, Line 57:       return
Can you make this something like:

 pytest.skip('breakpad tests only run in exhaustive')


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown,

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

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

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................

IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

The breakpad tests were writing core files when triggering minidump
writes. This was actually not needed and interfered with test execution
and artifact collection. Most notably processes would take a long time
to terminate while writing core files (IMPALA-3684). The core files
would also be wrongly collected by Jenkins (IMPALA-3693).

This change adds code to stop test clusters reliably, making
test_breakpad independent from calling setup-impala-cluster.py via
os.system. It also disables core dumps for the duration of the test and
re-enables them afterwards.

Change-Id: If592339632aa662b59be09d911229566d5772321
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 34 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 1:

Taking a look.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 2: Verified+1

PS2 (9d0bab6b9764d18189e4650af92738eef3f7cbd4) validated by private jenkins run (hdfs, cluster tests) http://sandbox.jenkins.cloudera.com/job/impala-umbrella-build-and-test/1576/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Silvius Rus <sr...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3339/2/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS2, Line 186:     self.kill_processes(self.cluster.impalads[:1], SIGSEGV)
I'm curious: why did you choose [:1] instead of [0]?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 2: Code-Review+2

Approved by gatekeeper.  Test-only change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Silvius Rus <sr...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3339/1/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS1, Line 63:     if cls.exploration_strategy() != 'exhaustive':
            :       return
If you use pytest.skip() as I suggested L57, this block is not needed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-2.6.0 5.8.0) IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests

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

Change subject: IMPALA-3684, IMPALA-3693: Disable core files for breakpad tests
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

I saw that the breakpad tests succeeded in Jenkins, and this looks good.

http://gerrit.cloudera.org:8080/#/c/3339/2/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

PS2, Line 186:     self.kill_processes(self.cluster.impalads[:1], SIGSEGV)
> the method takes a list, so I thought this was more clear than [self.c.i[0]
No, it's just a subtly I missed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If592339632aa662b59be09d911229566d5772321
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes