994302 - disable speculative connections for talos

Status: RESOLVED DUPLICATE of bug 996871

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

in bug 992611 we found that speculative connections was causing a lot of random failures/leaks, it is a good idea to do this for talos:
https://bugzilla.mozilla.org/show_bug.cgi?id=992611#c14

here is the change:
https://hg.mozilla.org/releases/mozilla-aurora/rev/4fdcd09ca66d

getting started with talos:
https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code

add the pref here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l237 (note the format will be slightly different).

Comment 1

[Patrick]

I don't know a lot about talos, but I'd still like to help out with this.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Patrick, welcome to Mozilla!

I tried to outline the steps to do this above.  This should be an easy fix, a single line actually.  I am in irc #ateam, as :jmaher- I would love to help you there or if you are not comfortable with IRC, ask some questions here in the bug.

Follow the steps to getting started with Talos and post a patch in the bug.  Talos is in Mecurial, so post a patch as a diff:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ

Comment 3

[Patrick]

Hey, everything is set up and talos is working fine. But I'm not really sure how to correctly translate 

' user_pref("network.http.speculative-parallel-limit", 0); '

over to PrefConfiguration.py, other than that everything has been going pretty smoothly.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

glad to hear you got setup on your own!  This is a great question and can be confusing.

If you look at the prefs here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l237

They are in this format:
         'browser.bookmarks.max_backups': 0,

we want to convert user_pref("network.http.speculative-parallel-limit", 0); into that format.  

The format in the talos definition is almost identical except that it stores all the preferences/values in a data structure and not defining each preference/value pair inside of user_pref().

This comment is intentionally not answering it directly- I am sure you can figure this out.

One thing to do prior to uploading the patch is verify you can run the ts test.
./talos --app `which firefox` -a ts_paint --develop

that will open and close the browser really fast 20 times and then generate some results.  Verify it runs the same before and after your change.

Thanks for working on this!

Comment 5

[Patrick]

Alright I've updated the code, and I'm going to start to post the patch as diff.

Comment 6

[Patrick]

Scratch that, I've had some difficulties getting mercurial to work, if you've got some time free tomorrow could we meet in an irc for a bit to chat about it?

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I will be on irc from about 5am pacific time until about 2pm pacific time- find me in #ateam as :jmaher, or post your mercurial issues here in the bug.

Comment 8

[Patrick]

Attachment: bug994302.patch

I messed around with mercurial a little bit more and I think that this is the correct format for uploading it to bugzilla.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

The patch format looks good. However, you should include the bug number in your commit message. No need to upload a new patch for it, just something to keep in mind going forward :). The link below provides some more info on writing good commit messages:
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment

Comment 10

[Patrick]

Alright thanks for the heads up, I'll keep that in mind.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8407962 [details] [diff] [review]
bug994302.patch

Review of attachment 8407962 [details] [diff] [review]:
-----------------------------------------------------------------

you can make the top line of the message:
Bug 994302 - disable speculative connections for talos

::: talos/PerfConfigurator.py
@@ +259,5 @@
>          'hangmonitor.timeout': 0,
>          'network.proxy.http': 'localhost',
>          'network.proxy.http_port': 80,
>          'network.proxy.type': 1,
> +	'network.http.speculative-parallel-limit', 0

at the end of the line you are missing a ,

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

Patrick, I am just checking in with you to see if you have figured this out yet?  Feel free to ask questions here in bugzilla or on irc!

Comment 13

[Mike Hoye [:mhoye]]

Just tapping on the glass here - Patrick, are you still working on this?

Comment 14

[Ed Morley [:emorley]]

This ended up being fixed as part of bug 996871, since it was later found to be blocking bug 1026970 and so was needed more urgently.