Lars Volker has posted comments on this change.

Change subject: IMPALA-3677: Write minidump on SIGUSR1
......................................................................


Patch Set 7:

(2 comments)

Thanks for the reviews, please see PS8.

http://gerrit.cloudera.org:8080/#/c/3312/7/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

Line 117:   def wait_for_num_processes(self, daemon, num_expected, timeout=60):
> This method seems to be doing a bit more than it needs to, which I'm mainly
Yes, my original assumption was that the time we need to write all minidumps 
should never exceed 5 seconds (which was an more of an  educated guess). The 
number of expected processes is at most cluster_size, which is the number of 
local impalads running on the same machine. At 100MB/sec and 2.5MB per file we 
should be able to write about 200 minidumps in 5 seconds, so that looks safe to 
me. However there could be unknown things affecting this time.

Anyways, now there's a wait loop in place. Should I modify it to scale the 
timeout with num_expected. If you're ok I'd leave it at 5sec, as I'd be very 
interested to have pop up as a failure if it ever takes longer than this.


PS7, Line 173: 3
> This could still be replaced with 'cluster_size', right?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40149e48e391451de21a5c8bda18e2307fc89513
Gerrit-PatchSet: 7
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to