> On Feb. 22, 2017, 1:48 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > line 284 > > <https://reviews.apache.org/r/56892/diff/1/?file=1640969#file1640969line284> > > > > 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.
After checking the code of client.close(), I noticed that it cannot throw an exception to the user as all the associated exceptions were being caught in the method definition itself. Anyway I refactored the code to set client, kerberosContext to null irrespective of the error it encounters. - Vamsee ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56892/#review166290 ----------------------------------------------------------- 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 > >
