Skip to content

Add more logging around testStopQueryLocal to see what's going on #124911

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

Jump to bottom
Merged
merged 3 commits into from
Mar 14, 2025
Merged

Add more logging around testStopQueryLocal to see what's going on #124911

merged 3 commits into from
Mar 14, 2025

Conversation

@smalyshev
Copy link
Contributor

@smalyshev smalyshev commented Mar 14, 2025 •
edited
Loading

Also adds a bit more info to EsqlExecutionInfo.toString

@smalyshev smalyshev added >test Issues or PRs that are addressing/adding tests auto-backport Automatically create backport pull requests when merged v8.19.0 :Search Foundations/CCS labels Mar 14, 2025
@smalyshev smalyshev marked this pull request as ready for review March 14, 2025 18:13
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Mar 14, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 14, 2025

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@smalyshev smalyshev requested a review from quux00 March 14, 2025 18:26
pawankartik-elastic
pawankartik-elastic approved these changes Mar 14, 2025
View reviewed changes
Copy link
Contributor

@pawankartik-elastic pawankartik-elastic left a comment

Choose a reason for hiding this comment

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

Nothing blocking.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlExecutionInfo.java
@Override
public String toString() {
return "EsqlExecutionInfo{" + "overallTook=" + overallTook + ", clusterInfo=" + clusterInfo + '}';
return "EsqlExecutionInfo{"
Copy link
Contributor

@pawankartik-elastic pawankartik-elastic Mar 14, 2025

Choose a reason for hiding this comment

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

I hope this doesn't have any unintended side-effects (it shouldn't, TBH).

Copy link
Contributor Author

@smalyshev smalyshev Mar 14, 2025

Choose a reason for hiding this comment

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

We shouldn't rely on these in tests - or anywhere more important - but if something will pop up I'll fix it. We added some stuff to EsqlExecutionInfo but never updated this, and now I need it for logging.

...ternalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterAsyncQueryStopIT.java
populateRuntimeIndex(LOCAL_CLUSTER, "pause", INDEX_WITH_BLOCKING_MAPPING);

Tuple<Boolean, Boolean> includeCCSMetadata = randomIncludeCCSMetadata();
boolean responseExpectMeta = includeCCSMetadata.v2();
Copy link
Contributor

@pawankartik-elastic pawankartik-elastic Mar 14, 2025

Choose a reason for hiding this comment

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

Any specific reason?

Copy link
Contributor Author

@smalyshev smalyshev Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's never actually used. I just noticed it now.

@smalyshev smalyshev enabled auto-merge (squash) March 14, 2025 21:22
@smalyshev smalyshev merged commit aa2db74 into elastic:main Mar 14, 2025
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 14, 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 124911

smalyshev added a commit to smalyshev/elasticsearch that referenced this pull request Mar 14, 2025
@smalyshev
Add more logging around testStopQueryLocal to see what's going on (el…
e5f9a8a 
…astic#124911)

(cherry picked from commit aa2db74)

# Conflicts:
#	muted-tests.yml
@smalyshev smalyshev mentioned this pull request Mar 14, 2025
Merged
@smalyshev
Copy link
Contributor Author

smalyshev commented Mar 14, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation

elasticsearchmachine pushed a commit that referenced this pull request Mar 15, 2025
@smalyshev
Add more logging around testStopQueryLocal to see what's going on (#1…
2a591a6 
…24911) (#124932)

(cherry picked from commit aa2db74)

# Conflicts:
#	muted-tests.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pawankartik-elastic pawankartik-elastic pawankartik-elastic approved these changes

@quux00 quux00 Awaiting requested review from quux00

Assignees

No one assigned

Labels

auto-backport Automatically create backport pull requests when merged :Search Foundations/CCS Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smalyshev @elasticsearchmachine @pawankartik-elastic