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

localhost background/at jobs: record host name #2849

Conversation

matthewrmshin
Copy link
Contributor

Ensure that localhost background/at jobs are recorded as running on the host name of the current suite host, rather than just localhost. On suite restart on a different suite host, this allows the restart logic to correctly poll the status of the background/at jobs that may still be running on the previous suite host.

Perfect companion of #2809.

@matthewrmshin matthewrmshin added this to the next release milestone Nov 8, 2018
@matthewrmshin matthewrmshin self-assigned this Nov 8, 2018
@matthewrmshin
Copy link
Contributor Author

@oliver-sanders @kinow please review or reassign to someone near you.

@oliver-sanders
Copy link
Member

Perfect companion of #2809.

+1

@oliver-sanders
Copy link
Member

#2809 will require an associated change to work with this PR, I'm guessing the following would do the job:

 if (
-    itask.task_host == 'localhost' and
+    itask.task_host == get_host() and
-    itask.summary['batch_sys_name'] in ['background', 'at'] and
     itask.state.status in TASK_STATUSES_ACTIVE
 ):

@matthewrmshin any thoughts?

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, comments ditto. Travis-CI happy 🎉

Tested using master first, and got

kinow@kinow-VirtualBox:~/cylc-run/five/log/suite$ cat log.20181109T004933Z 
2018-11-09T00:49:33Z INFO - Suite server: url=https://kinow-VirtualBox:43092/ pid=3126
2018-11-09T00:49:33Z INFO - Run: (re)start=0 log=1
2018-11-09T00:49:33Z INFO - Cylc version: 7.7.1-352-g395e2
2018-11-09T00:49:33Z INFO - Run mode: live
2018-11-09T00:49:33Z INFO - Initial point: 20130808T0000Z
2018-11-09T00:49:33Z INFO - Final point: 20130812T0000Z
2018-11-09T00:49:33Z INFO - Cold Start 20130808T0000Z
2018-11-09T00:49:33Z INFO - [prep.20130808T0000Z] -submit-num=1, owner@host=localhost

Then switched to this pull request's branch, and confirmed that the host had changed.

2018-11-09T00:55:14Z INFO - Suite starting: server=kinow-VirtualBox:43006 pid=5271
2018-11-09T00:55:14Z INFO - Cylc version: 7.7.1-332-g19ad1
2018-11-09T00:55:14Z INFO - Run mode: live
2018-11-09T00:55:14Z INFO - Initial point: 20130808T0000Z
2018-11-09T00:55:14Z INFO - Final point: 20130812T0000Z
2018-11-09T00:55:14Z INFO - Cold Start 20130808T0000Z
2018-11-09T00:55:14Z INFO - [prep.20130808T0000Z] -submit-num=1, owner@host=kinow-VirtualBox

And just to make sure, had a look inside the sqlite database, and confirmed at least the task_jobs table had the new value too, instead of localhost.

...
sqlite> select * from task_jobs;
20130808T0000Z|prep|1|0|1|2018-11-09T00:55:14Z|2018-11-09T00:55:15Z|0|2018-11-09T00:55:15Z|2018-11-09T00:55:15Z||0|kinow-VirtualBox|background|5328
20130808T0000Z|foo|1|0|1|2018-11-09T00:55:17Z|2018-11-09T00:55:18Z|0|2018-11-09T00:55:18Z|2018-11-09T00:55:18Z||0|kinow-VirtualBox|background|5421
...

@kinow
Copy link
Member

kinow commented Nov 9, 2018

Happy to merge, but it got conflicted 😢

Ensure that localhost background/at jobs are recorded as running on the
host name of the current suite host, rather than just `localhost`. On
suite restart on a different suite host, this allows the restart logic
to correctly poll the status of the background/at jobs that may still be
running on the previous suite host.
@matthewrmshin matthewrmshin force-pushed the persist-localhost-bg-at-host-as-suite-hostname branch from 19ad1dd to 4006ff8 Compare November 9, 2018 09:46
@matthewrmshin
Copy link
Contributor Author

Branch rebased to resolve conflicts.

@kinow
Copy link
Member

kinow commented Nov 9, 2018

Just waiting for Travis-CI to hit the merge button 🎉

@kinow kinow merged commit a9861cd into cylc:master Nov 9, 2018
@matthewrmshin matthewrmshin deleted the persist-localhost-bg-at-host-as-suite-hostname branch November 9, 2018 10:37
@cylc cylc deleted a comment Nov 9, 2018
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Nov 9, 2018
@oliver-sanders oliver-sanders mentioned this pull request Nov 9, 2018
3 tasks
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Nov 9, 2018
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Nov 14, 2018
oliver-sanders added a commit to metomi/rose that referenced this pull request Nov 16, 2018
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Nov 21, 2018
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Nov 21, 2018
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.

3 participants