909865 - Add telemetry for usage of speculative HTTP connections

Status: RESOLVED FIXED

(Core :: Networking, defect, P5)

Description

[Guilherme Gonçalves [:ggp]]

This will be useful for measuring the effectiveness of the Seer (bug 881804).

Comment 1

[Guilherme Gonçalves [:ggp]]

Attachment: Add telemetry for speculative HTTP connection use.

Comment 2

[u408661]

Comment on attachment 796156 [details] [diff] [review]
Add telemetry for speculative HTTP connection use.

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

So I'm not 100% sure (hence the needinfo? to mcmanus), but I think we could actually end up double-counting some nsHalfOpenSockets with this patch, in the (likely rare) case when we try to use a half-open socket that ends up being abandoned for some reason. Assuming I'm right, I think a better approach would be to add mWasSpeculative to nsHalfOpenSocket, and do all the telemtry accumulation in the dtor, based on the values of mWasSpeculative and mSpeculative.

Comment 3

[u408661]

Patrick - see comment #2 for my question.

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 796156 [details] [diff] [review]
Add telemetry for speculative HTTP connection use.

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

I don't think this patch is measuring what you think its measuring :)

It is measuring the fraction of connection objects that were opened speculatively that were then paired with a transaction before the handshake completed. I suspect you want to measure the fraction of connection objects that were opened speculatively that are paired with at least one transaction before being closed.

Specifically, once a connection is established the half open gets dtorred but the underlying socket(s) not necessarily so - they can end up in the idle connection pool waiting for a request. And with the seer that's a perfectly reasonable series of events. (right now there is a 5 second timeout on them.. nick I would think we might want to make that higher given the seer patterns) search for "small non-zero idle timeout" to find the code where that happens. so the http conn is going to have to inherit any "was speculative" flag at that point (for both the case where it goes idle and the case where it is matched up immediately).. and the telemetry can be done when the connection is reclaimed-and-not-reusable(do it there rather than the dtor so you don't have threading issues)

Comment 5

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 6

[Andrew Creskey [:acreskey]]

Until general speculative connections are fixed (bug 1816678) we will be limited in what we are gathering.

We have httpconnmgr_unused_speculative_conn, "How many speculative connections are made needlessly", which is uniformly reporting the value 1

Also some network predictor speculative probes:
PREDICTOR_TOTAL_PRECONNECTS - 0
PREDICTOR_TOTAL_PRECONNECTS_CREATED - 1.15
PREDICTOR_TOTAL_PRECONNECTS_UNUSED - 1.15
PREDICTOR_TOTAL_PRECONNECTS_USED - 1.15
(I don't understand the results but maybe it's because the connections are not being made.)

What we can add now is tracking of how often our current speculative-connection pool is maxed out.

Comment 7

[Andrew Creskey [:acreskey]]

Attachment: Bug 909865 - Add telemetry for usage of speculative HTTP connections r=#necko-reviewers

We want to have a better understanding of speculative connect failures and socket limit exhaustion before the fix to Bug 1813618 lands.

Comment 8

[Andrew Creskey [:acreskey]]

Attachment: spec_connect_data_review.md

Please see attached request for data review.

Comment 9

[Chris H-C :chutten|PTO (back Oct 23)]

Comment on attachment 9321369 [details]
spec_connect_data_review.md

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection can be controlled through Firefox's Preferences.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, [e-mail] and [e-mail] are responsible.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does the data collection use a third-party collection tool?

No.

---

Result: datareview+

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63cc5bd9d512
Add telemetry for usage of speculative HTTP connections r=necko-reviewers,valentin

Comment 11

[Cristian Tuns]

Backed out for causing build bustages in nsHttpConnectionMgr.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnectionMgr.cpp:3531:7: error: use of undeclared identifier 'glean'

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/191734ba52d5
Add telemetry for usage of speculative HTTP connections r=necko-reviewers,valentin

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/191734ba52d5

Comment 14

[Andrew Creskey [:acreskey]]

I put up a query for the speculative connect outcomes:
https://sql.telemetry.mozilla.org/queries/91069#225437
About 94% abort rate, but note that the vast majority of these are technically the 'early connect' behaviour of nsHttpChannel, not strictly 'speculative connections.'