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 <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
