#Issue63469 Update documentation to include performance impact warnings and workaround - #63610
#Issue63469 Update documentation to include performance impact warnings and workaround#63610shashwatpandeyvns wants to merge 4 commits into
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
|
@shashwatpandeyvns Could you update communication guide page as well? |
@rawmind I believe the update for the communication guide "https://github.com/saltstack/salt-get-started/pull/36" has been merged to the master file successfully. BTW how do I check if this is merged or not? All thanks to you for helping me. |
|
@rawmind In this one few workflows failed and a review is required as well, is there any issue with the code? |
|
@shashwatpandeyvns thank you for the PR. As I said I'm not a part of salt team. You should wait until someone from the team make review and approve it. Unfortunately, I can't assist you to resolve GH pipelines issues. |
barbaricyawps
changed the title
barbaricyawps
left a comment
barbaricyawps
left a comment
There was a problem hiding this comment.
@shashwatpandeyvns , thanks for your docs PR! I love to see people who are motivated to help improve the docs.
I've made a few blocking suggestions about your rST formatting and I've also made some non-blocking suggestions about phrasing that I'm hoping you'll work to resolve.
After that, I'll need a core team member to do an additional review for technical accuracy.
Thanks again for your help!
|
@shashwatpandeyvns, thanks so much for being so responsive and making those changes. I will have some time on Monday to take another look at this PR and so I'll definitely review your changes in more depth at that time. Thanks again for your help! |
barbaricyawps
left a comment
barbaricyawps
left a comment
There was a problem hiding this comment.
@shashwatpandeyvns , this looks good to me! I'll get a Core team member to do a quick technical review.
barbaricyawps
added
the
pending-discussion
|
@shashwatpandeyvns , I'm just circling back on this PR. Did you have a chance to respond to @Ch3LL 's question about your warning note? |
Ch3LL
added
Chlorine v3007.0
and removed
pending-discussion
|
pre-commit is failing here. and it looks ot be related to this change. |
|
Bump ^ |
| Using globs to reject the specified public key may cause performance issues | ||
| because of a Salt master degredation dramatic performance salt-master degradation. | ||
| See [`performance guide`](https://docs.saltproject.io/en/latest/topics/tutorials/intro_scale.html#too-many-minions-re-authing) | ||
| for more information about performance impacts and workarounds. |
There was a problem hiding this comment.
This needs to be reworded:
Using globs to reject public keys may degrade Salt master performance.
See the performance guide
for details on performance impacts and possible workarounds.
| Using globs to reject the specified public key may cause performance issues | ||
| because of a Salt master degredation dramatic performance salt-master degradation. | ||
| See [`performance guide`](https://docs.saltproject.io/en/latest/topics/tutorials/intro_scale.html#too-many-minions-re-authing) | ||
| for more information about performance impacts and workarounds. |
There was a problem hiding this comment.
This warning needs say something more along the lines of "rejecting many keys simultaneously may cause performance issues" instead of talking about globbing as there is no globbing here.
| Using globs to reject the specified public key may cause performance issues | ||
| because of a Salt master degredation dramatic performance salt-master degradation. | ||
| See [`performance guide`](https://docs.saltproject.io/en/latest/topics/tutorials/intro_scale.html#too-many-minions-re-authing) | ||
| for more information about performance impacts and workarounds. |
There was a problem hiding this comment.
Same wording needs to be updated here
| Using globs to reject the specified public key may cause performance issues | ||
| because of a Salt master degredation dramatic performance salt-master degradation. | ||
| See [`performance guide`](https://docs.saltproject.io/en/latest/topics/tutorials/intro_scale.html#too-many-minions-re-authing) | ||
| for more information about performance impacts and workarounds. |
There was a problem hiding this comment.
This warning needs say something more along the lines of "deleting many keys simultaneously may cause performance issues" instead of talking about globbing as there is no globbing here.
- Replace broken English and duplicated 'degradation' phrase in all 4 warning blocks with accurate, idiomatic text - Fix markdown link syntax to rST hyperlink syntax so Sphinx renders clickable links - Add blank line before each .. warning:: directive so Sphinx parses the admonition correctly - Tailor each warning to its specific operation: reject vs reject-all, delete vs delete-all (per twangboy review 2025-04-02) - Wrap long lines in intro_scale.rst to fit within 80 characters Fixes saltstack#63469
6 participants
What does this PR do?
What issues does this PR fix or reference?
Fixes: #63469
Previous Behavior
Remove this section if not relevant
New Behavior
Remove this section if not relevant
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes/No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.