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

Publish data store updates #3389

Merged
merged 8 commits into from
Dec 3, 2019

Conversation

dwsutherland
Copy link
Member

@dwsutherland dwsutherland commented Sep 28, 2019

These changes partially address #3207
Closes #3014

Dependant PR: cylc/cylc-uiserver#95

Functional/Done:

Utility included (have been using it for testing):
cylc-subscribe and the publish of the protobuf topics: i.e. PbWorkflow data element, which can be subscribed to via the topic workflow:

(flow) sutherlander@cortex-vbox:~$ cylc run --hold noz
            ._.                                                       
            | |                 The Cylc Suite Engine [8.0a1]         
._____._. ._| |_____.           Copyright (C) 2008-2019 NIWA          
| .___| | | | | .___|   & British Crown (Met Office) & Contributors.  
| !___| !_! | | !___.  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
!_____!___. |_!_____!  This program comes with ABSOLUTELY NO WARRANTY.
      .___! |              It is free software, you are welcome to    
      !_____!             redistribute it under certain conditions;   
                        see `COPYING' in the Cylc source distribution. 
                                                                       

*** listening on tcp://cortex-vbox:43078/ ***
*** publishing on tcp://cortex-vbox:43031 ***

To view suite server program contact information:
 $ cylc get-suite-contact noz

Other ways to see if the suite is still running:
 $ cylc scan -n 'noz' cortex-vbox
 $ cylc ping -v --host=cortex-vbox noz
 $ ps -opid,args 13994  # on cortex-vbox
(flow) sutherlander@cortex-vbox:suite$ cylc subscribe noz workflow
Connecting to tcp://cortex-vbox:43031
Received:  workflow
{
    "stamp": "sutherlander|noz@1569677013.108348",
    "id": "sutherlander|noz",
    "name": "noz",
    "status": "held",
    "host": "cortex-vbox",
    "port": 43078,
    "owner": "sutherlander",
    "tasks": [
        "sutherlander|noz|bar",
        "sutherlander|noz|baz",
        "sutherlander|noz|foo",
.
.
.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included (needed?).
  • No documentation update required.

@dwsutherland dwsutherland added this to the cylc-8.0a2 milestone Sep 28, 2019
@dwsutherland dwsutherland self-assigned this Sep 28, 2019
@dwsutherland dwsutherland changed the title Publish data store updates [WIP] Publish data store updates Sep 28, 2019
@dwsutherland dwsutherland added POC Proof of Concept and removed POC Proof of Concept labels Sep 28, 2019
@cylc cylc deleted a comment Sep 28, 2019
bin/cylc-subscribe Outdated Show resolved Hide resolved
@dwsutherland dwsutherland mentioned this pull request Oct 6, 2019
6 tasks
@dwsutherland
Copy link
Member Author

dwsutherland commented Oct 9, 2019

Made some significant changes to how data-store updates are managed in the WorkflowService/cylc-flow:

  • Changes/Deltas are generated as elements with only updated fields populated.
  • Deltas are gathered and applied to the local data-store (so the same application of changes can be used with the UIS)
  • Deltas are published as a multi-part messages.

cylc subscribe [OPTIONS] REG [TOPIC] can be used the the following topics:

  • edges
  • families
  • family_proxies
  • jobs
  • tasks
  • task_proxies
  • workflow

for example:

(flow) sutherlander@cortex-vbox:cylc-flow$ cylc subscribe noz jobs
Connecting to tcp://cortex-vbox:43063
Received:  jobs
{
    "time": 1570594467.2935598,
    "checksum": "4230119217",
    "deltas": [
        {
            "stamp": "sutherlander|noz|20190101T00|baz|1@1570594467.2907376",
            "id": "sutherlander|noz|20190101T00|baz|1",
            "submitNum": 1,
            "state": "ready",
            "taskProxy": "sutherlander|noz|20190101T00|baz",
            "batchSysName": "background",
            "host": "localhost",
            "jobLogDir": "/home/sutherlander/cylc-run/noz/log/job/20190101T00/baz/01",
            "script": "sleep 15; echo \"$CYLC_TASK_NAME\"",
            "name": "baz",
            "cyclePoint": "20190101T00"
        },
        {
            "stamp": "sutherlander|noz|20190102T00|baa|1@1570594467.292317",
            "id": "sutherlander|noz|20190102T00|baa|1",
            "state": "succeeded",
            "finishedTime": "2019-10-09T04:14:26Z",
            "messages": [
                "succeeded"
            ]
        }
    ]
}
Received:  jobs
{
    "time": 1570594467.8002422,
    "checksum": "1037995818",
    "deltas": [
        {
            "stamp": "sutherlander|noz|20190102T00|bar|1@1570594467.7975347",
            "id": "sutherlander|noz|20190102T00|bar|1",
            "submitNum": 1,
            "state": "ready",
            "taskProxy": "sutherlander|noz|20190102T00|bar",
            "batchSysName": "background",
            "host": "localhost",
            "jobLogDir": "/home/sutherlander/cylc-run/noz/log/job/20190102T00/bar/01",
            "script": "sleep 15; echo \"$CYLC_TASK_NAME\"",
            "name": "bar",
            "cyclePoint": "20190102T00"
        }
    ]
}
Received:  jobs
{
    "time": 1570594468.7934058,
    "checksum": "1102876461",
    "deltas": [
        {
            "stamp": "sutherlander|noz|20190101T00|baz|1@1570594468.7916427",
            "id": "sutherlander|noz|20190101T00|baz|1",
            "state": "submitted",
            "submittedTime": "2019-10-09T04:14:28Z",
            "batchSysJobId": "3656",
            "messages": [
                "submitted"
            ]
        }
    ]
}
.
.
.

More info to come..

@cylc cylc deleted a comment Oct 10, 2019
@dwsutherland dwsutherland removed the WIP label Oct 10, 2019
@kinow
Copy link
Member

kinow commented Oct 10, 2019

GIFrecord_2019-10-10_171502

Looks promising! 🎉

@cylc cylc deleted a comment Oct 13, 2019
@dwsutherland dwsutherland changed the title [WIP] Publish data store updates Publish data store updates Oct 14, 2019
@kinow
Copy link
Member

kinow commented Oct 14, 2019

This is a WIP details to come...

Just checking, is it still WIP?

@dwsutherland
Copy link
Member Author

This is a WIP details to come...

Just checking, is it still WIP?

Just changed it to fully fledged PR..
However, there is still tests to add and there may be more changes if the UIS requires it..

@dwsutherland dwsutherland force-pushed the publish-data-store-updates branch 2 times, most recently from 35a472f to 77607df Compare October 18, 2019 10:45
@cylc cylc deleted a comment Oct 18, 2019
@dwsutherland dwsutherland force-pushed the publish-data-store-updates branch 2 times, most recently from 4aa6d00 to a13d538 Compare October 25, 2019 04:18
@cylc cylc deleted a comment Oct 25, 2019
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Tested the full stack, working as expected 🚀!

Thanks @dwsutherland this is a pretty gnarly part of the system, you've managed to get it going nicely.

Testing I did find one oddity which is that with cycling suites, cylc subscribe will list tasks each time a new cycle is added which is superfluous as the task definitions haven't changed. I guess we're getting an erroneous but harmless delta?

@dwsutherland
Copy link
Member Author

Testing I did find one oddity which is that with cycling suites, cylc subscribe will list tasks each time a new cycle is added which is superfluous as the task definitions haven't changed. I guess we're getting an erroneous but harmless delta?

This is probably intentional, because with new edges you get new task proxies, and with new task proxies you get changes to task definition elements (as the relationships have to be set (IDs added to the proxies field)).

@kinow
Copy link
Member

kinow commented Nov 28, 2019

Two approvals, but probably good idea to get @hjoliver eyes on this too.

@dwsutherland
Copy link
Member Author

Two approvals, but probably good idea to get @hjoliver eyes on this too.

Got one more commit, API version, module names, change log.

@hjoliver
Copy link
Member

Two approvals, but probably good idea to get @hjoliver eyes on this too.

Yeah, I'll prioritize this ASAP

@dwsutherland dwsutherland force-pushed the publish-data-store-updates branch 3 times, most recently from 70cb61b to 38fd61e Compare November 29, 2019 01:48
@dwsutherland
Copy link
Member Author

dwsutherland commented Nov 29, 2019

Ok, finishing touches in:

  • API version to scan items (for UI Server, and cylc scan to know API version)
  • Module names changed (as discussed on Riot), done here to avoid an extra pair of breaking changes and API version increment.
  • Change entry added

@hjoliver
Copy link
Member

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- cylc/flow/data_store_mgr.py  24
- cylc/flow/network/publisher.py  3
- cylc/flow/daemonize.py  4
- cylc/flow/network/subscriber.py  5
         

Complexity decreasing per file
==============================
+ cylc/flow/network/client.py  -1
         

See the complete overview on Codacy

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Alright, this is going in (tested as working, the update/sync methodology seems sound and robust, and no code issues that I've seen). 🎉

@hjoliver hjoliver merged commit d45547f into cylc:master Dec 3, 2019
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.

Should the new SuiteRuntimeClient extend ZMQClient?
4 participants