Skip to content

Include failures in partial response#124929

Merged
dnhatn merged 5 commits intoelastic:mainfrom
dnhatn:return-partial-failures
Mar 16, 2025
Merged

Include failures in partial response#124929
dnhatn merged 5 commits intoelastic:mainfrom
dnhatn:return-partial-failures

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented Mar 14, 2025 •
edited
Loading

This change includes failures when ESQL returns partial results. It also carries failures between cluster requests.

Relates #122802
@dnhatn dnhatn force-pushed the return-partial-failures branch 2 times, most recently from b71ca6b to 7536a7f Compare March 15, 2025 00:16
@dnhatn dnhatn force-pushed the return-partial-failures branch from 7536a7f to 6fd6697 Compare March 15, 2025 03:53
@dnhatn dnhatn force-pushed the return-partial-failures branch from 6fd6697 to b164f25 Compare March 15, 2025 04:54
@dnhatn dnhatn changed the title Return partial failures Mar 15, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 15, 2025

Hi @dnhatn, I've created a changelog YAML for you.
@dnhatn dnhatn added the auto-backport Automatically create backport pull requests when merged label Mar 15, 2025
@dnhatn dnhatn mentioned this pull request Mar 15, 2025
Open
10 tasks
@dnhatn dnhatn marked this pull request as ready for review March 15, 2025 18:44
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 15, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 15, 2025

Pinging @elastic/es-analytical-engine (Team:Analytics)
@dnhatn
Copy link
Member Author

dnhatn commented Mar 16, 2025

Thanks @smalyshev.
@dnhatn dnhatn merged commit 6b6fc80 into elastic:main Mar 16, 2025
17 checks passed
@dnhatn dnhatn deleted the return-partial-failures branch March 16, 2025 18:44
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 16, 2025

💔 Backport failed

Status Branch Result
❌ 8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 124929
@dnhatn dnhatn mentioned this pull request Mar 17, 2025
Merged
@dnhatn
Copy link
Member Author

dnhatn commented Mar 17, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Mar 17, 2025
@dnhatn
Include failures in partial response (elastic#124929)
c694ae3 
This change includes failures when ESQL returns partial results. It also
carries failures between cluster requests.

Relates elastic#122802
dnhatn added a commit that referenced this pull request Mar 17, 2025
@dnhatn
Include failures in partial response (#124929)
3fea0b6 
This change includes failures when ESQL returns partial results. It also
carries failures between cluster requests.

Relates #122802
idegtiarenko
idegtiarenko reviewed Mar 17, 2025
View reviewed changes
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestSender.java
final Set<Exception> seen = Collections.newSetFromMap(new IdentityHashMap<>());
for (Map.Entry<ShardId, ShardFailure> e : shardFailures.entrySet()) {
final ShardFailure failure = e.getValue();
if (ExceptionsHelper.unwrap(failure.failure(), TaskCancelledException.class) != null) {
Copy link
Contributor

@idegtiarenko idegtiarenko Mar 17, 2025

Choose a reason for hiding this comment

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

I wonder if it is worth keeping unwrapped exception in trackShardLevelFailure?
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Mar 17, 2025
@weizijun
Merge branch 'main' into add_num_candidates_a_options
2902989 
* main: (95 commits)
  Mute org.elasticsearch.datastreams.lifecycle.DataStreamLifecycleServiceIT testLifecycleAppliedToFailureStore elastic#124999
  Merge template mappings properly during validation (elastic#124784)
  [Build] Rework internal build plugin plugin to work with Isolated Projects (elastic#123461)
  [Build] Require reason for usesDefaultDistribution (elastic#124707)
  Mute org.elasticsearch.packaging.test.DockerTests test011SecurityEnabledStatus elastic#124990
  Mute org.elasticsearch.xpack.ilm.TimeSeriesDataStreamsIT testRolloverAction elastic#124987
  Mute org.elasticsearch.packaging.test.BootstrapCheckTests test10Install elastic#124957
  Mute org.elasticsearch.integration.DataStreamLifecycleServiceRuntimeSecurityIT testRolloverLifecycleAndForceMergeAuthorized elastic#124978
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQuery elastic#124977
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQueryLocal elastic#121672
  Mention zero-window state in networking docs (elastic#124967)
  Remove remoteAddress field from TransportResponse (elastic#120016)
  Include failures in partial response (elastic#124929)
  Prevent work starvation bug if using scaling EsThreadPoolExecutor with core pool size = 0  (elastic#124732)
  Re-enable analysis stemmer test (elastic#124961)
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQueryLocalNoRemotes elastic#124959
  ESQL: Catch parsing exception (elastic#124958)
  ESQL: Improve error message for ( and [ (elastic#124177)
  Mute org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT test {lookup-join.MvJoinKeyFromRow SYNC} elastic#124951
  Mute org.elasticsearch.datastreams.lifecycle.DataStreamLifecycleServiceIT testErrorRecordingOnRetention elastic#124950
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
#	server/src/test/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldTypeTests.java
@dnhatn dnhatn mentioned this pull request Mar 20, 2025
Merged
elasticsearchmachine pushed a commit that referenced this pull request Mar 20, 2025
@dnhatn
Adjust bwc for ES|QL partial failures from remote (#125279)
5a73689 
Relates #124929
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request Mar 21, 2025
@dnhatn
Adjust bwc for ES|QL partial failures from remote (elastic#125279)
4b0dea3 
Relates elastic#124929
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Mar 21, 2025
@dnhatn @smalyshev
Adjust bwc for ES|QL partial failures from remote (elastic#125279)
69db7d5 
Relates elastic#124929
@mark-vieira
Copy link
Contributor

mark-vieira commented Mar 27, 2025

Why was this backported to 8.x and not 9.0?
@dnhatn
Copy link
Member Author

dnhatn commented Mar 27, 2025

Why was this backported to 8.x and not 9.0?

@mark-vieira Two reasons why we didn’t backport this to 9.0: (1) it requires wire-compact, and (2) this enhancement has no impact on 9.0, where we don't support partial results.
@mark-vieira
Copy link
Contributor

mark-vieira commented Mar 27, 2025

@mark-vieira Two reasons why we didn’t backport this to 9.0: (1) it requires wire-compact, and (2) this enhancement has no impact on 9.0, where we don't support partial results.

Gotcha, is that common to skip branches like this? I hit this when doing some backports and found it awkward that there was a "gap" here.
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
@dnhatn @omricohenn
Include failures in partial response (elastic#124929)
677113e 
This change includes failures when ESQL returns partial results. It also 
carries failures between cluster requests.

Relates elastic#122802
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
@dnhatn @omricohenn
Adjust bwc for ES|QL partial failures from remote (elastic#125279)
c01bf85 
Relates elastic#124929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0

5 participants

@dnhatn @elasticsearchmachine @mark-vieira @smalyshev @idegtiarenko