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.

Sign up for GitHub

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

Jump to bottom

Cleanup output futures list as they complete #16057

Merged
merged 1 commit into from
Apr 26, 2024
Merged

Cleanup output futures list as they complete #16057

merged 1 commit into from
Apr 26, 2024

Conversation

Frassle
Copy link
Member

@Frassle Frassle commented Apr 25, 2024 •
edited

Description

Fixes #16055.

#15744 added precise output tracking to the python runtime, so that we can error correctly on any output that raises an exception, and don't wait for all async tasks that are running which could be unrelated to the pulumi program.

Unfortunately that change had a memory leak where it tracked all outputs for the entire run of the program. Not an issue for our small programs in the tests but problematic when a program ends up making thousands of resources with tens of thousands of output values.

This cleans the outputs up from the tracking list as they complete successfully.

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
    • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Apr 25, 2024 •
edited

Changelog

[uncommitted] (2024-04-26)

Bug Fixes

  • [sdk/python] Fix a memory leak in tracking outputs.
    #16057

@Frassle Frassle force-pushed the fraser/outputLeak branch 6 times, most recently from 15e9901 to 0f1f513 Compare April 25, 2024 20:18
@Frassle Frassle marked this pull request as ready for review April 25, 2024 20:22
@Frassle Frassle requested a review from a team as a code owner April 25, 2024 20:22
justinvp
justinvp reviewed Apr 25, 2024
View reviewed changes
tests/integration/integration_python_test.go Outdated Show resolved Hide resolved
sdk/python/lib/pulumi/output.py Outdated Show resolved Hide resolved
tests/integration/python_await/output_leak/__main__.py Outdated Show resolved Hide resolved
@Frassle Frassle force-pushed the fraser/outputLeak branch 5 times, most recently from 2d3e234 to 2a8e1cc Compare April 25, 2024 21:00
@justinvp justinvp mentioned this pull request Apr 25, 2024
Merged
@justinvp justinvp enabled auto-merge April 25, 2024 21:09
github-merge-queue bot pushed a commit that referenced this pull request Apr 25, 2024
@justinvp
Prepare for v3.114.0 release (#16061)
07b6e21 
Tentative changelog, but also planning to include
#16057

### Features

- [auto/{go,nodejs,python}] Add support for the continue-on-error
parameter of the up command to the Automation API
  [#15953](#15953)

- [engine] Add a --continue-on-error flag to pulumi up
  [#15740](#15740)


### Bug Fixes

- [sdk/nodejs] Fix a race condition that could cause the NodeJS runtime
to terminate before finishing all work
  [#16005](#16005)

- [sdk/python] Fix an exception when setting providers resource option
with a dict
  [#16022](#16022)

- [sdk/python] Fix event loop tracking in the python SDK when using
remote transforms
  [#16039](#16039)

- [sdk/python] Workaround lazy module loading regression
  [#16038](#16038)


### Miscellaneous

- [cli/plugin] Move PluginKind type definition into apitype and
re-export for backward compatibility
@justinvp justinvp added this pull request to the merge queue Apr 25, 2024
@justinvp justinvp mentioned this pull request Apr 25, 2024
Merged
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 25, 2024
@Frassle Frassle added the ci/test Test CI pipelines on this PR label Apr 26, 2024
@Frassle Frassle added this pull request to the merge queue Apr 26, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 26, 2024
@Frassle
Cleanup output futures list as they complete (#16057)
d61d4b4 
<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

Fixes #16055.

#15744 added precise output
tracking to the python runtime, so that we can error correctly on any
output that raises an exception, and don't wait for all async tasks that
are running which could be unrelated to the pulumi program.

Unfortunately that change had a memory leak where it tracked all outputs
for the entire run of the program. Not an issue for our small programs
in the tests but problematic when a program ends up making thousands of
resources with tens of thousands of output values.

This cleans the outputs up from the tracking list as they complete
successfully.

## Checklist

- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
  - [ ] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 26, 2024
@Frassle Frassle enabled auto-merge April 26, 2024 15:17
@Frassle Frassle added this pull request to the merge queue Apr 26, 2024
Merged via the queue into master with commit 52b58ae Apr 26, 2024
90 of 91 checks passed
@Frassle Frassle deleted the fraser/outputLeak branch April 26, 2024 17:18
github-merge-queue bot pushed a commit that referenced this pull request Apr 26, 2024
@justinvp
Freeze v3.114.0 (#16062)
88e5625 
Will wait to merge this until after
#16057 merges

### Features

- [auto/{go,nodejs,python}] Add support for the continue-on-error
parameter of the up command to the Automation API
  [#15953](#15953)

- [engine] Add a --continue-on-error flag to pulumi up
  [#15740](#15740)


### Bug Fixes

- [sdk/nodejs] Fix a race condition that could cause the NodeJS runtime
to terminate before finishing all work
  [#16005](#16005)

- [sdk/python] Fix an exception when setting providers resource option
with a dict
  [#16022](#16022)

- [sdk/python] Fix event loop tracking in the python SDK when using
remote transforms
  [#16039](#16039)

- [sdk/python] Workaround lazy module loading regression
  [#16038](#16038)


### Miscellaneous

- [cli/plugin] Move PluginKind type definition into apitype and
re-export for backward compatibility
  [#15946](#15946)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinvp justinvp justinvp approved these changes

Assignees
No one assigned
Labels
ci/test Test CI pipelines on this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Python memory pressure from tracking all outputs till the end of the program
3 participants
@Frassle @pulumi-bot @justinvp