impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-2.5.0_5.7.0) IMPALA-2982: lazy initialization of HBase connection
Date Mon, 29 Feb 2016 19:26:18 GMT
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

Mime
View raw message