Skip to content
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

Fix esql enrich memory leak (#109275) #110450

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Jul 4, 2024

This PR was reviewed in #109275

Block and Vector use a non-thread-safe RefCounted. Threads that increase or decrease the references must have a happen-before relationship. However, this order is not guaranteed in the enrich lookup for the reference of selectedPositions. The driver can complete the MergePositionsOperator, which decreases the reference count of selectedPositions, while the finally block may also decrease it in a separate thread. These actions occur without a defined happen-before relationship.

Closes #108532

Block and Vector use a non-thread-safe RefCounted. Threads that increase
or decrease the references must have a happen-before relationship.
However, this order is not guaranteed in the enrich lookup for the
reference of selectedPositions. The driver can complete the
MergePositionsOperator, which decreases the reference count of
selectedPositions, while the finally block may also decrease it in a
separate thread. These actions occur without a defined happen-before
relationship.

Closes elastic#108532
@dnhatn dnhatn added v8.15.1 :Analytics/ES|QL AKA ESQL >non-issue v8.14.3 auto-backport-and-merge auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Jul 4, 2024
@dnhatn dnhatn marked this pull request as ready for review July 4, 2024 18:26
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 4, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine merged commit c8ece6a into elastic:main Jul 4, 2024
15 checks passed
@dnhatn dnhatn deleted the fix-enrich-leak branch July 4, 2024 19:27
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jul 4, 2024
This PR was reviewed in elastic#109275

Block and Vector use a non-thread-safe RefCounted. Threads that increase
or decrease the references must have a happen-before relationship.
However, this order is not guaranteed in the enrich lookup for the
reference of selectedPositions. The driver can complete the
MergePositionsOperator, which decreases the reference count of
selectedPositions, while the finally block may also decrease it in a
separate thread. These actions occur without a defined happen-before
relationship.

Closes elastic#108532
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.14 Commit could not be cherrypicked due to conflicts
8.15

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 110450

elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2024
This PR was reviewed in #109275

Block and Vector use a non-thread-safe RefCounted. Threads that increase
or decrease the references must have a happen-before relationship.
However, this order is not guaranteed in the enrich lookup for the
reference of selectedPositions. The driver can complete the
MergePositionsOperator, which decreases the reference count of
selectedPositions, while the finally block may also decrease it in a
separate thread. These actions occur without a defined happen-before
relationship.

Closes #108532
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jul 4, 2024
This PR was reviewed in elastic#109275

Block and Vector use a non-thread-safe RefCounted. Threads that increase
or decrease the references must have a happen-before relationship.
However, this order is not guaranteed in the enrich lookup for the
reference of selectedPositions. The driver can complete the
MergePositionsOperator, which decreases the reference count of
selectedPositions, while the finally block may also decrease it in a
separate thread. These actions occur without a defined happen-before
relationship.

Closes elastic#108532
elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2024
This PR was reviewed in #109275

Block and Vector use a non-thread-safe RefCounted. Threads that increase
or decrease the references must have a happen-before relationship.
However, this order is not guaranteed in the enrich lookup for the
reference of selectedPositions. The driver can complete the
MergePositionsOperator, which decreases the reference count of
selectedPositions, while the finally block may also decrease it in a
separate thread. These actions occur without a defined happen-before
relationship.

Closes #108532
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-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.14.3 v8.15.1 v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] EnrichIT testManyDocuments failing
2 participants