New issue
Advanced search Search tips

Issue 696616 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 490870



Sign in to add a comment

Accuracy metric may not be recorded if the network quality is too slow

Project Member Reported by tbansal@chromium.org, Feb 27 2017

Issue description

In Network Quality Estimator (NQE), the accuracy metric may not be recorded in RecordAccuracyAfterMainFrame(). This may happen if the network quality was too slow, and NQE was unable to take any bandwidth sample in the last 15 seconds. This would cause GetRecentEffectiveConnectionType() to return UNKNOWN as the ECT, and ECT prediction accuracy would not be recorded.

This can be fixed by making kbps as an optional metric when computing ECT. This would be a no-op change for the method GetEffectiveConnectionType(), and would only affect GetRecentEffectiveConnectionType().
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 27 2017

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

commit 73f1a3b3440892c277003ef73862420592b93ab6
Author: tbansal <tbansal@chromium.org>
Date: Mon Feb 27 23:39:53 2017

Make kbps optional when computing effective connection type

Make kbps optional when computing effective connection type since
the kbps is always available.

This change only affects GetRecentEffectiveConnectionType(), and not
GetEffectiveConnectionType(). The former is used only for recording
accuracy.

BUG= 696616 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2717883002
Cr-Commit-Position: refs/heads/master@{#453394}

[modify] https://crrev.com/73f1a3b3440892c277003ef73862420592b93ab6/net/nqe/network_quality_estimator.cc

Labels: Merge-Request-57
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 28 2017

Labels: -Merge-Request-57 Hotlist-Merge-Review Merge-Review-57
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 4 by gov...@chromium.org, Feb 28 2017

Before we approve merge to M57, could you please confirm whether this change is well baked/verified in Canary and safe to merge to M57? Also this is reported as P3 can this wait until M58 as we're very close to M57 Stable promotion?
This is a very safe change, and affects UMA logging only. It is not super critical, but it would be very helpful in collecting the data.

Comment 6 by gov...@chromium.org, Feb 28 2017

Labels: -Merge-Review-57 Merge-Approved-57
Approving merge to M57 branch 2987 per comment #5. Please merge ASAP. Thank you.
Could you please merge your changes into M57 branch 2987 latest before 5:00 PM PT today so we can take it in for tomorrow's Beta release. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 28 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b97fc71698dc30f5a51e6c16565b85868a3d1ab

commit 3b97fc71698dc30f5a51e6c16565b85868a3d1ab
Author: Tarun Bansal <tbansal@google.com>
Date: Tue Feb 28 20:56:05 2017

Make kbps optional when computing effective connection type

Make kbps optional when computing effective connection type since
the kbps is always available.

This change only affects GetRecentEffectiveConnectionType(), and not
GetEffectiveConnectionType(). The former is used only for recording
accuracy.

BUG= 696616 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2717883002
Cr-Commit-Position: refs/heads/master@{#453394}
(cherry picked from commit 73f1a3b3440892c277003ef73862420592b93ab6)

Review-Url: https://codereview.chromium.org/2719993006 .
Cr-Commit-Position: refs/branch-heads/2987@{#722}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/3b97fc71698dc30f5a51e6c16565b85868a3d1ab/net/nqe/network_quality_estimator.cc

Labels: M-57
Status: Fixed (was: Started)

Sign in to add a comment