Skip to content

Make Engine.awaitPendingClose protected #130004

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

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Jun 25, 2025

So that the method can be used by classes implementing the Engine abstract class.

So that the method can be used by classes
implementing the Engine abstract class.
@tlrx tlrx added >non-issue :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v9.1.0 labels Jun 25, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

@elasticsearchmachine elasticsearchmachine added Team:Distributed Indexing Meta label for Distributed Indexing team v9.2.0 labels Jun 25, 2025
@@ -2199,7 +2199,7 @@ public void close() throws IOException {
awaitPendingClose();
}

private void awaitPendingClose() {
protected final void awaitPendingClose() {
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't we getting the closedLatch in the closeNoLock method parameter?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's helpful to describe how you're planning to use this method in the inheritors

Copy link
Member Author

Choose a reason for hiding this comment

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

aren't we getting the closedLatch in the closeNoLock method parameter?

Yes, but that one is final in InternalEngine

Maybe it's helpful to describe how you're planning to use this method in the inheritors

Exposing the method as package-protected in implementations, allowing its usage in tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

See the linked pull request for such an usage.

@tlrx tlrx added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 26, 2025
@elasticsearchmachine elasticsearchmachine merged commit 65788cb into elastic:main Jun 26, 2025
32 checks passed
@tlrx tlrx deleted the 2025/06/25/await-pending-close-protected branch June 26, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue Team:Distributed Indexing Meta label for Distributed Indexing team v9.1.0 v9.2.0
4 participants