Skip to content

[Lens][Embeddable] Fix memory leak on ES|QL variables subscription#210826

Merged
dej611 merged 3 commits intoelastic:mainfrom
dej611:fix/esql-variables-memory-leak
Feb 14, 2025
Merged

[Lens][Embeddable] Fix memory leak on ES|QL variables subscription#210826
dej611 merged 3 commits intoelastic:mainfrom
dej611:fix/esql-variables-memory-leak

Conversation

@dej611
Copy link
Copy Markdown
Contributor

@dej611 dej611 commented Feb 12, 2025

Summary

This PR fixes a bug due to multiple subscription created by the ESQL variables logic in the embeddable to never been cancelled.
The fix was to move the subscription in the loader module and make it cleanup correctly together with other subscription.

Unit tests have been added to check the correct re-render behaviour.

Checklist

@dej611 dej611 added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes Feature:Lens backport:prev-major labels Feb 12, 2025
@dej611 dej611 requested a review from a team as a code owner February 12, 2025 13:28
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@dej611 dej611 self-assigned this Feb 12, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.6MB 1.6MB -31.0B

History

cc @dej611

@dej611 dej611 merged commit 34baecb into elastic:main Feb 14, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.16, 8.17, 8.18, 8.x

https://github.com/elastic/kibana/actions/runs/13326167883

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2025
…lastic#210826)

## Summary

This PR fixes a bug due to multiple subscription created by the ESQL
variables logic in the embeddable to never been cancelled.
The fix was to move the subscription in the loader module and make it
cleanup correctly together with other subscription.

Unit tests have been added to check the correct re-render behaviour.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 34baecb)
@kibanamachine
Copy link
Copy Markdown
Contributor

💔 Some backports could not be created

Status Branch Result
8.16 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.16:
- chore(NA): upgrade to webpack 5 (#191106)
- [Links] Fix link settings not persisting (#211041)
- [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)
- [Fleet] Fix unattended Transforms in integration packages not automatically restarting after reauthorizing (#210217)
8.17 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.17:
- chore(NA): upgrade to webpack 5 (#191106)
- [Links] Fix link settings not persisting (#211041)
- [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)
- [Automatic Import] Fix generated name for integration title (#210916)
- [Fleet] Fix unattended Transforms in integration packages not automatically restarting after reauthorizing (#210217)
8.18
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 210826

Questions ?

Please refer to the Backport tool documentation

@dej611 dej611 added backport:version Backport to applied version labels v8.18.0 and removed backport:prev-major labels Feb 14, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.18

https://github.com/elastic/kibana/actions/runs/13326553682

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2025
…lastic#210826)

## Summary

This PR fixes a bug due to multiple subscription created by the ESQL
variables logic in the embeddable to never been cancelled.
The fix was to move the subscription in the loader module and make it
cleanup correctly together with other subscription.

Unit tests have been added to check the correct re-render behaviour.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 34baecb)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.18

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 14, 2025
…ion (#210826) (#211171)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens][Embeddable] Fix memory leak on ES|QL variables subscription
(#210826)](#210826)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-14T09:18:55Z","message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","backport:prev-major","v9.1.0"],"title":"[Lens][Embeddable]
Fix memory leak on ES|QL variables
subscription","number":210826,"url":"https://github.com/elastic/kibana/pull/210826","mergeCommit":{"message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210826","number":210826,"mergeCommit":{"message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33"}}]}]
BACKPORT-->

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Feb 14, 2025
…tion (#210826) (#211170)

# Backport

This will backport the following commits from `main` to `8.18`:
- [[Lens][Embeddable] Fix memory leak on ES|QL variables subscription
(#210826)](#210826)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-14T09:18:55Z","message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","backport:prev-major","v9.1.0"],"title":"[Lens][Embeddable]
Fix memory leak on ES|QL variables
subscription","number":210826,"url":"https://github.com/elastic/kibana/pull/210826","mergeCommit":{"message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210826","number":210826,"mergeCommit":{"message":"[Lens][Embeddable]
Fix memory leak on ES|QL variables subscription (#210826)\n\n##
Summary\n\nThis PR fixes a bug due to multiple subscription created by
the ESQL\nvariables logic in the embeddable to never been
cancelled.\nThe fix was to move the subscription in the loader module
and make it\ncleanup correctly together with other subscription.\n\nUnit
tests have been added to check the correct re-render behaviour.\n\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"34baecba3e11a14f5aaf3badc053056084945b33"}}]}]
BACKPORT-->

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
…lastic#210826)

## Summary

This PR fixes a bug due to multiple subscription created by the ESQL
variables logic in the embeddable to never been cancelled.
The fix was to move the subscription in the loader module and make it
cleanup correctly together with other subscription.

Unit tests have been added to check the correct re-render behaviour.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.18.0 v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants