Event handler enhancements #1503

matthewrmshin · 2015-06-16T14:34:02Z

New and additional settings to suite.rc:

Configurable remote task job logs retrieval.
Configurable task event email.
General task event handlers.
Task event handler can now be script templates.
Event handlers can now retry on failure.

Improve diagnostics to job-activity.log.

Report command and return code as well as out and err of subprocess activities.
Clearer labels for activities.

Suite run time database, new task_job_logs table:

Store location, size and modified time of the job logs.
(Location column can be updated by our anticipated future job log housekeeping functionality.)
It is an improved version of what is currently updated by rose suite-hook rose_prune and rose suite-log --update, and viewed by Rose Bush.

Other re-factor and tidy up:

Store submit/run and event handler (re)try info in context objects.
Store the context and result of a shell command (for invoking under cylc.mp_pool) in a single object.
- All information in one place.
- Easy to log.
- Easy to retry command.
Use table name constants for task proxy DB inserts/updates.

This closes #181, addresses bullet points 1 to 3 of #992, and begins the 1st step for #1052.

matthewrmshin · 2015-06-16T14:35:09Z

(This is not ready yet, but it needs some initial sound bites.)

hjoliver · 2015-06-17T03:53:07Z

bin/cylc-job-logs-retrieve

+    # N.B. "scp" does not have a "max-size" option.
+    check_call([
+        "rsync", "-a", "--rsh=" + ssh_tmpl, "--max-size=" + opts.max_size,
+        source + "/", target])


Do we need to worry about this: #1494 (comment)

It should be OK with our normal naming conventions.

We can now have ":" in filenames: YYYY-MM-DDThh:mm

I think we only use the basic format YYMMDDThhmm under log/job/, or don't we?

% find ~/cylc-run/bar/log/job/ /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00 /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/NN /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01 /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01/job.status /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01/job.err /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01/job.out /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01/job /home/oliverh/cylc-run/bar/log/job/2015-08-08T00:00/foo/01/job-activity.log

I guess you have something like [cylc]cycle point format defined in your suite?

(The : character would probably make PATH-like search rather interesting?)

cycle point format = %Y-%m-%dT%H:%M

That's the official "extended format" with minutes under ISO 8601. Which is nice and readable. My point is we're not disallowing this, or transforming in log paths (maybe we should?)

I'll create a test for this to ensure this is okay.

Test added. I think rsync only has a problem if we have a colon right after a string that can be a valid URI scheme. It should not happen in our case here.

hjoliver · 2015-06-17T05:27:32Z

I get a traceback on shutdown:

Processing 1 queued command(s)
  + set_stop_cleanly
Traceback (most recent call last):
  File "/home/oliverh/cylc/cylc.git/lib/cylc/run.py", line 75, in main
    server.run()
  File "/home/oliverh/cylc/cylc.git/lib/cylc/scheduler.py", line 957, in run
    proc_pool.handle_results_async()
  File "/home/oliverh/cylc/cylc.git/lib/cylc/mp_pool.py", line 168, in handle_results_async
    callback(value)
  File "/home/oliverh/cylc/cylc.git/lib/cylc/task_proxy.py", line 630, in job_submission_callback
    for line in result.out.splitlines(True):
AttributeError: 'NoneType' object has no attribute 'splitlines'

hjoliver · 2015-06-17T06:04:45Z

lib/cylc/cfgspec/suite.py

+                'mail retry delays'           : vdr( vtype='interval_minutes_list', default=[] ),
+                'mail smtp'                   : vdr( vtype='string' ),
+                'mail to'                     : vdr( vtype='string' ),
+                },


This is like the original cylc event hooks config - one handler and list of events for it - which we changed because it doesn't allow different handlers for different events. Is this intended to replace the current system (just left in for back compat) or do you want to keep both?

I intend to keep both, so we can have the best of both worlds.

Can you add setting of default values for "handlers" and "handler events" as has been done recently for hooks under #1501

Yes, in theory. However, this change is an add-on to the [runtime][TASK][event hooks] section, which does not currently have the equivalent of #1501, so I am a bit reluctant to bundle this in as part of this change. I agree that we should have a follow on to implement #1501 functionality for [runtime][TASK][event hooks] and [runtime][TASK][events].

Raised as #1529

matthewrmshin · 2015-06-17T12:18:16Z

All problems raised should be addressed. I'll document the proposed change and add some new tests next.

hjoliver · 2015-06-19T03:40:01Z

Matt, is this now done as far as you're concerned? If so, I'll take another look (was distracted by zombie processes today, sorry).

matthewrmshin · 2015-06-19T09:00:16Z

Matt, is this now done as far as you're concerned? If so, I'll take another look (was distracted by zombie processes today, sorry).

The functionality should be OK. I have yet to find time to add something the CUG and some automated tests for the new functionality.

matthewrmshin · 2015-06-23T10:28:06Z

Branch re-based. I have now added docs and some automated tests.

matthewrmshin · 2015-06-24T15:28:30Z

@hjoliver this is now ready for review.

hjoliver · 2015-06-25T06:31:05Z

(sorry for the delay - the authentication branch just absorbed another day ... I should get on to this tomorrow)

matthewrmshin · 2015-06-25T08:13:35Z

(sorry for the delay - the authentication branch just absorbed another day ... I should get on to this tomorrow)

Don't worry. It has taken me forever to get this branch done in the first place.

matthewrmshin · 2015-06-25T11:59:38Z

Quick summary of what's new:

[runtime]
    [[root]]
        script = true
        [[[remote]]]
            host = my-favourite-host
            # Switch on job log retrieval for this family/task
            retrieve job logs = True
            # Specify initial + retry delays, default is to run immediately
            retrieve job logs retry delays = PT5S, PT30S, PT1M
        [[[events]]]
            # Email on these events
            mail events = submission failed, submission retry, failed, retry
            # You can even modify "from:", "to:" and the SMTP server's "host:port"
            # mail from = notifications@$HOSTNAME
            # mail to = $USER
            # mail smtp = localhost:25
            # Can specify an initial + retry delays
            # mail retry delays = PT0S
            #
            # Generic event handlers for specified events
            handler = hello-event-handler '%(point)s' '%(name)s' '%(submit_num)s' '%(event)s'
            handler events = succeeded, failed
            # Can specify an initial + some retry delays
            # This setting applies to specific event handlers as well.
            # handler retry delays = PT0S

Each event handler can be a template script or a command like before. For a template, the following sub-strings will be substituted:

%(event)s: event name
%(suite)s: suite name
%(point)s: cycle point
%(name)s: task name
%(submit_num)s: job submit number associated with event
%(id)s: task ID, i.e. task name dot cycle point
%(message)s: event message

(The classic interface is equivalent to command '%(event)s' '%(suite)s' '%(id)s' '%(message)s'.)

matthewrmshin · 2015-06-25T12:00:37Z

(Just broken the tests slightly by my latest change. Will fix soon.)

matthewrmshin · 2015-06-25T13:56:03Z

Branch re-based, broken tests fixed.

hjoliver · 2015-06-25T23:48:56Z

The test battery passes in my environment.

Although ... tests/events/09-task-event-mail.t gets screwed by a $HOME/.mailrc file that sets an smtp server ... can this be handled somehow in the test?

hjoliver · 2015-06-26T01:38:48Z

doc/cug.tex

-the suite host.
+although they can be retrieved by right-clicking on the task in the GUI. If you
+want the job logs pulled back to the suite host automatically, you can set
+\lstinline@[[[remote]]]retrieve job log=True@:


This looks funny when process because it is directly above a long-format code block. Maybe expand it more in text, "you can set 'retrieve job log = True' under the task [[[remote]]] section:"?

And address other comments from reviewer.

matthewrmshin · 2015-07-01T10:46:19Z

Long hyperlink fixed. Branch re-based.

matthewrmshin · 2015-07-01T10:50:51Z

I have updated my what's new quick summary.

matthewrmshin · 2015-07-01T14:03:56Z

@arjclark please review-2.

Ensure that event handler entries go to the job-activity.log with the correct submit number.

arjclark · 2015-07-06T08:54:54Z

lib/cylc/rundb.py

@@ -173,9 +173,69 @@ class CylcSuiteDAO(object):
    MAX_TRIES = 100
    TABLE_BROADCASTS = "broadcasts"
    TABLE_TASK_JOBS = "task_jobs"
+    TABLE_TASK_JOB_LOGS = "task_job_logs"


Is this new table going to pose a problem for restarts on older suites, or will it be created automatically if missing?

Tables are created automatically if they are missing.

Cool, thought so but needed to confirm.

arjclark · 2015-07-06T12:53:26Z

Getting an error from the test battery under ./tests/database/00-simple.t, looks like the test just needs updating:

-CREATE TABLE task_jobs(cycle TEXT, name TEXT, submit_num INTEGER, is_manual_submit INTEGER, try_num INTEGER, time_submit TEXT, time_submit_exit TEXT, submit_status TEXT, time_run TEXT, time_run_exit TEXT, run_signal TEXT, run_status TEXT, user_at_host TEXT, batch_sys_name TEXT, batch_sys_job_id TEXT);
+CREATE TABLE task_jobs(cycle TEXT, name TEXT, submit_num INTEGER, is_manual_submit INTEGER, try_num INTEGER, time_submit TEXT, time_submit_exit TEXT, submit_status INTEGER, time_run TEXT, time_run_exit TEXT, run_signal TEXT, run_status INTEGER, user_at_host TEXT, batch_sys_name TEXT, batch_sys_job_id TEXT);

arjclark · 2015-07-06T12:54:26Z

^ presumably accidentally broken by matthewrmshin@c015453

matthewrmshin · 2015-07-06T13:07:05Z

Test fixed.

arjclark · 2015-07-07T07:45:49Z

Looks good to me. Tests passing in my environment, follow on work captured in additional issues #1528 and #1529

Event handler enhancements

matthewrmshin self-assigned this Jun 16, 2015

matthewrmshin added this to the soon milestone Jun 16, 2015

matthewrmshin force-pushed the hook branch 2 times, most recently from 5f29071 to 562beb1 Compare June 16, 2015 15:15

hjoliver reviewed Jun 17, 2015
View reviewed changes

matthewrmshin force-pushed the hook branch from de38180 to f699bf2 Compare June 17, 2015 11:01

matthewrmshin force-pushed the hook branch from 4824c9c to 3412fad Compare June 18, 2015 08:30

matthewrmshin force-pushed the hook branch 3 times, most recently from 9c515cd to fc86552 Compare June 23, 2015 10:27

matthewrmshin force-pushed the hook branch from ddb9f26 to c3626cc Compare June 25, 2015 13:55

hjoliver mentioned this pull request Jun 26, 2015

Edit run - confirmation prompts #1401

Merged

hjoliver reviewed Jun 26, 2015
View reviewed changes

matthewrmshin added 5 commits July 1, 2015 11:44

task event handler enhancement: fix typos in doc

b89a96d

And address other comments from reviewer.

event handler enhancements: review feedbacks

25cfaea

retrieve job logs: don't ignore hidden files

d56945f

Test job-logs-retrieve OK with extended date-time

ef5f84b

doc: fix long hyperlink

fe3a260

matthewrmshin force-pushed the hook branch from 6bc0718 to fe3a260 Compare July 1, 2015 10:45

matthewrmshin assigned arjclark and unassigned matthewrmshin Jul 1, 2015

matthewrmshin mentioned this pull request Jul 1, 2015

rose bush: take advantage of new tables in cylc suite run time database file metomi/rose#1646

Closed

cylc.rundb: task_jobs table: integer for statuses

c015453

matthewrmshin mentioned this pull request Jul 3, 2015

Take advantage of cylc event handler enhancements metomi/rose#1649

Merged

cylc.task_proxy: command_log: correct submit num

0981026

Ensure that event handler entries go to the job-activity.log with the correct submit number.

arjclark reviewed Jul 6, 2015
View reviewed changes

t: database 00: fix schema output

e4ec590

This was referenced Jul 6, 2015

suite.rc: add suite level events #1528

Closed

siterc: allow setting of [runtime][TASK][event hooks] and [events] #1529

Closed

arjclark added a commit that referenced this pull request Jul 7, 2015

Merge pull request #1503 from matthewrmshin/hook

5938836

Event handler enhancements

arjclark merged commit 5938836 into cylc:master Jul 7, 2015

matthewrmshin modified the milestones: next release, soon Jul 7, 2015

matthewrmshin deleted the hook branch July 7, 2015 12:44

matthewrmshin mentioned this pull request Oct 13, 2015

rose suite-hook: Prevent transfer of very large log files metomi/rose#1300

Closed

matthewrmshin mentioned this pull request Jun 27, 2016

Builtin event handler functionalities #992

Closed

7 tasks

Event handler enhancements #1503

Event handler enhancements #1503

Conversation

matthewrmshin commented Jun 16, 2015

matthewrmshin commented Jun 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjoliver commented Jun 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewrmshin commented Jun 17, 2015

hjoliver commented Jun 19, 2015

matthewrmshin commented Jun 19, 2015

matthewrmshin commented Jun 23, 2015

matthewrmshin commented Jun 24, 2015

hjoliver commented Jun 25, 2015

matthewrmshin commented Jun 25, 2015

matthewrmshin commented Jun 25, 2015

matthewrmshin commented Jun 25, 2015

matthewrmshin commented Jun 25, 2015

hjoliver commented Jun 25, 2015

Choose a reason for hiding this comment

matthewrmshin commented Jul 1, 2015

matthewrmshin commented Jul 1, 2015

matthewrmshin commented Jul 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjclark commented Jul 6, 2015

arjclark commented Jul 6, 2015

matthewrmshin commented Jul 6, 2015

arjclark commented Jul 7, 2015