Fix git_pillar race condition #31836
Conversation
This does a few things: 1. Introduces the concept of a checkout lock, to prevent concurrent "_pillar" master funcs from trying to checkout the repo at the same time. 2. Refrains from checking out unless the SHA has changed. 3. Cleans up some usage of the GitProvider subclass' "url" attribute when "id" should be used.
This is necessary for "salt-run fileserver.clear_lock" to work
terminalmage
force-pushed
the
issue31293
branch
from
82a5ada
to
5048fa8
Compare
Fix git_pillar race condition
|
I patched this into salt 2015.8.7. I'll let you know how it works out. |
|
@deuscapturus Thanks! |
|
@terminalmage Thanks Erik. I just applied the patch to my master (which is also now running 2015.8.7). So far so good. I did a number of |
|
@terminalmage git HEAD is not being lost. However, git_pillar updates are not working for me. When I |
|
@deuscapturus Thanks for the feedback, I don't think this pull request touched that file so there could be something that I need to update there to make it work with the recent updates. I'll take a look. |
|
@deuscapturus Also, keep in mind that the scheduled updates that happen every ~60 sec should still be working, at least they were for me. |
|
@terminalmage We didn't see any updates for a 24 hour period after applying the patch. Again clearing cache and restarting bring the pillar cache up-to-date. We see no errors in the log. It could be that we didn't patch the master correctly. Could a new rpm build of 2015.8 be created with this fix? |
|
@deuscapturus OK thanks, I'm continuing to look into this and should have a fix today. |
|
OK, the updates are being fetched, but it appears my code for checking out the fetched updates appears not to be working. Should be easy enough to narrow down. |
|
@deuscapturus See #31903, it's a one-line change that you can apply in-place if you like to test whether or not it works for you. |
|
git pillar updates are working again with #31903 I will now wait and see that the original issue is also resolved. Thank you @terminalmage for you support on this issue! |
|
Thanks for the confirmation @deuscapturus! |
This pull request rewrites how salt.utils.gitfs handles locking. It also makes
two changes to how git_pillar/winrepo handle checking out a branch:
_pillarmaster funcs from attempting to checkout at the same time. If a lock is in place, the master will wait a short period of time and try again, until the lock is removed. After the lock is gone, there should be no need to checkout the repo again since the prior instance which obtained the lock will have updated the repo. And, thanks to item 1 above, since the SHAs will now match, no checkout will be performed once the lock that was previously in place has cleared.Fixes #31293.
EDIT: should also fix #29239.