1229178 - Make comparing scheduling changes easier

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

[Armen [:armenzg]]

I've been trying to use mach taskcluster-graph to determine exactly what my scheduling changes are.

I've been using vimdiff and grepping to help me but it is still pretty dificult:
./mach taskcluster-graph --pushlog-id=99534 --project=try '--message=try: -b do -p linux64_tc -u all -t none' --owner=[e-mail] --revision-hash=d846b1d922038241d8fc7bd9952a6c8a0f2469ea --extend-graph | grep -v "task" | grep -v "2015" | grep -v "2016"  > after_changes.txt

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

I'm planning to add some nice utility options to `mach taskgraph` that will make this sort of thing a lot easier.

Comment 3

[Michael Shal [:mshal]]

'mach taskcluster-graph' used to have a --dry-run flag for this purpose, but that is not currently available in 'mach taskgraph'. I am trying to add it back in bug 1277665.

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

--dry-run doesn't do any comparison (plus other issues on that bug).

I have some work in progress on this bug, but it's lower-priority than removing the legacy kind right now.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

By the way, I think the right approach here is to build a library for generating timestamps which bases everything on a single "now" for a single run.  So all tasks in a run will have the same "created" timestamp.  Then the diffing utility can baseline the timestamps in its inputs to the same date, resulting in meaningful, but equivalent, datestamps in the diff.

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1229178: modify --json output to contain a single object

Review commit: https://reviewboard.mozilla.org/r/61378/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61378/

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Sorry for the churn.  This doesn't fix the issue, but it's a patch I've been carrying around for a bit that at least generates task-graphs locally in the same format that they appear in the decision-task artifacts.

Comment 8

[Michael Shal [:mshal]]

(In reply to Dustin J. Mitchell [:dustin] from comment #4)
> --dry-run doesn't do any comparison (plus other issues on that bug).

The point of --dry-run is to get the output consistent between runs so that you can use diff to do the comparison. Eg:

$ ./mach [taskcluster command] --dry-run > output1.txt
$ (apply patch to some task definition file, such as changing a task's tier)
$ ./mach [taskcluster command] --dry-run > output2.txt
$ diff -u output1.txt output2.txt
--- output1.txt	2016-06-30 17:20:23.200305024 -0400
+++ output2.txt	2016-06-30 17:21:31.478279696 -0400
@@ -21399,7 +21399,7 @@
                 "build_product": "firefox", 
                 "build_type": "dbg", 
                 "index": {
-                    "rank": 0
+                    "rank": 1467021545
                 }, 
                 "locations": {
                     "build": "public/build/target.tar.bz2", 
@@ -21420,8 +21420,7 @@
                     "machine": {
                         "platform": "linux32"
                     }, 
-                    "symbol": "B", 
-                    "tier": 2
+                    "symbol": "B"
                 }, 
                 "treeherderEnv": [
                     "production", 

With the taskIds and dates stubbed out, it is easy to use standard tools to compare the changes in the graph when the task definitions are changed. Does this make sense? I feel like we might be talking about different problems.

Comment 9

[Michael Shal [:mshal]]

(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> By the way, I think the right approach here is to build a library for
> generating timestamps which bases everything on a single "now" for a single
> run.  So all tasks in a run will have the same "created" timestamp.  Then
> the diffing utility can baseline the timestamps in its inputs to the same
> date, resulting in meaningful, but equivalent, datestamps in the diff.

By a "run" do you mean a single execution of the 'mach taskgraph' command? If so I don't think this addresses the problem.

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8766508 [details]
Bug 1229178: modify --json output to contain a single object

https://reviewboard.mozilla.org/r/61378/#review58476

This looks fine to me, though it does not actually address the problem.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3dcecf06e37b
modify --json output to contain a single object r=mshal

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

Thanks for the review.  I know it doesn't fix the issue (see comment 7), but it was a useful patch in my queue so I thought I'd get it landed.

I think we are talking about the same thing.  My intent is to have an easy way to diff taskgraphs, just as you said in comment 8.  So you would run

./mach taskgraph -p parameters.yml tasks > before.json
<make some changes to the task-graph inputs>
./mach taskgraph -p parameters.yml tasks > after.json

and then

  diff -u {before,after}.json

will produce output containing the resulting changes to the task graph, without any spurious difference.  I've thought better of my suggestion in comment 5, which required use of a "diffing utility", e.g., `./mach taskgraph diff {before,after}.json`, that knew how to re-baseline the datestamps.  Instead, I now think that taskgraphs should represent all datestamps as offsets from the created time.  Those offsets would be converted into actual datestamps only just before calling `queue.createTask`, so any JSON files would contain offsets only.  This has a lot of additional advantages, beyond easy diffing:

 - if the decision task's execution takes longer than the "slop" allowed by the `queue.createTask` method (15 minutes), the current implementation will result in errors from createTask.  Updating the timestamps only when calling that method would avoid this issue
 - martianwars' Action tasks could apply the same logic to creating tasks, even though the create call might be hours or days later
 - it's easy to include and edit the offsets in task definitions without doing date/time arithmetic

The other spurious difference you'll see right now is in task labels -- they are of the form `TaskLabel==<slugid>` when generated by the legacy kind.  That's one of many problems with the legacy kind that will be remedied by its deletion (bug 1281004).

If you want to chip in, that'd be awesome -- and I think implementing the relative datestamps might be a good point to do so.  I think they should be implemented as a JSON structure similarly to {'task-reference': '..'} -- something like {'relative-datestamp': '14 days'}.  If you don't have time, I'll get to it eventually :)

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

Kalpesh just filed this in bug 1284005

Comment 14

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/3dcecf06e37b

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Given Kalpesh's work in bug 1284005, there's almost nothing left to do here -- once the legacy kind is gone, task graphs will be directly comparable.  We do need to sort labels, though!

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Duh, helps if you look at the source before making bugzilla comments.  It already sorts keys!