New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 787942 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Speculative QUIC connect to stale DNS results

Project Member Reported by mge...@chromium.org, Nov 22 2017

Issue description

We can use stale DNS results to start the QUIC handshake at the same time as we do a new host resolution, and then use the session only if its IP address shows up in the new results.

Internal bug: b/32734392
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/693336310e49f25fc167ed28d4feebe7f8fe4c78

commit 693336310e49f25fc167ed28d4feebe7f8fe4c78
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Fri Jan 05 19:53:15 2018

Move ResolveStaleFromCache() from HostResolverImpl to HostResolver

This API is now needed outside of DNS code, so it needs to be accessible
from the HostResolver.

Bug: 787942
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I53ed78a77bfbb607fcfad42b3c59d371c114bbfc
Reviewed-on: https://chromium-review.googlesource.com/848036
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527358}
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/components/cronet/stale_host_resolver.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/components/cronet/stale_host_resolver.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/content/network/proxy_resolver_factory_mojo_unittest.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/host_resolver.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/host_resolver_impl.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/host_resolver_mojo.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/host_resolver_mojo.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/dns/mock_host_resolver.h
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/proxy/proxy_resolver_v8_tracing_unittest.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/proxy/proxy_resolver_v8_tracing_wrapper_unittest.cc
[modify] https://crrev.com/693336310e49f25fc167ed28d4feebe7f8fe4c78/net/socket/socks_client_socket_unittest.cc

Comment 2 by mge...@chromium.org, Jan 16 2018

Here's my roadmap for this:

1. Add the new state, and split up OnIOComplete(). No behavior change.
2. Add the use_stale_dns_ option to QuicStreamFactory, add tests for cases that should work the same as they already do.
3. Handle cases where stale DNS is used. Maybe more than one CL.
4. Add all relevant UMA histograms.
5. Plumb through to Cronet.

Sometime after discussions are resolved, make any needed changes (I don't expect anything too intrusive) and plumb through to Chrome.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86b0e51f6efca6f227f87cad915cb944c4b14e2c

commit 86b0e51f6efca6f227f87cad915cb944c4b14e2c
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Mon Jan 22 16:48:10 2018

Prepare QuicStreamFactory::Job for racing host resolution

This CL contains two main changes:
1. Split up OnIOComplete() into separate callbacks for different events
completing. This will allow future changes to wait on both events at
once.
2. Add a new state for "confirm connection". It currently has all the
logic that was previously in the "connect complete" state. This will be
used in the future to wait until both host resolution and connection are
done.

These changes do not affect any externally visible behavior.

Bug: 787942
Change-Id: I53e4c6a7b040fc9a2fa506914ff2e5b7de2e68b1
Reviewed-on: https://chromium-review.googlesource.com/869030
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530883}
[modify] https://crrev.com/86b0e51f6efca6f227f87cad915cb944c4b14e2c/net/quic/chromium/quic_stream_factory.cc

Comment 4 by mge...@chromium.org, Mar 23 2018

Owner: pauljensen@chromium.org
Cc: ianswett@chromium.org pauljensen@chromium.org rch@chromium.org
Owner: renjietang@chromium.org
Status: Assigned (was: Started)
Re-assigning to renjietang@ as there's no progress on this thread for a while. It'd be nice for Renjie to pick up the work. 
Cc: zhongyi@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0cb4a2c444e544df98dfadfcaf188b64bd547a0

commit a0cb4a2c444e544df98dfadfcaf188b64bd547a0
Author: Renjie <renjietang@chromium.org>
Date: Wed Sep 26 23:37:30 2018

Add experiment of racing stale dns on connection.

In QuicStreamFactory::Job, chances are host resolution won't return synchronously. While waiting for the host resolution callback, the stale host resolver will be queried, and a connection from the stale host result will be established but not confirmed. After host resolution returns, the two IP addresses will be compared. If they match, the connection will be confirmed. Otherwise the connection from stale host will be closed and connection from the fresh resolution will be established.

Bug: 787942
Change-Id: I7a5f78ec42e97778d7c7d200f4d8ac894611fd19
Reviewed-on: https://chromium-review.googlesource.com/1186112
Commit-Queue: Renjie Tang <renjietang@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594522}
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/components/network_session_configurator/browser/network_session_configurator.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/components/network_session_configurator/browser/network_session_configurator_unittest.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/http/http_network_session.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/http/http_network_session.h
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/http/http_proxy_client_socket_wrapper_unittest.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/log/net_log_event_type_list.h
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory.h
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory_fuzzer.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory_peer.cc
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory_peer.h
[modify] https://crrev.com/a0cb4a2c444e544df98dfadfcaf188b64bd547a0/net/quic/quic_stream_factory_test.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d2d8d91b568c4a0c1948dfa288f42a9d27747ed

commit 8d2d8d91b568c4a0c1948dfa288f42a9d27747ed
Author: Renjie <renjietang@chromium.org>
Date: Sat Sep 29 00:29:03 2018

Validate hosts right after host resolution returns.

This CL is a fix/improvement on crrev.com/c/1186112. Previously in the situation where host resolution returned first but connection from stale host hasn't called back yet, nothing was done. And this could cause a crash because when QuicChromiumClientSession::OnCryptoHandshakeEvent() was finally called, OnConnectComplete callback would run. In the case where the stale host doesn't match with the fresh host, the session would be closed and deleted. So anything after the callback would fail.

To solve this, hosts should be validated immedaitely when host resolution returns. This change also improves performance because now if the two hosts are different, the Job won't waste time waiting for the invalid stale connection to finish. To further avoid accessing a deleted session, when a connection is deemed invalid, CloseSessionOnErrorLater() is used instead of CloseSessionOnError().

Bug: 787942
Change-Id: I4bdb0e5b810445134e4ef305ba401d330adb2f79
Reviewed-on: https://chromium-review.googlesource.com/1250140
Commit-Queue: Renjie Tang <renjietang@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595269}
[modify] https://crrev.com/8d2d8d91b568c4a0c1948dfa288f42a9d27747ed/net/quic/quic_stream_factory.cc
[modify] https://crrev.com/8d2d8d91b568c4a0c1948dfa288f42a9d27747ed/net/quic/quic_stream_factory_test.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/249aa3d681cfd901cafe53ea8429bf54a7ba7be2

commit 249aa3d681cfd901cafe53ea8429bf54a7ba7be2
Author: Renjie <renjietang@chromium.org>
Date: Wed Oct 10 20:53:08 2018

add UMA metrics for the stale host racing experiment.

Bug: 787942
Change-Id: I93c1dfea283927ae49aa0963e4fe2f0905242eb5
Reviewed-on: https://chromium-review.googlesource.com/c/1254564
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Renjie Tang <renjietang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598493}
[modify] https://crrev.com/249aa3d681cfd901cafe53ea8429bf54a7ba7be2/net/quic/quic_stream_factory.cc
[modify] https://crrev.com/249aa3d681cfd901cafe53ea8429bf54a7ba7be2/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94b907168c4133e87042a82144ad91a63324ad17

commit 94b907168c4133e87042a82144ad91a63324ad17
Author: Renjie <renjietang@chromium.org>
Date: Thu Oct 18 21:03:19 2018

Add cronet experiment option to allow stale DNS racing experiment on cronet.

Bug: 787942
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ia9d210d3d427de332dae4f7ad7fb988239819926
Reviewed-on: https://chromium-review.googlesource.com/c/1289158
Commit-Queue: Renjie Tang <renjietang@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600901}
[modify] https://crrev.com/94b907168c4133e87042a82144ad91a63324ad17/components/cronet/url_request_context_config.cc
[modify] https://crrev.com/94b907168c4133e87042a82144ad91a63324ad17/components/cronet/url_request_context_config_unittest.cc

Sign in to add a comment