> On Dec. 16, 2016, 7:05 p.m., Vadim Spector wrote: > > It is probably ok, but ... > > a) Did you investigate why service stayed alive? Perhaps, some knowledge > > about the implementation and its defficiencies to be gained there. Some > > unaccounted runaway threads? Or are we unable to termonaie some threads? > > b) System.exit() means that Sentry service better be the only thing running > > in JVM. Which I presume is the case now. Are we ok with making that a > > requirement from now on? > > Alexander Kolbasov wrote: > For a) please see SENTRY-1526. The service stayed alive because of the > deadlock. > b) I don't understand the question - what else can run in this JVM? This > is a Sentry Server that we are trying to kill.
a) Can the deadlock be fixed instead? SENTRY-1526 seem to suggest that the choice of synchronization objects can be changed to prevent the deadlock. It still does not explain why we need to use System.exit(). b) Architecturally, there is nothing insane about running Sentry Service embedded inside some kind of container in the future. We may not need it now, but the proposed implementation makes this impossible moving forward, therefore my concern. - Vadim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54808/#review159478 ----------------------------------------------------------- On Dec. 16, 2016, 7:23 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54808/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2016, 7:23 a.m.) > > > Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Vamsee > Yarlagadda, and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > SENTRY-1526 Sentry processed stays alive after being killed > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 578364933a3cdcf6c142b836360a83d322fe5c11 > > Diff: https://reviews.apache.org/r/54808/diff/ > > > Testing > ------- > > Now process successfully dies after hitting ^C. > > > Thanks, > > Alexander Kolbasov > >
