Overview
Request 1055804 superseded
- Prevent deadlocks in salt-ssh executions
- Added:
* use-rlock-to-avoid-deadlocks-in-salt-ssh.patch
- Update to Salt release version 3005.1
* See release notes: https://docs.saltstack.com/en/latest/topics/releases/3005.1.html
- Create new salt-tests subpackage containing Salt tests
- Modified:
* activate-all-beacons-sources-config-pillar-grains.patch
* add-amazon-ec2-detection-for-virtual-grains-bsc-1195.patch
* add-custom-suse-capabilities-as-grains.patch
* add-environment-variable-to-know-if-yum-is-invoked-f.patch
* add-migrated-state-and-gpg-key-management-functions-.patch
* add-publish_batch-to-clearfuncs-exposed-methods.patch
* add-salt-ssh-support-with-venv-salt-minion-3004-493.patch
* add-sleep-on-exception-handling-on-minion-connection.patch
* add-standalone-configuration-file-for-enabling-packa.patch
* add-support-for-gpgautoimport-539.patch
* add-support-for-name-pkgs-and-diff_attr-parameters-t.patch
* align-amazon-ec2-nitro-grains-with-upstream-pr-bsc-1.patch
* allow-vendor-change-option-with-zypper.patch
* async-batch-implementation.patch
* avoid-excessive-syslogging-by-watchdog-cronjob-58.patch
* bsc-1176024-fix-file-directory-user-and-group-owners.patch
* change-the-delimeters-to-prevent-possible-tracebacks.patch
* clarify-pkg.installed-pkg_verify-documentation.patch
* debian-info_installed-compatibility-50453.patch
* detect-module.run-syntax.patch
* dnfnotify-pkgset-plugin-implementation-3002.2-450.patch
* do-not-load-pip-state-if-there-is-no-3rd-party-depen.patch
- Created by PSuarezHernandez
- In state superseded
- Supersedes 1046563
- Superseded by 1056172
Request History
PSuarezHernandez created request
- Prevent deadlocks in salt-ssh executions
- Added:
* use-rlock-to-avoid-deadlocks-in-salt-ssh.patch
- Update to Salt release version 3005.1
* See release notes: https://docs.saltstack.com/en/latest/topics/releases/3005.1.html
- Create new salt-tests subpackage containing Salt tests
- Modified:
* activate-all-beacons-sources-config-pillar-grains.patch
* add-amazon-ec2-detection-for-virtual-grains-bsc-1195.patch
* add-custom-suse-capabilities-as-grains.patch
* add-environment-variable-to-know-if-yum-is-invoked-f.patch
* add-migrated-state-and-gpg-key-management-functions-.patch
* add-publish_batch-to-clearfuncs-exposed-methods.patch
* add-salt-ssh-support-with-venv-salt-minion-3004-493.patch
* add-sleep-on-exception-handling-on-minion-connection.patch
* add-standalone-configuration-file-for-enabling-packa.patch
* add-support-for-gpgautoimport-539.patch
* add-support-for-name-pkgs-and-diff_attr-parameters-t.patch
* align-amazon-ec2-nitro-grains-with-upstream-pr-bsc-1.patch
* allow-vendor-change-option-with-zypper.patch
* async-batch-implementation.patch
* avoid-excessive-syslogging-by-watchdog-cronjob-58.patch
* bsc-1176024-fix-file-directory-user-and-group-owners.patch
* change-the-delimeters-to-prevent-possible-tracebacks.patch
* clarify-pkg.installed-pkg_verify-documentation.patch
* debian-info_installed-compatibility-50453.patch
* detect-module.run-syntax.patch
* dnfnotify-pkgset-plugin-implementation-3002.2-450.patch
* do-not-load-pip-state-if-there-is-no-3rd-party-depen.patch
PSuarezHernandez superseded request
superseded by 1056172
How come we need to patch this downstream?
This patch locking mechanism for "salt-ssh" is not yet pushed to upstream, we only have it downstream.
Can we then at least link to the PR that introduced it downstream instead of a commit with no explanation on why the change is needed?
I've added some more notes/links for this new patch. New SR: https://build.opensuse.org/request/show/1056172
Thanks, that's a bit better. It still isn't clear why you needed to change Lock to RLock, which I really think deserves explanation (I would have asked to include that to the commit message had this been a PR).
Yeah, sorry for not creating a PR for this one - that would be the correct way to clarify this properly, but since there was a previous agreement about move these "Lock" to "RLock" I simply went ahead without PR.
Let me bring some context here as this is something Victor and I already discussed and agreed in the past.
The reason that motivated to use "RLock" here are basically the same that also motivated to use "RLock" here as well for a similar lock that we introduced: https://github.com/openSUSE/salt/pull/497
Basically, during our investigations around the logging and import deadlocks that we have been experiencing around "salt-ssh", we introduced different locks to prevent the possible deadlocks to happen. We figured out that introducing a normal "Lock" will cause an extra deadlock under certain stack scenarios (particularly when running salt-ssh via salt-api), where a same thread is acquiring the lock multiple times.
We decided it was reasonable to introduce "RLock" instead "Lock" for this locks we introduced. While we didn't change this one lock to "RLock" in 3004, it seems all the lazyloader / salt-api / salt-ssh / multiprocessing / threading magic is making to hit the deadlock for this one in 3005.1.
I hope this provides enough context around the motivations of this SR, and sorry again for now going via PR workflow on this one.
ok, thanks for the clarification. I guess hope we can drop these patches soon after upstreaming them.