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

Stop websockets from disconnecting immediately #63062

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

MKLeb
Copy link
Contributor

@MKLeb MKLeb commented Nov 15, 2022

What does this PR do?

Removes a check introduced here that was part of a patch to solve a memory leak (see the issue here. The regression test shows that there is no leak even after removing the previous patch.

Also, after a patch from about 5 years ago (I believe this one), the tag_map wasn't being properly cleaned up and was growing to be massive if a lot of calls to the API happened. That has been resolved.

What issues does this PR fix or reference?

Fixes: #59183

Previous Behavior

Websockets closed immediately, and the tag map leaked memory.

New Behavior

The connection remains open, and we have a properly cleaned tag map.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@MKLeb MKLeb requested a review from a team as a code owner November 15, 2022 00:55
@MKLeb MKLeb requested review from waynew and removed request for a team November 15, 2022 00:55
@Ch3LL Ch3LL added the Sulfur v3006.0 release code name and version label Nov 15, 2022
@Ch3LL Ch3LL requested a review from dwoz November 15, 2022 20:43
Copy link
Contributor

@dwoz dwoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see a regression test for #24965

@MKLeb
Copy link
Contributor Author

MKLeb commented Nov 17, 2022

@dwoz This should be ready then

@Ch3LL Ch3LL merged commit e0e4d56 into saltstack:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ws all_events api disconnects immediately
3 participants