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