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

time to load and update CT is excessive #88

Closed
pixelzoom opened this issue Mar 20, 2020 · 14 comments
Closed

time to load and update CT is excessive #88

pixelzoom opened this issue Mar 20, 2020 · 14 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 20, 2020

On Slack, multiple people have reported that CT takes a long time to load. @samreid reports 1.5 min to load, and it's taking me as much as 5 minutes.

@jonathanolson explained:

Looks like the result is ~13.5MB of JSON. We could restart CT if we want to minimize that. That's being shipped EVERY update

Slack discussion on dev-public channel:

Chris Malley 3:47 PM
Should we start pruning on a regular basis, like only keep the last N sets of results?

Michael Kauzmann:house_with_garden: 3:49 PM
Or make the json more sparse?

Chris Malley 3:49 PM
Some approach that doesn’t allow the payload to just continue growing until CT needs to be restarted. More sparse JSON would presumably allow us to keep more test results (larger N). But it would still eventually bog down.

Also note that dealing with the growing JSON size by restarting CT isn't a great solution, because we lose all history. It would be preferable to prune the history.

Assigning to @ariel-phet to prioritize and assign.

@ariel-phet
Copy link

@jonathanolson said there were potentially some straightforward improvements that should be able to be made.

He will work on this for about half a day and if her runs into complications he and myself will re-discuss timing and priority.

Marking high priority currently.

@jonathanolson
Copy link
Contributor

After some work, I think the restructuring of data and processing is significant enough (it's coupled) where it would be worth deciding on whether the current output of CT is ideal, or if we should change it.

@ariel-phet and @KatieWoe, are things like the "un-collapsed" view, the percentage of "passed" tests and other things useful? Would we want any additional features? It may be worth a brief meeting to see what directions we want to take CT in.

@KatieWoe
Copy link

It is useful, as it lets me know if something is being consistently missed/unfinished, and if so, what that is.

@ariel-phet
Copy link

@jonathanolson lets plan to discuss this topic at dev meeting - if we are doing some CT improvements, it would be good to get the entire team involved.

@KatieWoe we will plan to invite you to the zoom meeting when this issue comes up (currently dev meeting is still planned for Thursday, so plan to be available in the 12-1:30 time frame if possible)

@jbphet
Copy link
Contributor

jbphet commented Mar 26, 2020

Discussed in 3/26/2020 dev meeting, here is our "wish list":

Top items identified through Google doc brainstorm and vote process:

  • Run faster (and therefore complete more tests) (KW, JG)
  • Have greater granularity of what should be supported, e.g. does this sim include support for phet-io state? If not, don’t test that aspect, so that we don’t have “false failures”. It should be possible to update this information without restarting CT.
  • Better UI for viewing sim errors/knowing which tests have run
  • Make it possible to develop/run/test CT locally and easier for other devs to contribute

Other items from that process:

  • load and display results faster
  • Be able to set a sim for “thorough” versus “rudimentary” testing in order to save time
  • use a preexisting testing framework (don’t reinvent the wheel!)
  • greater scope of browsers
  • A single lint error in one repo shouldn’t turn an entire column red--obscures other problems
  • Determine if there is an issue assigned for reported failure
  • report shas/better metadata with errors
  • training for QA team on how to administer and how it works

@jonathanolson will digest this a bit and make some recommendations on how to approach making these improvements, such as whether to make incremental improvements or go for a more major overhaul.

@jbphet jbphet assigned ariel-phet and unassigned KatieWoe Mar 26, 2020
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 31, 2020

To the bullet list about, let's add "robustness". If there's a problem, CT has a tendency to fail hard.

For example, here's what I went through this morning... CT was crashing when I visited https://bayes.colorado.edu/continuous-testing/aqua/html/continuous-report.html. The error in the console was:

VM21:1 Uncaught SyntaxError: Unexpected token < in JSON at position 0
at JSON.parse ()
at XMLHttpRequest.req.onload (continuous-report.js:301)

Stack trace identified the failure as line 301 in continuos-report.js:

const data = JSON.parse( req.responseText );

Inspecting req.responseText, it's value was:

> req.responseText
"<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>503 Service Unavailable</title>
</head><body>
<h1>Service Unavailable</h1>
<p>The server is temporarily unable to service your
request due to maintenance downtime or capacity
problems. Please try again later.</p>
</body></html>

So the server was down. Instead of detecting that and displaying the above message in the browser, it just failed hard.

@jonathanolson
Copy link
Contributor

jonathanolson commented Apr 9, 2020

https://github.com/phetsims/aqua/issues/88
  Server
    Moving code to list tests to perennial (so it will update with each snapshot)
    Test priorities in this doc, so we can switch it
    Run locally
      Param for number of build threads
    Lint each repo as a separate test (no lint-everything)
    Do NOT lint repos during builds
  UI:
    Best way to separate "bugs"? How to determine with tagged issues?
    SHOW SHAs for snapshots
    Some way of marking sims/tests as "temporarily missing, not fully tested", exclamation in report?

@jonathanolson
Copy link
Contributor

I'll plan to make the above server changes, and will talk with @KatieWoe and interested parties about the UI in a design meeting.

jonathanolson added a commit that referenced this issue Apr 16, 2020
jonathanolson added a commit that referenced this issue Apr 16, 2020
jonathanolson added a commit to phetsims/perennial that referenced this issue Apr 25, 2020
jonathanolson added a commit to phetsims/joist that referenced this issue Apr 25, 2020
jonathanolson added a commit to phetsims/chipper that referenced this issue Apr 25, 2020
@Denz1994
Copy link
Contributor

From dev meeting on 04/30/20:

@jonathanolson is making improvements. No need for dev meeting.

@zepumph
Copy link
Member

zepumph commented May 1, 2020

@jonathanolson, to make CT display faster, have you thought about only loading the first row, and then async loading older rows after first draw?

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Nov 11, 2020

Load times have improved significantly since CT rewritten. I haven't had a problem with load times, but it sounds like it's still a problem for some developers.

In Slack on 11/6/2020:

Michael Kauzmann:spiral_calendar_pad: 3:41 PM
PSA: I was getting a bit tired of long load times for CT, so I made a ?maxColumns=X query parameter.
So far ?maxColumns=10 has been my favorite. I hope this helps someone else too.

Jesse Greenberg 3:42 PM
Oh cool, thanks!

@zepumph does that satisfy your needs for this issue? Can this issue be closed?

@zepumph
Copy link
Member

zepumph commented Nov 11, 2020

It doesn't feel like the robust way of handling this. It also doesn't handle the large amount of network that CT takes up to keep updating and adding rows. I don't know the details about the implementation, but I wouldn't want to make it sound like #109 was a solution to this problem--just a workaround.

@zepumph zepumph removed their assignment Nov 11, 2020
@zepumph
Copy link
Member

zepumph commented Nov 11, 2020

I think I need to change my opinion. I was surprised to see that with ?maxColumns=10 added to a CT browser tab, there was only an increase of a couple of MB after 30 minutes of having the tab open. This feels like a solution for slow load time. We may still want to make the usability more clear or automatic. Should we require ?maxColumns for CT to be usable past a couple of minutes without the tab running out of memory?

Startup vs 30 minutes of tab open:
image

@pixelzoom
Copy link
Contributor Author

This feels like a solution for slow load time.

Great! Closing.

Should we require ?maxColumns for CT to be usable past a couple of minutes without the tab running out of memory?

That's another issue that's had no forward progress, #95. I'll note your question there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants