impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (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 07:53:44 GMT
Sailesh Mukil has posted comments on this change.

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


Patch Set 1:

(2 comments)

How was this tested? I think Alex's concerns are valid about making sure not to introduce
a regression.

Quoting his questions:
"For example, will it be possible to restart the HBase and Impala services independently?
Did that even work before? What about partial service restarts? My point is that to me it
does not seem trivial to fix this issue while being absolutely sure there are no behavioral
regressions."

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

Line 52: lock_guard<mutex> lock(connection_lock_);
It doesn't seem to me like a mutex is really of too much help here. It wouldn't protect against
'connection_' getting NULLed out after this function returns.

Could we rather just do an atomic get() to copy over the connection here?


Line 104: if (connection_ != NULL) {
And do an atomic swap() to NULL out connection here, and store it's value locally and call
DeleteGlobalRef() on the stored value.


-- 
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-HasComments: Yes

Mime
View raw message