Skip to content

Include failures in partial response #124929

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

Merged
merged 5 commits into from
Mar 16, 2025
Merged

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Mar 14, 2025

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 Include failures in partial response Mar 15, 2025
@elasticsearchmachine
Copy link
Collaborator

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
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

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

💔 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
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
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
This change includes failures when ESQL returns partial results. It also
carries failures between cluster requests.

Relates #122802
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

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
* 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
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request Mar 21, 2025
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Mar 21, 2025
@mark-vieira
Copy link
Contributor

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 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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants