Skip to content

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Jul 9, 2025

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

@dnhatn dnhatn requested a review from nik9000 July 9, 2025 23:38
@dnhatn dnhatn marked this pull request as ready for review July 9, 2025 23:39
@dnhatn dnhatn requested a review from smalyshev July 9, 2025 23:39
@dnhatn dnhatn added the auto-backport Automatically create backport pull requests when merged label Jul 9, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 9, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@nik9000
Copy link
Member

nik9000 commented Jul 9, 2025

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.

Ouch.

@dnhatn
Copy link
Member Author

dnhatn commented Jul 10, 2025

Thanks Nik!

@dnhatn dnhatn merged commit 05b55b0 into elastic:main Jul 10, 2025
32 of 33 checks passed
@dnhatn dnhatn deleted the fix-ref-count-race branch July 10, 2025 01:13
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jul 10, 2025
)

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
@dnhatn
Copy link
Member Author

dnhatn commented Jul 10, 2025

💚 All backports created successfully

Status Branch Result
9.1
8.19

Questions ?

Please refer to the Backport tool documentation

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jul 10, 2025
)

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
@elastic elastic deleted a comment from elasticsearchmachine Jul 10, 2025
elasticsearchmachine pushed a commit that referenced this pull request Jul 10, 2025
…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
elasticsearchmachine pushed a commit that referenced this pull request Jul 10, 2025
…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
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 17, 2025
)

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
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 17, 2025
)

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 a commit to nik9000/elasticsearch that referenced this pull request Jul 17, 2025
elasticsearchmachine pushed a commit that referenced this pull request Jul 17, 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 >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.1 v9.1.1 v9.2.0

Projects

None yet

3 participants