----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67912/#review206075 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java Lines 248 (patched) <https://reviews.apache.org/r/67912/#comment289019> the exception that caused broken pipe is "java.net.SocketException", not "org.apache.thrift.TException". How can this fix the issue we have? should we catch all exception and close the HMSClient to be sure the connection won't be traped in bad state? - Na Li On July 13, 2018, 5:36 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67912/ > ----------------------------------------------------------- > > (Updated July 13, 2018, 5:36 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, and > Sergio Pena. > > > Bugs: SENTRY-2310 > https://issues.apache.org/jira/browse/SENTRY-2310 > > > Repository: sentry > > > Description > ------- > > If the communication between sentry and HMS goes down for any reason while > sentry is fetching full update from HMS, SentryHMSClient in HMSFollower would > be left with a reference to closed socket. As sentry is not handling the > failure and closing the SentryHMSClient, it continues using the same > SentryHMSClient. This will result in "java.net.SocketException: Broken pipe" > as the client tries to write on socket that is closed. > > Fix: Close the SentryHMSClient when there is an exception. Drawback with this > approach is that we need to make sure that this is done when ever new API's > are added to SentryHMSClient. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > 9d1a92f185ab9ddf5efa8f1b51ff054cf2cfbcd3 > > > Diff: https://reviews.apache.org/r/67912/diff/1/ > > > Testing > ------- > > Made sure that all the existing tests passed. > > > Thanks, > > kalyan kumar kalvagadda > >
