impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3677: Write minidump on SIGUSR1
Date Fri, 08 Jul 2016 20:52:52 GMT
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 <lv@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message