----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46402/#review129743 -----------------------------------------------------------
Fix it, then Ship it! Looks pretty good. A couple of minor issues, below. geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/ShutDownFunction.java (line 59) <https://reviews.apache.org/r/46402/#comment193286> I hate that gfsh swallows stack traces. Maybe at least log the stack trace so that someone has a hope of tracking down what went wrong? geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/functions/ShutDownFunction.java (line 78) <https://reviews.apache.org/r/46402/#comment193287> This catch block is redundant. - Dan Smith On April 20, 2016, 1:56 p.m., Jens Deppe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46402/ > ----------------------------------------------------------- > > (Updated April 20, 2016, 1:56 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 > >
