[Inference API] Check pipelines on delete inference endpoint #109123

maxhniebergall · 2024-05-28T17:04:08Z

This PR adds the requirement that an inference endpoint not be referenced by an InferenceProcessor IngestProcessors in an ingest pipeline. This PR also adds two new options to the Delete inference endpoint API: dry_runand force. dry_run:true will return a list of ingest processors which reference this inference endpoint. force:true will delete the inference endpoint regardless of whether or not it is referenced by ingest processors.

elasticsearchmachine · 2024-05-28T17:04:32Z

Hi @maxhniebergall, I've created a changelog YAML for you.

…hub.com/elastic/elasticsearch into checkPipelinesOnDeleteInferenceEndpoint

maxhniebergall · 2024-05-28T19:31:08Z

...c/main/java/org/elasticsearch/xpack/core/inference/action/DeleteInferenceEndpointAction.java

+import java.util.Objects;
+import java.util.Set;
+
+public class DeleteInferenceEndpointAction extends ActionType<AcknowledgedResponse> {


this file already existed, it was just renamed

maxhniebergall · 2024-05-28T19:31:25Z

...c/main/java/org/elasticsearch/xpack/core/inference/action/DeleteInferenceEndpointAction.java

+        }
+    }
+
+    public static class Response extends AcknowledgedResponse {


the main changes to this file are here, in the Response

maxhniebergall · 2024-05-28T19:32:12Z

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

+
+import java.util.Set;
+
+public class TransportDeleteInferenceEndpointAction extends AcknowledgedTransportMasterNodeAction<DeleteInferenceEndpointAction.Request> {


this file was renamed

maxhniebergall · 2024-05-28T19:32:25Z

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

+            if (request.isDryRun()) {
+                masterListener.onResponse(
+                    new DeleteInferenceEndpointAction.Response(
+                        false,
+                        InferenceProcessorInfoExtractor.pipelineIdsForResource(state, Set.of(request.getInferenceEndpointId()))
+                    )
+                );
+                return;
+            } else if (request.isForceDelete() == false
+                && endpointIsReferencedInPipelines(state, request.getInferenceEndpointId(), listener)) {
+                    return;
+                }


this is the main change in this file

maxhniebergall · 2024-05-28T19:33:45Z

...e/src/main/java/org/elasticsearch/xpack/inference/utils/InferenceProcessorInfoExtractor.java

+ * this class was duplicated from org.elasticsearch.xpack.ml.utils.InferenceProcessorInfoExtractor
+ */
+
+public final class InferenceProcessorInfoExtractor {


I couldn't figure out how to import this function from the ML module without causing Jarr hell, so I just duplicated it here.

I tried moving org.elasticsearch.xpack.ml.utils.InferenceProcessorInfoExtractor to package org.elasticsearch.xpack.core.ml.utils and it seems like I can reference it from both the inference plugin and ml. I didn't replace all the ml plugin's references though. I did have to hardcode the TYPE like you mentioned below (maybe that could be moved too, I didn't look too hard).

Does moving it there cause issues for you?

I did try that initially and it caused Jar hell, so I am hesitant to try that again. It seems like it should work, but for some reason it didn't work when I tried it.

# Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java

elasticsearchmachine · 2024-05-28T19:43:11Z

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

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

.../src/main/java/org/elasticsearch/xpack/inference/rest/RestDeleteInferenceEndpointAction.java

jonathan-buttner · 2024-05-28T21:24:34Z

...e/src/main/java/org/elasticsearch/xpack/inference/utils/InferenceProcessorInfoExtractor.java

+ * this class was duplicated from org.elasticsearch.xpack.ml.utils.InferenceProcessorInfoExtractor
+ */
+
+public final class InferenceProcessorInfoExtractor {


I tried moving org.elasticsearch.xpack.ml.utils.InferenceProcessorInfoExtractor to package org.elasticsearch.xpack.core.ml.utils and it seems like I can reference it from both the inference plugin and ml. I didn't replace all the ml plugin's references though. I did have to hardcode the TYPE like you mentioned below (maybe that could be moved too, I didn't look too hard).

Does moving it there cause issues for you?

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

timgrein

Left some smaller comments, cool stuff! 👏

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

...e/src/main/java/org/elasticsearch/xpack/inference/utils/InferenceProcessorInfoExtractor.java

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

jonathan-buttner · 2024-05-29T18:34:13Z

...in/java/org/elasticsearch/xpack/inference/action/TransportDeleteInferenceEndpointAction.java

+            if (modelIdsReferencedByPipelines.contains(inferenceEndpointId)) {
+                listener.onFailure(
+                    new ElasticsearchStatusException(
+                        "Model "


Should this say Inference endpoint instead of Model?

I think I'll do a follow up PR that more thoroughly replaces model in the module

…y are referenced by semantic text (#110399) Following on #109123

maxhniebergall added 7 commits May 28, 2024 11:25

renaming

aa28a7e

check for pipeline references

a7727b6

add force and dry run options to request

a58ff8f

add force delete and dry run options to Actions

ac64d14

Revert dependency on ml module

290082e

duplicate InferenceProcessorInfoExtractor to inference module

31d57bf

Ignore missing IngestMetadata during attempted endpoint delete

4d58493

maxhniebergall added >enhancement :ml Machine learning v8.15.0 labels May 28, 2024

maxhniebergall and others added 6 commits May 28, 2024 13:04

Update docs/changelog/109123.yaml

8ce76b8

rename RestDeleteInference Action

9318e22

fix dry run

3929cfd

Add integration test and misc improvements

e5e62ef

Merge branch 'checkPipelinesOnDeleteInferenceEndpoint' of https://git…

da3c3e2

…hub.com/elastic/elasticsearch into checkPipelinesOnDeleteInferenceEndpoint

Update 109123.yaml

fcbf4c5

maxhniebergall commented May 28, 2024

View reviewed changes

maxhniebergall requested review from jonathan-buttner May 28, 2024 19:34

Merge branch 'main' into checkPipelinesOnDeleteInferenceEndpoint

9cdbc52

# Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java

maxhniebergall marked this pull request as ready for review May 28, 2024 19:42

elasticsearchmachine added the Team:ML Meta label for the ML team label May 28, 2024

merge fixes

b7c3a1c

jonathan-buttner reviewed May 28, 2024

View reviewed changes

timgrein reviewed May 29, 2024

View reviewed changes

maxhniebergall self-assigned this May 29, 2024

Improvements from PR review

73c9135

jonathan-buttner reviewed May 29, 2024

View reviewed changes

jonathan-buttner and others added 4 commits May 29, 2024 15:51

Moving shared pieces to core (#109183)

e688dea

replace model in exception message with inference endpoint

038918c

Merge branch 'main' into checkPipelinesOnDeleteInferenceEndpoint

30d4071

test fix

d1faacb

jonathan-buttner approved these changes May 30, 2024

View reviewed changes

maxhniebergall merged commit 6532af8 into main May 31, 2024
16 checks passed

maxhniebergall deleted the checkPipelinesOnDeleteInferenceEndpoint branch May 31, 2024 14:14

davidkyle mentioned this pull request Jun 5, 2024

[ML] Fix serialising inference delete response #109384

Merged

This was referenced Jun 5, 2024

[ML] Update Delete Inference Endpoint for dry_run and force options elastic/elasticsearch-specification#2602

Merged

[ML] Add dry run and force to json spec for Delete Inference endpoint #109402

Merged

maxhniebergall mentioned this pull request Jul 2, 2024

[Inference API] Prevent inference endpoints from being deleted if they are referenced by semantic text #110399

Merged

elasticsearchmachine pushed a commit that referenced this pull request Jul 3, 2024

[Inference API] Prevent inference endpoints from being deleted if the…

89a1bd9

…y are referenced by semantic text (#110399) Following on #109123

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Inference API] Check pipelines on delete inference endpoint #109123

[Inference API] Check pipelines on delete inference endpoint #109123

maxhniebergall commented May 28, 2024

elasticsearchmachine commented May 28, 2024

maxhniebergall May 28, 2024

maxhniebergall May 28, 2024

maxhniebergall May 28, 2024

maxhniebergall May 28, 2024

maxhniebergall May 28, 2024

jonathan-buttner May 28, 2024

maxhniebergall May 29, 2024

elasticsearchmachine commented May 28, 2024

jonathan-buttner May 28, 2024

timgrein left a comment

jonathan-buttner May 29, 2024

maxhniebergall May 30, 2024


		import java.util.Set;

		public class TransportDeleteInferenceEndpointAction extends AcknowledgedTransportMasterNodeAction<DeleteInferenceEndpointAction.Request> {

[Inference API] Check pipelines on delete inference endpoint #109123

[Inference API] Check pipelines on delete inference endpoint #109123

Conversation

maxhniebergall commented May 28, 2024

elasticsearchmachine commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticsearchmachine commented May 28, 2024

Choose a reason for hiding this comment

timgrein left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment