You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2019/03/19 19:01:45 UTC
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12792
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
IMPALA-8320: Handle psutil error in tests
We use psutil.Process.cmdline to access a process's command line. This
field is a property and tries to retrieve process information from the
OS. If the process has terminated in the meantime, such access will
result in an error which we need to handle. This change adds proper
error handling. I ran the existing Breakpad tests to make sure we don't
regress, but there's no obvious way to test that particular corner case
that would be worth the effort.
Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
---
M tests/common/impala_cluster.py
1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/12792/1
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12792
to look at the new patch set (#2).
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
IMPALA-8320: Handle psutil error in tests
We use psutil.Process.cmdline to access a process's command line. This
field is a property and tries to retrieve process information from the
OS. If the process has terminated in the meantime, such access will
result in an error which we need to handle. This change adds proper
error handling. I ran the existing Breakpad tests to make sure we don't
regress, but there's no obvious way to test that particular corner case
that would be worth the effort.
Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
---
M tests/common/impala_cluster.py
1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/12792/2
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2467/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2019 21:40:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
IMPALA-8320: Handle psutil error in tests
We use psutil.Process.cmdline to access a process's command line. This
field is a property and tries to retrieve process information from the
OS. If the process has terminated in the meantime, such access will
result in an error which we need to handle. This change adds proper
error handling. I ran the existing Breakpad tests to make sure we don't
regress, but there's no obvious way to test that particular corner case
that would be worth the effort.
Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Reviewed-on: http://gerrit.cloudera.org:8080/12792
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/common/impala_cluster.py
1 file changed, 7 insertions(+), 1 deletion(-)
Approvals:
Lars Volker: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Thank you for fixing this.
http://gerrit.cloudera.org:8080/#/c/12792/1/tests/common/impala_cluster.py
File tests/common/impala_cluster.py:
http://gerrit.cloudera.org:8080/#/c/12792/1/tests/common/impala_cluster.py@205
PS1, Line 205: cmdline = ''
: try:
: cmdline = process.cmdline
: except psutil.NoSuchProcess:
: # IMPALA-8320: psutil.Process.cmdline is a property and the process could have
: # disappeared between the time we built the process list and now.
: continue
Nit: Do you think this should be below the comment about IMPALA-6889 and saving the cmdline? i.e.
# IMPALA-6889: ...
# ...
[your code setting cmdline here]
if len(cmdline) == 0:
...
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2019 20:10:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 01:23:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Addressed Joe's comment, carrying his +2.
http://gerrit.cloudera.org:8080/#/c/12792/1/tests/common/impala_cluster.py
File tests/common/impala_cluster.py:
http://gerrit.cloudera.org:8080/#/c/12792/1/tests/common/impala_cluster.py@205
PS1, Line 205: # IMPALA-6889: When a process shuts down and becomes a zombie its cmdline becomes
: # empty for a brief moment, before it gets reaped by its parent (see man proc). We
: # copy the cmdline to prevent it from changing between the following checks and
: # the construction of the *Process objects.
: cmdline = ''
: try:
: cmdline
> Nit: Do you think this should be below the comment about IMPALA-6889 and sa
Done
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2019 20:55:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3923/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2019 20:55:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8320: Handle psutil error in tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12792 )
Change subject: IMPALA-8320: Handle psutil error in tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2466/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5fec061f845d3deb6328e52e590a7a79196dfd4
Gerrit-Change-Number: 12792
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2019 19:35:08 +0000
Gerrit-HasComments: No