Skip to content

refactor: enhance semantic_text inference error msg #131519

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.

Sign up for GitHub

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
mromaios merged 8 commits into from
Jul 18, 2025
+18 −5
Merged

refactor: enhance semantic_text inference error msg #131519

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5cdb8b0
refactor: enhance semantic_text inference error msg
mromaios Jul 18, 2025
c42155c
use getter
mromaios Jul 18, 2025
fc0d908
[CI] Auto commit changes from spotless
Jul 18, 2025
3837942
update tests
mromaios Jul 18, 2025
c20b4a9
Merge branch 'expand_inference_error_msg' of github.com:mromaios/elas…
mromaios Jul 18, 2025
04d4e6a
[CI] Auto commit changes from spotless
Jul 18, 2025
ab023c1
Merge branch 'main' into expand_inference_error_msg
mromaios Jul 18, 2025
0d6d549
Merge branch 'main' into expand_inference_error_msg
mromaios Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .../java/org/elasticsearch/xpack/inference/action/filter/ShardBulkInferenceActionFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,9 @@ private boolean incrementIndexingPressure(IndexRequestWithIndexingPressure index
addInferenceResponseFailure(
itemIndex,
new InferenceException(
"Insufficient memory available to update source on document [" + indexRequest.getIndexRequest().id() + "]",
"Unable to insert inference results into document ["
Copy link
Contributor Author

@mromaios mromaios Jul 18, 2025

Choose a reason for hiding this comment

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

❓ Do we want to also mention update source here, or is it fine unifying the error msg?

Copy link
Contributor

@Mikep86 Mikep86 Jul 18, 2025

Choose a reason for hiding this comment

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

In this case, I think it's fine that we have a unified error message. The fact that this error is thrown prior to us actually updating source is an implementation detail we don't want to leak.

+ indexRequest.getIndexRequest().id()
+ "] due to memory pressure. Please retry the bulk request with fewer documents or smaller document sizes.",
e
)
);
Expand Down Expand Up @@ -749,7 +751,9 @@ private void applyInferenceResponses(BulkItemRequest item, FieldInferenceRespons
item.abort(
item.index(),
new InferenceException(
"Insufficient memory available to insert inference results into document [" + indexRequest.id() + "]",
"Unable to insert inference results into document ["
+ indexRequest.id()
+ "] due to memory pressure. Please retry the bulk request with fewer documents or smaller document sizes.",
e
)
);
Expand Down
11 changes: 8 additions & 3 deletions .../org/elasticsearch/xpack/inference/action/filter/ShardBulkInferenceActionFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,10 @@ public void testIndexingPressureTripsOnInferenceRequestGeneration() throws Excep
BulkItemResponse.Failure doc1Failure = doc1Response.getFailure();
assertThat(
doc1Failure.getCause().getMessage(),
containsString("Insufficient memory available to update source on document [doc_1]")
containsString(
"Unable to insert inference results into document [doc_1]"
+ " due to memory pressure. Please retry the bulk request with fewer documents or smaller document sizes."
)
);
assertThat(doc1Failure.getCause().getCause(), instanceOf(EsRejectedExecutionException.class));
assertThat(doc1Failure.getStatus(), is(RestStatus.TOO_MANY_REQUESTS));
Expand Down Expand Up @@ -791,7 +794,8 @@ public void testIndexingPressureTripsOnInferenceResponseHandling() throws Except
BulkItemResponse.Failure doc1Failure = doc1Response.getFailure();
assertThat(
doc1Failure.getCause().getMessage(),
containsString("Insufficient memory available to insert inference results into document [doc_1]")
containsString("Unable to insert inference results into document [doc_1]"
+ " due to memory pressure. Please retry the bulk request with fewer documents or smaller document sizes.")
);
assertThat(doc1Failure.getCause().getCause(), instanceOf(EsRejectedExecutionException.class));
assertThat(doc1Failure.getStatus(), is(RestStatus.TOO_MANY_REQUESTS));
Expand Down Expand Up @@ -902,7 +906,8 @@ public void testIndexingPressurePartialFailure() throws Exception {
BulkItemResponse.Failure doc2Failure = doc2Response.getFailure();
assertThat(
doc2Failure.getCause().getMessage(),
containsString("Insufficient memory available to insert inference results into document [doc_2]")
containsString("Unable to insert inference results into document [doc_2]"
+ " due to memory pressure. Please retry the bulk request with fewer documents or smaller document sizes.")
);
assertThat(doc2Failure.getCause().getCause(), instanceOf(EsRejectedExecutionException.class));
assertThat(doc2Failure.getStatus(), is(RestStatus.TOO_MANY_REQUESTS));
Expand Down