1898191 - Improving HTTPS RR Handling to Mitigate Performance Regression

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Description

[Kershaw Chang [:kershaw]]

Currently, during connection establishment, we wait for HTTPS RR in two places:

nsHttpChannel: This code path is triggered only for plain HTTP loads and is used for HTTPS upgrades.
nsHttpTransaction: This is used for ECH.
Unfortunately, this implementation causes some performance regression when HTTPS RR queries are made via the native resolver.

To mitigate this regression, we have a few options:

1. Not waiting if DoH is disabled.
2. As suggested in spec 5.1, we could check if we already have A/AAAA responses. If yes, we don't wait for HTTPS RR.
3. Implement an HTTPS RR cache similar to the Alt-Svc cache.
   I believe option 2 is a MUST to do. If we choose option 1, we can avoid implementing option 3. Otherwise, we might need to implement option 3, but it is a bit more complicated.

Valentin, what do you think?
Thanks.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Thank you for looking into this, Kershaw.
I think we want option 1 for now - If DoH is disabled, and the HTTPS record is not already in the cache, don't wait for it.
I agree that option 3 is more complicated. Long term we probably want to implement something that improves the usage of HTTPS records, so either a persistent cache or a better strategy for resolving HTTPS records sooner.

I think one of the issues with HTTPS records right now is that they're less likely to be cached in the ISP's DNS cache, so they take longer to resolve than IP records. That will also probably change in the future.

Comment 2

[Kershaw Chang [:kershaw]]

Attachment: Bug 1898191 - Don't wait for HTTPS RR when DoH is disabled, r=#necko

Comment 3

[Kershaw Chang [:kershaw]]

Attachment: Bug 1898191 - Skip HTTPS RR if we already have Alt-Svc, r=#necko

Depends on D211749

Comment 4

[Kershaw Chang [:kershaw]]

Attachment: Bug 1898191 - Remove setWaitForHTTPSSVCRecord, r=#necko

We don't need it anymore.

Depends on D211750

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb3ca8188e43
Don't wait for HTTPS RR when DoH is disabled, r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/fd05c941d725
Remove setWaitForHTTPSSVCRecord, r=necko-reviewers,valentin

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/cb3ca8188e43
https://hg.mozilla.org/mozilla-central/rev/fd05c941d725