Return-Path: 
 <dev-return-140-apmail-impala-dev-archive=impala.apache.org@impala.incubator.apache.org>
X-Original-To: apmail-impala-dev-archive@minotaur.apache.org
Delivered-To: apmail-impala-dev-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id E39FC1864A
	for <apmail-impala-dev-archive@minotaur.apache.org>;
 Mon, 29 Feb 2016 19:26:43 +0000 (UTC)
Received: (qmail 2283 invoked by uid 500); 29 Feb 2016 19:26:31 -0000
Delivered-To: apmail-impala-dev-archive@impala.apache.org
Received: (qmail 2240 invoked by uid 500); 29 Feb 2016 19:26:31 -0000
Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@impala.incubator.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@impala.incubator.apache.org>
List-Post: <mailto:dev@impala.incubator.apache.org>
List-Id: <dev.impala.incubator.apache.org>
Reply-To: dev@impala.incubator.apache.org
Delivered-To: mailing list dev@impala.incubator.apache.org
Received: (qmail 2220 invoked by uid 99); 29 Feb 2016 19:26:30 -0000
Received: from pnap-us-west-generic-nat.apache.org (HELO
 spamd4-us-west.apache.org) (209.188.14.142)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 29 Feb 2016 19:26:30 +0000
Received: from localhost (localhost [127.0.0.1])
	by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org)
 with ESMTP id 560ECC0140
	for <dev@impala.apache.org>; Mon, 29 Feb 2016 19:26:30 +0000 (UTC)
X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org
X-Spam-Flag: NO
X-Spam-Score: 0.362
X-Spam-Level: 
X-Spam-Status: No, score=0.362 tagged_above=-999 required=6.31
	tests=[RDNS_DYNAMIC=0.363, SPF_PASS=-0.001] autolearn=disabled
Received: from mx2-lw-us.apache.org ([10.40.0.8])
	by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new,
 port 10024)
	with ESMTP id Ygxq0bzu33U0 for <dev@impala.apache.org>;
	Mon, 29 Feb 2016 19:26:26 +0000 (UTC)
Received: from ip-10-146-233-104.ec2.internal
 (ec2-75-101-130-251.compute-1.amazonaws.com [75.101.130.251])
	by mx2-lw-us.apache.org (ASF Mail Server at mx2-lw-us.apache.org) with ESMTPS
 id 883E75FBC2
	for <dev@impala.incubator.apache.org>; Mon, 29 Feb 2016 19:26:19 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
	by ip-10-146-233-104.ec2.internal (8.14.4/8.14.4) with ESMTP id
 u1TJQI3F016146;
	Mon, 29 Feb 2016 19:26:18 GMT
Message-Id: <201602291926.u1TJQI3F016146@ip-10-146-233-104.ec2.internal>
Date: Mon, 29 Feb 2016 19:26:18 +0000
From: "Tim Armstrong (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, dev@impala.incubator.apache.org
CC: Sailesh Mukil <sailesh@cloudera.com>
Reply-To: tarmstrong@cloudera.com
X-Gerrit-MessageType: comment
Subject: 
 =?UTF-8?Q?[Impala-CR](cdh5-2.5.0=5F5.7.0)_IMPALA-2982:_lazy_initialization_of_HBase_connection=0A?=
X-Gerrit-Change-Id: I405f5cbfc62dd4a4aeb6f4019cac358376478884
X-Gerrit-ChangeURL: <http://gerrit.cloudera.org:8080/2345>
X-Gerrit-Commit: c8d19af7d63e5709cf5bc654a758872a1e194fe5
In-Reply-To: 
 <gerrit.1456716900000.I405f5cbfc62dd4a4aeb6f4019cac358376478884@gerrit.cloudera.org>
References: 
 <gerrit.1456716900000.I405f5cbfc62dd4a4aeb6f4019cac358376478884@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.10-rc0

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2982: lazy initialization of HBase connection
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/2345/1/be/src/runtime/hbase-table-factory.cc
File be/src/runtime/hbase-table-factory.cc:

Line 40: connection_cl_ =
       :     env->FindClass("org/apache/hadoop/hbase/client/Connection");
> Single line.
Done


Line 52: lock_guard<mutex> lock(connection_lock_);
> You're right. I was just thinking along the lines of a concurrent GetConnec
Done


Line 91: RETURN_IF_ERROR(
       :       JniUtil::LocalToGlobalRef(env, local_connection, &connection_));
> Can fit in a single line.
Done


Line 104: if (connection_ != NULL) {
> It wouldn't matter if we're holding a lock and concurrent initialization ha
If we have multiple threads executing the destructor we've got bigger problems.

I'm realising that the lifetime of this object is messed up, it's confused about whether it's a static class or a global singleton. The destructor is reversing the effects of the static Init() method. This is really confusing but shouldn't have any adverse effects with the way it's currently used. I'm not sure if it makes sense to try to fix that in this patch.


http://gerrit.cloudera.org:8080/#/c/2345/1/be/src/runtime/hbase-table-factory.h
File be/src/runtime/hbase-table-factory.h:

Line 41:   /// Opens the connections to HBase and Zookeeper on the first call.
> Add "Returns existing connection on subsequent calls".
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I405f5cbfc62dd4a4aeb6f4019cac358376478884
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.5.0_5.7.0
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes