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

sr cleanup is too dangerous #187

Closed
benlapETS opened this issue Apr 17, 2019 · 6 comments
Closed

sr cleanup is too dangerous #187

benlapETS opened this issue Apr 17, 2019 · 6 comments
Assignees
Labels
bug Something isn't working Design Problem hard to fix, affects design. efficiency improve performance and/or reduce overhead Priority 3 - Important worrisome impact... should study carefully

Comments

@benlapETS
Copy link
Contributor

benlapETS commented Apr 17, 2019

This option is dangerous if involuntary triggered in an operational environment, we may want to add a switch.

@petersilva petersilva added the bug Something isn't working label Sep 16, 2019
@petersilva
Copy link
Contributor

this bug mostly affects pxatx, which is running 2.17 or earlier... all other systems have explicit protection for xpublic, which is the biggest problem. However sr.py is rewritten in #174, and in there, there is no implementation of sr cleanup (it has been removed.)

losing the function is actually an intermediate step. The rewritten sr.py creates a global state data structure, which makes it possible to do a safe version of cleanup (because it looks at all configurations, not just one at a time) and will delete only if none of the configs reference the exchange. This will help with exchanges that don´t have a special name, like xwinnow, etc... that might fall victim to the same symptom.

So there are some necessary steps to making sr cleanup safe, using the new implementation:

  • add/ensure there is a means (in admin.conf?) to declare exchanges that are not used by any local configs to protect them from clean up declare exchange hoho in admin.conf should protect it.

  • add parsing of exchanges to the parser in the new sr.py, to know which exchanges should exist.

  • add interrogation of state of broker to sr.py to get a global broker state in one data structure (all exchanges, queues, and bindings.)

@petersilva
Copy link
Contributor

petersilva commented Sep 16, 2019

note we have had two occurrences of deletion of bindings on xpublic on important pumps, and this caused considerable havoc. however current versions explicitly exclude xpublic from cleanup. so the severity of the defect at this time is not high.

@petersilva petersilva added the Design Problem hard to fix, affects design. label Sep 16, 2019
@petersilva
Copy link
Contributor

The declare should likely also use the logic above... the current implementation from #174 still spawns processes to do declare, getting global state as above would allow for a much improved declare.

@petersilva petersilva added the efficiency improve performance and/or reduce overhead label Sep 16, 2019
@petersilva
Copy link
Contributor

implemented new config parse for sr.py on issue187 branch.

petersilva pushed a commit that referenced this issue Dec 30, 2019
petersilva pushed a commit that referenced this issue Jan 2, 2020
petersilva pushed a commit that referenced this issue Jan 4, 2020
@petersilva
Copy link
Contributor

dev for this is on https://github.com/MetPX/sarracenia/tree/issue187

@petersilva petersilva self-assigned this Jan 14, 2020
@petersilva petersilva added the Priority 3 - Important worrisome impact... should study carefully label Feb 26, 2020
petersilva added a commit that referenced this issue May 6, 2020
… command.

that python module is way too slow to use at scale buge opened with them.

fix #174, help with #315 and #180, and #187
@petersilva
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Design Problem hard to fix, affects design. efficiency improve performance and/or reduce overhead Priority 3 - Important worrisome impact... should study carefully
Projects
None yet
Development

No branches or pull requests

2 participants