Closed Bug 1512011 Opened 1 year ago Closed 3 months ago

[talos] Replace mozhttpd with wptserve

Categories

(Testing :: Talos, enhancement, P1)

Version 3
enhancement

Tracking

(firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: raphael, Assigned: b4hand)

References

(Regressed 1 open bug)

Details

Attachments

(1 file)

According to bug 1150512 the webserver used in Talos "mozhttpd" is considered unsupported. davehunt suggested to replace mozhttpd with wptserve for Python projects in mozilla-central.
Priority: -- → P3
Assignee: nobody → bforehand
Status: NEW → ASSIGNED
Priority: P3 → P1

Please see comments in phab, I'm seeing a strange PNG file pop-up during tp5o with the patch applied.

Also there are some failures in the try push (Comment 2), unsure if they are related to the server change or not. Here's a try push of talos tests on OSX only with latest central (with patch NOT applied) to see if the failures are pre-existing:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86237e947d4e0327e670c7ecfcb99d5b2193d2f4

It seems the pageset that is being loaded tries to load some images within an iframe. This was not a problem when using mozhttpd because it seems to wrap everything with a header.

When switching to the wptserve server, we no longer do this and now Firefox is being prompted to download the images since their headers are now application/octet-stream.

I followed the steps here and was able to get the page to load without prompting for a download.

Thanks Benjamin, much appreciated!

So if I'm understanding correctly, there are two options here:

  1. Update the very old tp5n.zip pageset archive (on tooltool) and add a '.headers' file for the offending page (I believe it is 'bild.de').
  • or -
  1. We remove the particular offending page ('bild.de') from the list of URLs used by tp5o [0]. I think that may let us run the existing tests and solve the problem, without updating the pageset archive (but need to test that out).

[0] https://searchfox.org/mozilla-central/rev/1fe0cf575841dbf3b7e159e88ba03260cd1354c0/testing/talos/talos/tests/tp5o.html#38

:mconley, davehunt, do you have any preference here as to which way we should go? The tp5n.zip pageset is quite old, will it really matter if we remove one test page?

Flags: needinfo?(mconley)
Flags: needinfo?(dave.hunt)

The source linked in comment 4 returns a content type of JSON, is this really what we're doing for the PNG in the existing tests? My preference would be to match the headers that we're currently returning, if this is as simple as adding a .headers file to the tp5 pageset. There's a bigger discussion on replacing the tp5 pageset, but I don't think that should block progress here.

Out of interest, have we tested the performance impact of switching mozhttpd to wptserve? If there's an impact expected then we should notify the performance sheriffs as they can expect a new baseline.

Flags: needinfo?(dave.hunt)

I agree that the tp5n set is old. In fact, it's probably so old that we might be losing coverage due to newer web features and web APIs since the original recording was made. I'd honestly prefer a new set of recordings - that'd be useful for making the tests more reflective for Fission too.

Flags: needinfo?(mconley)

Thanks guys. Updating the tp5 pageset is quite a big task, as tp5 uses raw html source files on disk, not mitmproxy playback recordings. My understanding is updating the pageset would require modifying all source pages to remove any dynamic external links to ensure all dependencies are in the local files. Can be done of course but would be a pretty big time consuming project. Another option is to migrate talos tp5 to use mozproxy/mitmproxy and use the tp6 recordings instead. However that would also be a pretty big change but do-able of course.

Anyway I think those questions are outside of this immediate issue (replacing mozhttpd with wptserve). Looks like the preference here is to update the tp5 pageset to fix the issue that is introduced by the switch to wptserve.

:b4hand, do you want to add that to your patch (adding the .headers file)? If you get an updated page set working locally and provide a new tp5n.zip file, I can upload it to tooltool.

Flags: needinfo?(bforehand)

I think it makes more sense to change the index.html within the pageset to fix the problem, vs adding a header file. Changing the iframes to img tags works. I can provide that pageset zip too.

Flags: needinfo?(bforehand)

(In reply to Benjamin Forehand Jr[:b4hand] from comment #9)

I think it makes more sense to change the index.html within the pageset to fix the problem, vs adding a header file. Changing the iframes to img tags works. I can provide that pageset zip too.

Great thanks Benjamin!

I provided you with a pageset, could you confirm that it is uploaded to tooltool? I can run a job on Try after that. Thanks!

Please see comment 11.

Flags: needinfo?(rwood)

(In reply to Benjamin Forehand Jr[:b4hand] from comment #12)

Please see comment 11.

Thanks, uploaded the provided 'tp5n.zip' to tooltool and sent you the new manifest.

Flags: needinfo?(rwood)

Hi Benjamin, I'm just awaiting a green try run please with your latest updates, thanks!

Flags: needinfo?(bforehand)

I am still having some issues with Try. All my jobs (4) never started. I am reaching out to the firefox-ci team today to hopefully get some help. Sorry about the delay.

Flags: needinfo?(bforehand)

(In reply to Benjamin Forehand Jr[:b4hand] from comment #15)

I am still having some issues with Try. All my jobs (4) never started. I am reaching out to the firefox-ci team today to hopefully get some help. Sorry about the delay.

Thanks for the update Benjamin, if you want me to push your patch to try on your behalf later today just let me know.

Flags: needinfo?(bforehand)

I think that would work. I am stuck in a rebase/patch/someone_help land with mercurial, and if the patch just needs to be tested, please push on try for me. Thanks.

Flags: needinfo?(bforehand)
Pushed by rwood@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5dd01231a71c
[talos] Replace mozhttpd with wptserve; r=rwood,perftest-reviewers
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Regressions: 1607491
You need to log in before you can comment on or make changes to this bug.