Skip to content

ESQL: Fix limit task test#117270

Merged
nik9000 merged 5 commits intoelastic:mainfrom
nik9000:fix_107293
Nov 22, 2024
Merged

ESQL: Fix limit task test#117270
nik9000 merged 5 commits intoelastic:mainfrom
nik9000:fix_107293

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Nov 21, 2024

Fix a test for the task results when running the LIMIT operation. We were releasing a few permits to get the query started. And when you combine that with the page worth of permits that the test was releasing we'd sometimes finish the entire limited query, stopping the task too early to find a running task.

Closes #107293

Fix a test for the task results when running the `LIMIT` operation. We
were releasing a few permits to get the query started. And when you
combine that with the page worth of permits that the test was releasing
we'd sometimes finish the entire limited query, stopping the task too
early to find a running task.

Closes elastic#107293
@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL v9.0.0 v8.18.0 labels Nov 21, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Nov 21, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@nik9000 nik9000 requested a review from alex-spies November 21, 2024 18:15
@nik9000 nik9000 added the auto-backport Automatically create backport pull requests when merged label Nov 21, 2024
Copy link
Copy Markdown
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Nik!

private ActionFuture<EsqlQueryResponse> startEsql(String query) {
scriptPermits.drainPermits();
scriptPermits.release(between(1, 5));
prereleasedDocs = between(1, 5);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The number 5 here seems a bit magical. Can we express this in terms of pageSize so it's clear that this never covers all pages?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

YEah!

@nik9000 nik9000 enabled auto-merge (squash) November 22, 2024 17:27
@nik9000 nik9000 merged commit 34d9652 into elastic:main Nov 22, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💚 Backport successful

Status Branch Result
8.x

nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Nov 22, 2024
Fix a test for the task results when running the `LIMIT` operation. We
were releasing a few permits to get the query started. And when you
combine that with the page worth of permits that the test was releasing
we'd sometimes finish the entire limited query, stopping the task too
early to find a running task.

Closes elastic#107293
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
Fix a test for the task results when running the `LIMIT` operation. We
were releasing a few permits to get the query started. And when you
combine that with the page worth of permits that the test was releasing
we'd sometimes finish the entire limited query, stopping the task too
early to find a running task.

Closes elastic#107293
nik9000 added a commit that referenced this pull request Dec 2, 2024
Fix a test for the task results when running the `LIMIT` operation. We
were releasing a few permits to get the query started. And when you
combine that with the page worth of permits that the test was releasing
we'd sometimes finish the entire limited query, stopping the task too
early to find a running task.

Closes #107293
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 Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] EsqlActionTaskIT testTaskContentsForLimitQuery failing

4 participants