Skip to content

Clean up Node#close. (#39317)#41301

Merged
jpountz merged 1 commit intoelastic:7.xfrom
jpountz:backport/39317/7.x
Apr 17, 2019
Merged

Clean up Node#close. (#39317)#41301
jpountz merged 1 commit intoelastic:7.xfrom
jpountz:backport/39317/7.x

Conversation

@jpountz
Copy link
Copy Markdown
Contributor

@jpountz jpountz commented Apr 17, 2019

Node#close is pretty hard to rely on today:

  • it might swallow exceptions
  • it waits for 10 seconds for threads to terminate but doesn't signal anything
    if threads are still not terminated after 10 seconds

This commit makes IOExceptions propagated and splits Node#close into
Node#close and Node#awaitClose so that the decision what to do if a node
takes too long to close can be done on top of Node#close.

It also adds synchronization to lifecycle transitions to make them atomic. I
don't think it is a source of problems today, but it makes things easier to
reason about.

`Node#close` is pretty hard to rely on today:
 - it might swallow exceptions
 - it waits for 10 seconds for threads to terminate but doesn't signal anything
   if threads are still not terminated after 10 seconds

This commit makes `IOException`s propagated and splits `Node#close` into
`Node#close` and `Node#awaitClose` so that the decision what to do if a node
takes too long to close can be done on top of `Node#close`.

It also adds synchronization to lifecycle transitions to make them atomic. I
don't think it is a source of problems today, but it makes things easier to
reason about.
@jpountz jpountz merged commit 9fd5237 into elastic:7.x Apr 17, 2019
@jpountz jpountz deleted the backport/39317/7.x branch April 17, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant