Skip to content

Do not release safe commit with CancellableThreads #59182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 8, 2020

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Jul 7, 2020

We are leaking a FileChannel in #39585 if we release a safe commit with CancellableThreads. Although it is a bug in Lucene where we do not close a FileChannel if we failed to create a NIOFSIndexInput, I think it's safer if we release a safe commit using the generic thread pool instead.

Closes #39585
Relates #45409

@dnhatn dnhatn added >non-issue :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v8.0.0 v7.9.0 v7.8.2 labels Jul 7, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Recovery)

@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Jul 7, 2020
Copy link
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks Nhat :) could we add a reasonable TODO that asks to simplify things again once the Lucene situation is fixed maybe?

@dnhatn
Copy link
Member Author

dnhatn commented Jul 8, 2020

@original-brownbear Thanks for reviewing. I've added a comment explaining why we prefer releasing a safe commit using the generic thread pool (see ff0f8f8).

@dnhatn
Copy link
Member Author

dnhatn commented Jul 8, 2020

testSendSnapshotSendsOps was fixed in 865b6b5.

@dnhatn dnhatn added >bug and removed >non-issue labels Jul 8, 2020
@dnhatn
Copy link
Member Author

dnhatn commented Jul 8, 2020

@elasticmachine update branch

@dnhatn dnhatn merged commit 5688e0e into elastic:master Jul 8, 2020
@dnhatn dnhatn deleted the release-commit-cancellable branch July 8, 2020 17:24
dnhatn added a commit that referenced this pull request Jul 8, 2020
We are leaking a FileChannel in #39585 if we release a safe commit with 
CancellableThreads. Although it is a bug in Lucene where we do not close
a FileChannel if we failed to create a NIOFSIndexInput, I think it's
safer if we release a safe commit using the generic thread pool instead.

Closes #39585
Relates #45409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.9.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI failures due to file handle leaks
4 participants