Skip to content

Do not release safe commit with CancellableThreads#59182

Merged
dnhatn merged 5 commits intoelastic:masterfrom
dnhatn:release-commit-cancellable
Jul 8, 2020
Merged

Do not release safe commit with CancellableThreads#59182
dnhatn merged 5 commits intoelastic:masterfrom
dnhatn:release-commit-cancellable

Conversation

@dnhatn
Copy link
Copy Markdown
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/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
Copy Markdown
Collaborator

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

@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Jul 7, 2020
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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/Recovery Anything around constructing a new shard, either from a local or a remote source. Team:Distributed Meta label for distributed team. 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