node shutdown: make close() syncronized
An example scenario where this will help: When the node is shutdown via api call (https://github.com/elasticsearch/elasticsearch/blob/master/src/test/java/org/elasticsearch/test/ExternalNode.java#L219 ) then the call returns immediately even if the node is not actually shutdown yet (https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/action/admin/cluster/node/shutdown/TransportNodesShutdownAction.java#L226). If at the same time the proces is killed, then the hook that would usually prevent uncontrolled shutdown (https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java#L75) has no effect: It again calls close() which might then just return for example because one of the lifecycles was moved to closed already. The bwc test FunctionScoreBackwardCompatibilityTests.testSimpleFunctionScoreParsingWorks failed because of this. The translog was not properly written because if the shutdown was called via api, the following process.destroy() (https://github.com/elasticsearch/elasticsearch/blob/master/src/test/java/org/elasticsearch/test/ExternalNode.java#L225) killed the node before the translog was written to disk. closes #7885
This commit is contained in:
parent
36c3e896de
commit
bac1da25f6
|
@ -306,7 +306,10 @@ public final class InternalNode implements Node {
|
|||
return this;
|
||||
}
|
||||
|
||||
public void close() {
|
||||
// During concurrent close() calls we want to make sure that all of them return after the node has completed it's shutdown cycle.
|
||||
// If not, the hook that is added in Bootstrap#setup() will be useless: close() might not be executed, in case another (for example api) call
|
||||
// to close() has already set some lifecycles to stopped. In this case the process will be terminated even if the first call to close() has not finished yet.
|
||||
public synchronized void close() {
|
||||
if (lifecycle.started()) {
|
||||
stop();
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue