Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12501 )

Change subject: IMPALA-8191: Wait for additional breakpad processes during test
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12501/1/tests/common/impala_cluster.py
File tests/common/impala_cluster.py:

http://gerrit.cloudera.org:8080/#/c/12501/1/tests/common/impala_cluster.py@321
PS1, Line 321:
> Do you know why this is set?
I don't know, it could be that some test mixed up the order of command line 
parameters?


http://gerrit.cloudera.org:8080/#/c/12501/1/tests/common/impala_cluster.py@322
PS1, Line 322:   def __get_pid(self):
> Instead of returning pid here, you could collect all matches, and then eith
Picking the parent should have prevented the issue altogether. The breakpad 
tests are the only case where we can have multiple PIDs for a process, and 
determining the parent seems to come with several edge cases that we would need 
to handle for a general solution, e.g. either of the two processes could exit 
while we are determining which one is which.

Since all we need to do is wait for all of them to be gone, I think it's best 
to just return a list for that test.


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

http://gerrit.cloudera.org:8080/#/c/12501/1/tests/custom_cluster/test_breakpad.py@88
PS1, Line 88:   def wait_for_all_processes_dead(self, processes, timeout=300):
            :     for process in processes:
            :       try:
            :         # For every process in th
> OMG.
Did that. I have mixed feelings about the result as it adds quite a bit of code 
for this special case. Let me know what you think.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4dcc5fecb9b5f38ae1504aae40f099837cf1bca
Gerrit-Change-Number: 12501
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Comment-Date: Tue, 19 Feb 2019 23:43:45 +0000
Gerrit-HasComments: Yes

Reply via email to