ESQL: Refactor value reading so it can split Pages #130573

nik9000 · 2025-07-03T16:58:35Z

This refactors our ValuesSourceReaderOperator so it can split pages
when it reads large values. It does not actually split the pages as
that's a bit tricky. But it sets the stage for the next PR that will
do so.

Move ValuesSourceReaderOperator to it's own package
Move many inner classes into their own top level classes
Extend from AbstractPageMappingToIteratorOperator instead of
AbstractPageMappingToOperator
- This allows returning more than one Page per input Page
- In this PR we still always return one Page per input Page
- Make new ReleasableIterator subclasses to satisfy
  AbstractPageMappingToIteratorOperator
- Change status of loading fields from pages_processed to
  pages_received and pages_emitted
Fix a bug in AbstractPageMappingToOperator which can leak circuit
breaker allocation if we fail to during receive. This isn't possible
in the existing implementations but is possible
in ValuesSourceReaderOperator.
Add a test with large text fields. Right now it still comes back in
one page because we don't cut the pages.

Closes #130727

elasticsearchmachine · 2025-07-03T16:59:08Z

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

nik9000 · 2025-07-03T16:59:26Z

This includes a transport change so the new status so it's going to be "fun" to backport, but I'd still like to get this fixed in 8.19.x. So I will backport it. But not automatically.

nik9000 · 2025-07-03T18:54:55Z

...ute/src/test/java/org/elasticsearch/compute/lucene/LuceneTopNSourceOperatorScoringTests.java


    @After
-    private void closeIndex() throws IOException {
+    public void closeScoringIndex() throws IOException {


The private method was making the ide sad. But you can't give them the same names as the super type one.

dnhatn

LGTM. Let's merge this so we can work on the fix. Thanks, Nik!

rjernst

transport constant changes look good

nik9000 · 2025-07-07T20:12:20Z

transport constant changes look good

thanks!

This refactors our `ValuesSourceReaderOperator` so it can split pages when it reads large values. It does not *actually* split the pages as that's a bit tricky. But it sets the stage for the next PR that will do so. * Move `ValuesSourceReaderOperator` to it's own package * Move many inner classes into their own top level classes * Extend from `AbstractPageMappingToIteratorOperator` instead of `AbstractPageMappingToOperator` * This allows returning more than one `Page` per input `Page` * In this PR we still always return one `Page` per input `Page` * Make new `ReleasableIterator` subclasses to satisfy `AbstractPageMappingToIteratorOperator` * Change `status` of loading fields from `pages_processed` to `pages_received` and `pages_emitted` * Fix a bug in `AbstractPageMappingToOperator` which can leak circuit breaker allocation if we fail to during `receive`. This isn't possible in the existing implementations but is possible in `ValuesSourceReaderOperator`. * Add a test with large text fields. Right now it still comes back in one page because we don't cut the pages. Closes elastic#130727 Also includes "Claim backported profile versions (elastic#130187)"

nik9000 · 2025-07-08T01:25:17Z

9.1: #130781
8.19: #130784

This refactors our `ValuesSourceReaderOperator` so it can split pages when it reads large values. It does not *actually* split the pages as that's a bit tricky. But it sets the stage for the next PR that will do so. * Move `ValuesSourceReaderOperator` to it's own package * Move many inner classes into their own top level classes * Extend from `AbstractPageMappingToIteratorOperator` instead of `AbstractPageMappingToOperator` * This allows returning more than one `Page` per input `Page` * In this PR we still always return one `Page` per input `Page` * Make new `ReleasableIterator` subclasses to satisfy `AbstractPageMappingToIteratorOperator` * Change `status` of loading fields from `pages_processed` to `pages_received` and `pages_emitted` * Fix a bug in `AbstractPageMappingToOperator` which can leak circuit breaker allocation if we fail to during `receive`. This isn't possible in the existing implementations but is possible in `ValuesSourceReaderOperator`. * Add a test with large text fields. Right now it still comes back in one page because we don't cut the pages. Closes elastic#130727

This refactors our `ValuesSourceReaderOperator` so it can split pages when it reads large values. It does not *actually* split the pages as that's a bit tricky. But it sets the stage for the next PR that will do so. * Move `ValuesSourceReaderOperator` to it's own package * Move many inner classes into their own top level classes * Extend from `AbstractPageMappingToIteratorOperator` instead of `AbstractPageMappingToOperator` * This allows returning more than one `Page` per input `Page` * In this PR we still always return one `Page` per input `Page` * Make new `ReleasableIterator` subclasses to satisfy `AbstractPageMappingToIteratorOperator` * Change `status` of loading fields from `pages_processed` to `pages_received` and `pages_emitted` * Fix a bug in `AbstractPageMappingToOperator` which can leak circuit breaker allocation if we fail to during `receive`. This isn't possible in the existing implementations but is possible in `ValuesSourceReaderOperator`. * Add a test with large text fields. Right now it still comes back in one page because we don't cut the pages. Closes #130727 Also includes "Claim backported profile versions (#130187)"

This refactors our `ValuesSourceReaderOperator` so it can split pages when it reads large values. It does not *actually* split the pages as that's a bit tricky. But it sets the stage for the next PR that will do so. * Move `ValuesSourceReaderOperator` to it's own package * Move many inner classes into their own top level classes * Extend from `AbstractPageMappingToIteratorOperator` instead of `AbstractPageMappingToOperator` * This allows returning more than one `Page` per input `Page` * In this PR we still always return one `Page` per input `Page` * Make new `ReleasableIterator` subclasses to satisfy `AbstractPageMappingToIteratorOperator` * Change `status` of loading fields from `pages_processed` to `pages_received` and `pages_emitted` * Fix a bug in `AbstractPageMappingToOperator` which can leak circuit breaker allocation if we fail to during `receive`. This isn't possible in the existing implementations but is possible in `ValuesSourceReaderOperator`. * Add a test with large text fields. Right now it still comes back in one page because we don't cut the pages. Closes elastic#130727

This refactors our `ValuesSourceReaderOperator` so it can split pages when it reads large values. It does not *actually* split the pages as that's a bit tricky. But it sets the stage for the next PR that will do so. * Move `ValuesSourceReaderOperator` to it's own package * Move many inner classes into their own top level classes * Extend from `AbstractPageMappingToIteratorOperator` instead of `AbstractPageMappingToOperator` * This allows returning more than one `Page` per input `Page` * In this PR we still always return one `Page` per input `Page` * Make new `ReleasableIterator` subclasses to satisfy `AbstractPageMappingToIteratorOperator` * Change `status` of loading fields from `pages_processed` to `pages_received` and `pages_emitted` * Fix a bug in `AbstractPageMappingToOperator` which can leak circuit breaker allocation if we fail to during `receive`. This isn't possible in the existing implementations but is possible in `ValuesSourceReaderOperator`. * Add a test with large text fields. Right now it still comes back in one page because we don't cut the pages. Closes #130727

We need to release the blocks of the page in AbstractPageMappingToIteratorOperator immediately in single-iteration cases, instead of delaying to the next iteration. This is because the blocks of the page are now shared with the output page. The output page can be passed to a separate driver, which may run concurrently with this driver, leading to data races in AbstractNonThreadSafeRefCounted, which is not thread-safe. An alternative would be to make RefCounted for Vectors/Blocks thread-safe when they are about to be shared with other drivers via #allowPassingToDifferentDriver. Relates #130573 Closes #130959 Closes #130958 Closes #130950 Closes #130925 Closes #130881 Closes #130796

) We need to release the blocks of the page in AbstractPageMappingToIteratorOperator immediately in single-iteration cases, instead of delaying to the next iteration. This is because the blocks of the page are now shared with the output page. The output page can be passed to a separate driver, which may run concurrently with this driver, leading to data races in AbstractNonThreadSafeRefCounted, which is not thread-safe. An alternative would be to make RefCounted for Vectors/Blocks thread-safe when they are about to be shared with other drivers via Relates elastic#130573 Closes elastic#130959 Closes elastic#130958 Closes elastic#130950 Closes elastic#130925 Closes elastic#130881 Closes elastic#130796

…130972) We need to release the blocks of the page in AbstractPageMappingToIteratorOperator immediately in single-iteration cases, instead of delaying to the next iteration. This is because the blocks of the page are now shared with the output page. The output page can be passed to a separate driver, which may run concurrently with this driver, leading to data races in AbstractNonThreadSafeRefCounted, which is not thread-safe. An alternative would be to make RefCounted for Vectors/Blocks thread-safe when they are about to be shared with other drivers via Relates #130573 Closes #130959 Closes #130958 Closes #130950 Closes #130925 Closes #130881 Closes #130796

…130971) We need to release the blocks of the page in AbstractPageMappingToIteratorOperator immediately in single-iteration cases, instead of delaying to the next iteration. This is because the blocks of the page are now shared with the output page. The output page can be passed to a separate driver, which may run concurrently with this driver, leading to data races in AbstractNonThreadSafeRefCounted, which is not thread-safe. An alternative would be to make RefCounted for Vectors/Blocks thread-safe when they are about to be shared with other drivers via Relates #130573 Closes #130959 Closes #130958 Closes #130950 Closes #130925 Closes #130881 Closes #130796

) We need to release the blocks of the page in AbstractPageMappingToIteratorOperator immediately in single-iteration cases, instead of delaying to the next iteration. This is because the blocks of the page are now shared with the output page. The output page can be passed to a separate driver, which may run concurrently with this driver, leading to data races in AbstractNonThreadSafeRefCounted, which is not thread-safe. An alternative would be to make RefCounted for Vectors/Blocks thread-safe when they are about to be shared with other drivers via #allowPassingToDifferentDriver. Relates elastic#130573 Closes elastic#130959 Closes elastic#130958 Closes elastic#130950 Closes elastic#130925 Closes elastic#130881 Closes elastic#130796

nik9000 added 10 commits July 2, 2025 11:35

Part

c41decd

Load

358b7e9

Migrate

41128fe

Compile

c395ad0

appendBlockArrays

83b5a29

Page mapping operator

36b566e

TEst

6ae4944

Big refactor and start

85f8a71

tests

8e8eb72

Runnable

5284db0

nik9000 added >non-issue :Analytics/ES|QL AKA ESQL v8.19.0 v9.1.0 v9.2.0 labels Jul 3, 2025

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 3, 2025

nik9000 added 2 commits July 3, 2025 13:02

nocommitsbgoe

093a14f

Merge branch 'main' into esql_danger_zone

0475ed3

nik9000 commented Jul 3, 2025

View reviewed changes

dnhatn approved these changes Jul 3, 2025

View reviewed changes

nik9000 added 2 commits July 7, 2025 08:12

Update

db32299

Merge branch 'main' into esql_danger_zone

a53bd0f

nik9000 mentioned this pull request Jul 7, 2025

ESQL: Don't build empty ValuesSourceReaderOperators #130727

Closed

nik9000 added 5 commits July 7, 2025 13:00

Fixup lookup

e20b4aa

Prepare for backport

6faf095

Merge branch 'main' into esql_danger_zone

5c6a74a

Explain

d1fe232

Explain

8f6bcc5

rjernst reviewed Jul 7, 2025

View reviewed changes

nik9000 enabled auto-merge (squash) July 7, 2025 20:32

nik9000 added 2 commits July 7, 2025 17:14

Merge branch 'main' into esql_danger_zone

c9cc7ec

Merge branch 'main' into esql_danger_zone

61e8a96

nik9000 merged commit 75fe33d into elastic:main Jul 8, 2025
33 checks passed

nik9000 added the backport pending label Jul 8, 2025

luigidellaquila mentioned this pull request Jul 8, 2025

[CI] EnrichIT testSumDurationByArtist failing #130788

Closed

nik9000 removed the backport pending label Jul 8, 2025

dnhatn mentioned this pull request Jul 9, 2025

Fix data race with AbstractPageMappingToIteratorOperator #130963

Merged

idegtiarenko mentioned this pull request Jul 11, 2025

Ensure single segment in test #131077

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ESQL: Refactor value reading so it can split Pages #130573

ESQL: Refactor value reading so it can split Pages #130573

Uh oh!

nik9000 commented Jul 3, 2025 •

edited

Loading

Uh oh!

elasticsearchmachine commented Jul 3, 2025

Uh oh!

nik9000 commented Jul 3, 2025

Uh oh!

nik9000 Jul 3, 2025

Uh oh!

dnhatn left a comment

Uh oh!

rjernst left a comment

Uh oh!

nik9000 commented Jul 7, 2025

Uh oh!

Uh oh!

nik9000 commented Jul 8, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ESQL: Refactor value reading so it can split Pages #130573

ESQL: Refactor value reading so it can split Pages #130573

Uh oh!

Conversation

nik9000 commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jul 3, 2025

Uh oh!

nik9000 commented Jul 3, 2025

Uh oh!

nik9000 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

dnhatn left a comment

Choose a reason for hiding this comment

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

nik9000 commented Jul 7, 2025

Uh oh!

Uh oh!

nik9000 commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nik9000 commented Jul 3, 2025 •

edited

Loading

nik9000 commented Jul 8, 2025 •

edited

Loading