Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201607082052.u68KqqNP018762@ip-10-146-233-104.ec2.internal>
Date: Fri, 8 Jul 2016 20:52:52 +0000
From: "Lars Volker (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, dev@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>, David Knupp <dknupp@cloudera.com>
Reply-To: lv@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-CR=5D=28cdh5-trunk=29_IMPALA-3677=3A_Write_minidump_on_SIGUSR1=0A?=
In-Reply-To: <gerrit.1465071954000.I40149e48e391451de21a5c8bda18e2307fc89513@gerrit.cloudera.org>
References: <gerrit.1465071954000.I40149e48e391451de21a5c8bda18e2307fc89513@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Fri, 08 Jul 2016 20:52:58 -0000

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