Skip to content
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

[ML] Close inference services #104726

Merged

Conversation

jonathan-buttner
Copy link
Contributor

This PR cleans up some logic and adds functionality to close the inference services when the InferencePlugin::close() is called. This removes the need to keep a reference to the HttpClientManager since it'll be closed by the individual services. This helps prepare the batching logic to have a separate http pool for searching and ingest.

@jonathan-buttner jonathan-buttner added >non-issue :ml Machine learning Team:ML Meta label for the ML team v8.13.0 labels Jan 24, 2024
@@ -20,7 +20,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

public class InferenceServiceRegistry extends AbstractLifecycleComponent {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe we need this to be an AbstractLifecycleComponent anymore since it's not part of the NodeConstruction class right?

@maxhniebergall
Copy link
Member

What is the purpose of close() in inference services? Is it just for closing http connections?

@jonathan-buttner
Copy link
Contributor Author

What is the purpose of close() in inference services? Is it just for closing http connections?

Yeah and we have some background threads that run and it shuts those down as well.

@jonathan-buttner
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-1

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathan-buttner jonathan-buttner marked this pull request as ready for review January 25, 2024 13:28
@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Member

@maxhniebergall maxhniebergall left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathan-buttner jonathan-buttner merged commit 05c7377 into elastic:main Jan 25, 2024
15 checks passed
@jonathan-buttner jonathan-buttner deleted the ml-infer-close-services branch January 25, 2024 14:38
henningandersen pushed a commit to henningandersen/elasticsearch that referenced this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants