Page MenuHomePhabricator

Phabricator task graphs need a limit on how big they draw
Closed, ResolvedPublic

Description

Steps to reproduce

Actual results

  • It draws too much in task graph's due to so many tasks being added to it as either sub or blocking tasks

Expected results

  • We should limit it so it doesn't crash or cause slowness and doesn't make it hard to view the tasks.

Upstream There doesn't seem to be an upstream task yet, but there are comments about this in https://secure.phabricator.com/T4788#183369

Event Timeline

Paladox created this task.Jul 14 2016, 12:28 AM
Restricted Application added subscribers: Zppix, TerraCodes, Aklapper. · View Herald TranscriptJul 14 2016, 12:28 AM
Restricted Application added a project: Upstream. · View Herald TranscriptJul 14 2016, 12:32 AM

quiddity also spotted It happends here https://phabricator.wikimedia.org/T126641

Quiddity updated the task description. (Show Details)Jul 14 2016, 1:29 AM
Quiddity added a subscriber: Quiddity.
kaldari added a comment.EditedJul 14 2016, 1:32 AM

Can we disable the task graph feature in the meantime? I had to turn off JavaScript in order to read the cross-wiki watchlist ticket.

Also happened on T33235, seems several Tracking-Neverending Epic Story ... are sharing it so why not high this?

mmodell closed this task as Resolved.Jul 14 2016, 4:24 AM
mmodell claimed this task.

This is now hotfixed by disabling the graph when there are more than 100 related tasks. Upstream will work on a better solution eventually but this relieves the breakage for now.

Just FYI, I fixed a bunch of these, by removing T4007 from a large number of tasks (Sorry to anyone who got spammed).

Here are before and after screenshots, of one task, just from removing T4007 as a parent.

We may want to consider removing that task enmasse, in the future. (no rush now, though).


@mmodell thanks for coordinating the hotfix!

This is now hotfixed by disabling the graph when there are more than 100 related tasks. Upstream will work on a better solution eventually but this relieves the breakage for now.

For those not subscribed to https://secure.phabricator.com/T4788:

epriestley added a revision: D16295: For now, hard limit task graph at 100 nodes.
epriestley added a commit: rP2a1b8ce85bef: For now, hard limit task graph at 100 nodes.

Rical added a subscriber: Rical.EditedJul 14 2016, 7:00 AM

The graph take a large place. We could statisticaly reduce it if we reverse horizontal and vertical axes.
Then most trees have only short horizontal branches.
If a sub task have several sub sub tasks, they are linked by a new vertical line.
In this case, these sub sub tasks can be group on successive lines.
Then most graphs become narrow.

Just FYI, I fixed a bunch of these, by removing T4007 from a large number of tasks (Sorry to anyone who got spammed).

Please revert now that this report is no longer an issue.

Just FYI, I fixed a bunch of these, by removing T4007 from a large number of tasks (Sorry to anyone who got spammed).

Please revert now that this report is no longer an issue.

Lets, not generate hundreds possibly thousands{{cn}} more un-needed emails, they should all have the Tracking-Neverending tag on them already.

Oh, one more screenshot from my laptop (I was getting multiple script timeout loading errors on dozens of tasks, with my less powerful desktop). Very guitarhero, as someone said.

Please revert now that this report is no longer an issue.

There's just a temporary hotfix in place, so that (those of us who had problems getting pages to load) can actually see a few of the tasks at all.
I think we'll need to wait and see what the possible permanent solutions are, before re-evaluating any further.

Just FYI, I fixed a bunch of these, by removing T4007 from a large number of tasks (Sorry to anyone who got spammed).

Please revert now that this report is no longer an issue.

Those were supposed to be gone anyway during the maintenance I was trying to perform in recent months (among other things to exactly prevent this issue), so basically they are now gone just without the necessary accompanying actions. Which doesn't harm anything (OTOH will make the further cleanup maintenance just slightly less comfortable).

Those were supposed to be gone anyway during the maintenance I was trying to perform in recent months

No such thing has ever been agreed with.

kaldari added a comment.EditedJul 15 2016, 12:15 AM

Can we please revert back to the previous version (before Task graphs)? I need to be able to see task parents and children, but all I get are "Task graph too large" errors. The previous simpler version was a lot better, IMO.

@kaldari I've proposed simple quick solution which would solve majority of affected tasks (not necessarily all though). However, I'm stucked between two mill-stones: Several people complain on mail notification spam. And when I proposed turning off the Phabricator notifications for couple minutes to perform the action, it hasn't been welcomed as well (one of the reasons was, there is fear of turning off Phabricator mailing system). If people could stand the mailspam knowing that it is for good reason and aim, that would be helpful...

Anyway, some better fallback shouold be coming soon. (But it won't solve the issue with bad underlaying data.)

greg added a subscriber: greg.Jul 15 2016, 1:04 AM

@kaldari see T140333 for a temp workaround. Mukunda is actively working on a fix.

@greg: Yeah, sounds like Mukunda has a good fix coming! I'll be patient in the meantime :)

greg added a comment.Jul 15 2016, 1:08 AM

Thanks and sorry.

Rical added a comment.EditedJul 15 2016, 6:15 AM

To complete T140326#2460977 we could add a rule:
A sub task with sub-sub tasks can group them in successive lines if uper task do not already group them.
In all cases we must have limits on vertical and on horizontal size.

Also we must have a detector of recursive loops with an error message to alert the user changing the task.
The user changing the task must know which new sub-task form a recursive loop.
Inform the user is better when he add each new sub-task.
Else, when the user valid all changes at end, a lot of loops can complicate the detection.

Also we must have a detector of recursive loops with an error message to alert the user changing the task.
The user changing the task must know which new sub-task form a recursive loop.
Inform the user is better when he add each new sub-task.
Else, when the user valid all changes at end, a lot of loops can complicate the detection.

That is already built in.

Rical removed a subscriber: Rical.Jul 21 2017, 9:49 AM

In https://www.mediawiki.org/wiki/Topic:Ugksfdnngocau3s8 there is a question whether it is possible to disable this fallback behavior.