----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56892/#review166290 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 276) <https://reviews.apache.org/r/56892/#comment238221> can you just do if (client == null) { return; } sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 278) <https://reviews.apache.org/r/56892/#comment238223> It would be good to log the cases when we actually closing the client sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 279) <https://reviews.apache.org/r/56892/#comment238219> If close() generates exception, we don't sent client to null and don't clean kerberos context. Can this be refactored so that we always set client to null and kerberos context to null even if we get exception? We may I don't want to have closed client or kereros context lying around. - Alexander Kolbasov On Feb. 21, 2017, 4:03 p.m., Vamsee Yarlagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56892/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2017, 4:03 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar > kalvagadda. > > > Repository: sentry > > > Description > ------- > > Looking at the code of HMSFollower, we seem to be establishing HMS connection > for all servers of Sentry servers irrespective of whether they are leader or > not. Whereas the subsequent operations of HMSFollower are only limited to the > leader of the Sentry group. To avoid extra connections, we can limit only the > leader to get a connection to HMS. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > f88f6f17e746f6dd4db8b8f4679a2dc9aa1e4ee0 > > Diff: https://reviews.apache.org/r/56892/diff/ > > > Testing > ------- > > > Thanks, > > Vamsee Yarlagadda > >
