----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46402/#review129628 -----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/ShutDownFunction.java (line 50) <https://reviews.apache.org/r/46402/#comment193064> Since getConnectedInstance and returns an IDS that is connected this extra check here for isConnected is not needed. geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/ShutDownFunction.java (line 61) <https://reviews.apache.org/r/46402/#comment193063> It is not obvious why this is done in its own thread. What do you think adding a comment to startShutdownThread explaining why you need to do the disconnect call in a non-daemon thread. Also I think you should do all the thread work in the method; don't return the thread and join on it. Instead do the join in this method. And you could call the method disconnectInNonDaemonThread. - Darrel Schneider On April 19, 2016, 12:55 p.m., Jens Deppe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46402/ > ----------------------------------------------------------- > > (Updated April 19, 2016, 12:55 p.m.) > > > Review request for geode, Darrel Schneider, Jinmei Liao, Kirk Lund, and Dan > Smith. > > > Repository: geode > > > Description > ------- > > - This fixes two issues when using gfsh 'shutdown' command > - One is that the JVM can exit prematurely because all remaining threads > are daemon threads. When coupled with network partition detection this > can result in member departed events causing split brain scenarios - > [GEODE-1236]. > - The other issue is that when a member is starting up it may have > synchronized on the CacheFactory class waiting on disk store recovery. > This prevented gfsh shutdown to run as it would also try and > synchronize on the CacheFactory and be blocked. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/ShutDownFunction.java > 90b582b778e4df2b4b6b6cda045d83d9098d8c56 > > Diff: https://reviews.apache.org/r/46402/diff/ > > > Testing > ------- > > > Thanks, > > Jens Deppe > >
