Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2668403003:
NQE: Do not record correlation if metric is missing (Closed)
DescriptionNQE: Do not record correlation if metric is missing
In Network Quality Estimator (NQE), do not record the correlation
metric if any of the underlying metric is unavailable. Also, do not
record the correlation metric if there was a connection change
recently. These changes ensure that the metric is recorded only when
valid values were available.
BUG=688198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
Review-Url: https://codereview.chromium.org/2668403003
Cr-Commit-Position: refs/heads/master@{#448032}
Committed: https://chromium.googlesource.com/chromium/src/+/10d16356dad16ba47a8b708d45aad2f291dabfff
Patch Set 1 : ps #
Total comments: 2
Patch Set 2 : ryansturm comments #
Messages
Total messages: 23 (16 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== NQE: Do not record correlation if metric is missing BUG= ========== to ========== NQE: Do not record correlation if metric is missing BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== NQE: Do not record correlation if metric is missing BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== NQE: Do not record correlation if metric is missing In Network Quality Estimator (NQE), do not record the correlation metric if any of the underlying metric is unavailable. Also, do not record the correlation metric if there was a connection change recently. These changes ensure that the metric is recorded only when valid values were available. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
tbansal@chromium.org changed reviewers: + ryansturm@chromium.org
ryansturm: ptal. Thanks.
lgtm % nit https://codereview.chromium.org/2668403003/diff/20001/net/nqe/network_quality... File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2668403003/diff/20001/net/nqe/network_quality... net/nqe/network_quality_estimator.cc:652: if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() == Move this if above the UseTrasportRTT block so that we don't need to run all the extra code like FitInKBitsPerMetricsBits. Keep lines 657 and 658 below where they are though.
https://codereview.chromium.org/2668403003/diff/20001/net/nqe/network_quality... File net/nqe/network_quality_estimator.cc (right): https://codereview.chromium.org/2668403003/diff/20001/net/nqe/network_quality... net/nqe/network_quality_estimator.cc:652: if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() == On 2017/02/02 23:06:16, Ryan Sturm wrote: > Move this if above the UseTrasportRTT block so that we don't need to run all the > extra code like FitInKBitsPerMetricsBits. Keep lines 657 and 658 below where > they are though. Done.
Description was changed from ========== NQE: Do not record correlation if metric is missing In Network Quality Estimator (NQE), do not record the correlation metric if any of the underlying metric is unavailable. Also, do not record the correlation metric if there was a connection change recently. These changes ensure that the metric is recorded only when valid values were available. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== NQE: Do not record correlation if metric is missing In Network Quality Estimator (NQE), do not record the correlation metric if any of the underlying metric is unavailable. Also, do not record the correlation metric if there was a connection change recently. These changes ensure that the metric is recorded only when valid values were available. BUG=688198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
The CQ bit was checked by tbansal@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ryansturm@chromium.org Link to the patchset: https://codereview.chromium.org/2668403003/#ps40001 (title: "ryansturm comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486083613751080,
"parent_rev": "70160d7fc805edef062cdc623873dce9ce1e5d82", "commit_rev":
"e30fb07d058896508ef21008f4f11223ff92b275"}
The CQ bit was unchecked by commit-bot@chromium.org
Prior attempt to commit was detected, but we were not able to check whether the issue was successfully committed. Please check Git history manually and re-check CQ or close this issue as needed.
The CQ bit was checked by tbansal@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486146097298450,
"parent_rev": "18ddddd18ab835e13d69cc78e108a2f55a4a5f30", "commit_rev":
"10d16356dad16ba47a8b708d45aad2f291dabfff"}
Message was sent while issue was closed.
Description was changed from ========== NQE: Do not record correlation if metric is missing In Network Quality Estimator (NQE), do not record the correlation metric if any of the underlying metric is unavailable. Also, do not record the correlation metric if there was a connection change recently. These changes ensure that the metric is recorded only when valid values were available. BUG=688198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== NQE: Do not record correlation if metric is missing In Network Quality Estimator (NQE), do not record the correlation metric if any of the underlying metric is unavailable. Also, do not record the correlation metric if there was a connection change recently. These changes ensure that the metric is recorded only when valid values were available. BUG=688198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Review-Url: https://codereview.chromium.org/2668403003 Cr-Commit-Position: refs/heads/master@{#448032} Committed: https://chromium.googlesource.com/chromium/src/+/10d16356dad16ba47a8b708d45aa... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/10d16356dad16ba47a8b708d45aa... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
