59 bytes, text/x-review-board-request
The existing logic assumes that both mNativeSuccess and mTRRSuccess are true when both resolves worked, but if the native resolver is faster than the first TRR response then the mTRRSuccess counter is still at zero when this code hits and DNS_TRR_COMPARE then counts that as a "NativeWorked". It also works the same in the other direction, if TRR gets a result before mNativeSuccess is set, it gets counted as "TRRWorked". So, the labels are slightly misleading, but I'm not sure we can do much about the logic since when one of the responses is much faster than the other, we can't tell yet if the other resolve works or will work.
Strike that. The real problem is that TRR is counted as "attempted" even when the resolve is actually TRR blacklisted or failed in other ways before the request is sent off to the DOH server. This flaw makes "TRR attampted but failed" seem higher than expected in telemetry data.
Comment on attachment 8986422 [details] bug 1463356 do not count "not started" TRR resolves as failures https://reviewboard.mozilla.org/r/251786/#review258208 ::: netwerk/dns/TRR.h:150 (Diff revision 2) > nsresult DohEncode(nsCString &target); > nsresult PassQName(unsigned int &index); > nsresult GetQname(nsAutoCString &aQname, unsigned int &aIndex); > nsresult DohDecode(nsCString &aHost); > nsresult ReturnData(); > - nsresult FailData(); > + nsresult FailData(nsresult error); brief documentation here about special error code value
Attachment #8986422 - Flags: review?(mcmanus) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/08b8d3da8635 do not count "not started" TRR resolves as failures r=mcmanus
You need to log in before you can comment on or make changes to this bug.