Fix publisher leak #34683
Fix publisher leak #34683
Conversation
|
Can we please start hard depending on psutils? Please?! That would greatly simplify signal handling for example.... |
|
I'm all for that. It would let us refactor core grains too. It's up to @thatch45 though. |
|
I know it has a history of not being stable, I also see some not future proof usage, but we stick with a version and only upgrade when actually needed and after proper testing... |
| grains = salt.grains.core._memdata(os_data) | ||
| total_mem = grains['mem_total'] | ||
| # Return the higher number between 5% of the system memory and 100MB | ||
| return min([total_mem * 0.05, 10 << 20]) |
There was a problem hiding this comment.
The comment says "Return the higher number", but doesn't min return the lower number?
There was a problem hiding this comment.
Oops. I was going too fast. Thanks for catching this!
|
Go Go Jenkins! |
|
@cachedout Were you going to update the |
|
@rallytime Yes, just did along with a number of other changes. |
|
@cachedout Ooh! Tests are happy now, except some whitespace for pylint. Can you grab that? |
* upstream/develop: (32 commits) Update 2016.3.2 release notes (saltstack#34850) Skip GCE unit tests - causes test suite to hang Update release notes for 2016.3.2 (saltstack#34848) Fix comment in master config, prevents the service from starting Fix Salt failure after merge of saltstack#34683 drop parsing of vdevs, error passthrough from zpool cli eliminate hardcoded vdev type from zpool state update the state wrapper to include show_low_sls salt.states.zpool - work with updates exec module salt.module.zpool - fix bug with properties on/off being parsed as true/false Check if a valid value is passed to unlyif/unless Fix saltstack#34345 salt.modules.zpool - drop vdev types to make it more future proof, fallback to zpool cli error messages keep this beacon from stack tracing at the loader (saltstack#34825) Skip mysql state test if mysqladmin is not available Lintfix PEP8: E262 salt/state.py: set `chunk['order'] = 0' with `order: first'; fixes saltstack#24744 Lint Document master setting Set up dynamic config ...
|
Is this fix available in publicly available versions of Salt? |
|
@rvora Yes - this is available in the 2016.3.2 release of Salt. |
|
This fix should be in 2016.3.2 |
This option was introduced in 2016 [1] and typing enforced in 2022 [2]. However, documentation was not added in the master and minion configuration docs. This change adds the mentioned documentation. 1. saltstack#34683 2. saltstack@ea35cb5
This is a first pass at fixing a memory leak in the IPC publisher.
See the full description of this issue in #34215. Basically, this sets a write_buffer for the publisher.
I have not yet tested this to see how this performs when the buffer is full. Messages should be dropped. I'd like to at least provide notification of this condition before we merge this but the feasibility of this remains to be seen.
I also want to make the buffer size user-configurable. Right now it is selected as a function of total memory on the machine.
This does, however, definitely solve the leak. When the buffer size is reached, the size of the Publisher does not continue to grow.
cc: @thatch45 @skizunov