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

Fix instance checking in daily pipeline #1270

Merged
merged 2 commits into from May 14, 2021
Merged

Fix instance checking in daily pipeline #1270

merged 2 commits into from May 14, 2021

Conversation

akremin
Copy link
Member

@akremin akremin commented May 13, 2021

Summary

This PR only impacts one function desispec.workflow.utils.check_running() used in the daily pipeline to check for other instances of itself. It didn't work in a cron environment because it checked for the python script name in the full process command of every running process, but cron launches a bash process with the python command in it. Since the bash process had a different PID, it always exited incorrectly thinking there was another python process.

This change checks that the command be from python or the script itself rather than bash. It also checks that the other instance is from the same user (which I think is the correct thing to do and certainly proved useful when testing it as my user last night in parallel with the official pipeline under the desi user).

Usage of scripts/functions doesn't change and no other functions are impacted.

Testing

I tested this last night under my user with cron job:
*/30 0-7,20-23 * * * source ${HOME}/bin/desi_daily_test_env.sh && nohup desi_daily_proc_manager --dry-run &>${HOME}/daily-${TIMESTAMP}.log &

The logs indicate it did the correct thing. The first job woke up and continued running. Cron jobs after woke up and exited, citing the PID of the first instance. I killed that instance and the next cron job started running and future jobs exiting citing the new PID as already running.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 27.67% when pulling 43bfebc on fix_check_running into 05cd4cd on master.

@sbailey sbailey merged commit a8322f2 into master May 14, 2021
@sbailey sbailey deleted the fix_check_running branch June 29, 2021 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants