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

Call os.fork less to avoid race conditions #54259

Merged
merged 8 commits into from Aug 24, 2019

Conversation

@dwoz
Copy link
Contributor

commented Aug 19, 2019

What issues does this PR fix or reference?

#54153
#54219

Previous Behavior

  • The minions' _threaded_return and _threaded_multi_return methods did a double fork so that we could join the parent process immediately, not blocking anything.
  • Schedules run by masters and minions did a double fork so that we could join the parent process immediately, not blocking anything.

New Behavior

  • The minions do not call join right away on job subprocesses instead they track and reap them when they are finished.
  • Masters and minions do not join schedule processes right away, instead the Schedule class tracks it's subprocesses and provide a method for the users of this class to call periodically to reap them.

Tests written?

Yes - Unit tests added for new classes

Commits signed with GPG?

Yes

@dwoz dwoz requested a review from saltstack/team-core as a code owner Aug 19, 2019
@pull-assigner pull-assigner bot requested a review from cmcmarrow Aug 19, 2019
salt/minion.py Show resolved Hide resolved
@Akm0d
Akm0d approved these changes Aug 19, 2019
salt/minion.py Show resolved Hide resolved
salt/minion.py Show resolved Hide resolved
salt/minion.py Outdated Show resolved Hide resolved
salt/utils/schedule.py Outdated Show resolved Hide resolved
salt/minion.py Outdated Show resolved Hide resolved
salt/utils/process.py Outdated Show resolved Hide resolved
salt/utils/process.py Outdated Show resolved Hide resolved
@dwoz dwoz force-pushed the dwoz:less_forking branch 2 times, most recently from 7d75747 to 6d684f5 Aug 21, 2019
@dwoz dwoz added the WarRoom label Aug 21, 2019
@dwoz dwoz force-pushed the dwoz:less_forking branch from dd658c8 to 39954d7 Aug 22, 2019
dwoz added 7 commits Aug 19, 2019
@dwoz dwoz force-pushed the dwoz:less_forking branch from 39954d7 to 316ec78 Aug 22, 2019
@waynew
waynew approved these changes Aug 22, 2019
@dwoz dwoz force-pushed the dwoz:less_forking branch from 40fc660 to 58eb8b6 Aug 23, 2019
@dwoz dwoz force-pushed the dwoz:less_forking branch from 58eb8b6 to bf4d0dd Aug 23, 2019
@dwoz dwoz merged commit 5838851 into saltstack:2019.2.1 Aug 24, 2019
22 of 24 checks passed
22 of 24 checks passed
ci/py2/windows2016 This commit cannot be built
Details
ci/py3/windows2016 This commit is being built
Details
WIP Ready for review
Details
ci/docs This commit looks good
Details
ci/lint This commit looks good
Details
ci/py2/amazon2 This commit looks good
Details
ci/py2/centos6 This commit looks good
Details
ci/py2/centos7 This commit looks good
Details
ci/py2/centos7/tcp This commit looks good
Details
ci/py2/debian8 This commit looks good
Details
ci/py2/debian9 This commit looks good
Details
ci/py2/fedora29 This commit looks good
Details
ci/py2/ubuntu1604 This commit looks good
Details
ci/py2/ubuntu1604/tcp This commit looks good
Details
ci/py2/ubuntu1804 This commit looks good
Details
ci/py3/amazon2 This commit looks good
Details
ci/py3/centos7 This commit looks good
Details
ci/py3/centos7/tcp This commit looks good
Details
ci/py3/debian8 This commit looks good
Details
ci/py3/debian9 This commit looks good
Details
ci/py3/fedora29 This commit looks good
Details
ci/py3/ubuntu1604 This commit looks good
Details
ci/py3/ubuntu1604/tcp This commit looks good
Details
ci/py3/ubuntu1804 This commit looks good
Details
dwoz added a commit to dwoz/salt that referenced this pull request Aug 24, 2019
Call os.fork less to avoid race conditions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.