Closed Bug 1658278 Opened 4 years ago Closed 4 years ago

Figure out better confirmation process for TRR

Categories

(Core :: Networking: DNS, enhancement, P2)

Product:
Core
Component:
Networking: DNS
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
88 Branch
Tracking Flags:
Tracking Status
firefox88 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(7 files)

Bug 1658278 - Treat calls to TRRService::CompleteLookup differently based on the purpose of the TRR request r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Move member initialization to header file r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Split TRR's CONFIRM_TRYING into CONFIRM_TRYING_OK and CONFIRM_TRYING_FAILED r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Add TRR confirmation test r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Make TRRServer.registerDoHAnswers easier to extend r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Add test for multiple failures triggering confirmation r=#necko
(deleted), text/x-phabricator-request
Details
Bug 1658278 - Fix test_httpssvc_iphint.js - do not set trr.mode before test a=testonly
(deleted), text/x-phabricator-request
Details

Currently a lot of TRR requests are skipped (>10%) because the TRR service is not confirmed.
This may happen because of intermittent networking issues, coming back from suspension, startup, etc.

What we should try to do is:

  • see if increasing the number of failed requests before we go into not_confirmed state helps.
  • use normal TRR responses as confirmation (that means doing TRR requests immediately and depend on the 1.5s timeout to cancel them if the network isn't up yet leading to falling back to DNS)
  • do TRR requests in parallel, until confirmation is done. That way, when we get the confirmation response back, we can start using the other responses immediately, instead of incurring another round-trip delay.
Blocks: doh
Depends on: 1691408
                                                                 multiple-failures / context change
                                                            +-------------------------------------------+
                                                            |                                           |
                                                            |                                           |
                                                            |                                           |
        +----------------+                           +------v---------+                        +----------------+
        |                |      TRR turned on        |                |      confirm ok        |                |
        |      OFF       +--------------------------->   TRY-OK       +------------------------>   OK           |
        |                |                           |                |                        |                |
        +-------^--------+                           +--^-------------+                        +-------^--------+
                |                                       |    |                                         |
                |                                       |    |                                         |
                |TRR turned off               +---------+    |confirm fail                             |confirm ok
                |                             |              |                                         |
        +----------------+             network|              |                                         |
        |                |             change |      +-------v--------+                        +----------------+
        |   ANY-STATE    |                    |      |                |     timer              |                |
        |                |                    +------+   FAIL         +------------------------>   TRY-FAIL     |
        +----------------+                           |                |                        |                |
                                                     +------^---------+                        +----------------+
                                                            |                                          |
                                                            |                                          |
                                                            |                                          |
                                                            +------------------------------------------+
                                                                          confirm fail

This is our new design for the confirmation. It separates the trying state into TRY_OK and TRY_FAIL
This way we can more easily determine if we should make the TRR attempt or not.

Depends on: 1694949
Attachment #9207800 - Attachment description: Bug 1658278 - [WIP] Split TRR's CONFIRM_TRYING into CONFIRM_TRYING_OK and CONFIRM_TRYING_FAILED r=#necko → Bug 1658278 - Split TRR's CONFIRM_TRYING into CONFIRM_TRYING_OK and CONFIRM_TRYING_FAILED r=#necko

Also adds nsIDNSService.currentTrrConfirmationState IDL method for test use

Depends on D107666

We now pass a map object instead multiple arguments.
This way if we need to add/remove/change something we don't need to worry
about backwards compatibility and having to change all the callsites as
we do in this patch :)

Depends on D108999

Blocks: 1699660
Blocks: 1699691
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2d8c8e2d2e6a Treat calls to TRRService::CompleteLookup differently based on the purpose of the TRR request r=necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/6630ba532e23 Move member initialization to header file r=necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/7c05c181ad22 Split TRR's CONFIRM_TRYING into CONFIRM_TRYING_OK and CONFIRM_TRYING_FAILED r=necko-reviewers,nhnt11,dragana,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/216852c6cf14 Add TRR confirmation test r=necko-reviewers,dragana,nhnt11 https://hg.mozilla.org/integration/autoland/rev/75da5d4751d1 Make TRRServer.registerDoHAnswers easier to extend r=necko-reviewers,dragana,nhnt11 https://hg.mozilla.org/integration/autoland/rev/4e6e499248e5 Add test for multiple failures triggering confirmation r=necko-reviewers,dragana,nhnt11
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e8ee87ef82c3 Fix test_httpssvc_iphint.js - do not set trr.mode before test a=testonly
Regressions: 1703194
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: