Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issue 1074523002: Disable logging of failed SPDY PINGs in a timemax bucket, and update histogram bucketing strategy f… (Closed)

Created:
5 years, 8 months ago by Bryan McQuade
Modified:
5 years, 8 months ago
Reviewers:
Ryan Hamilton
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable logging of failed SPDY PINGs in a timemax bucket, and update histogram bucketing strategy for Net.SpdyPing.RTT to match the strategy of Net.TCP_Connection_Latency. * timemax is int64max, whereas histograms use int values, so this doesn't end up working well and can break certain analysis operations such as computing the average SPDY PING time. Additionally, counts for failed SPDY PINGs are recorded in the Net.SpdySession.ClosedOnError enum histogram, so there's no need for us to log them in a special bucket in the RTT histo as well. * Also change the histogram bucketing strategy for Net.SpdyPing.RTT to match the strategy of Net.TCP_Connection_Latency since these are measuring similar things and it's useful to be able to compare TCP RTTs with application-layer RTTs. BUG=475129 Committed: https://crrev.com/4ce665a95aeacb2bf1c83e110784226cc2536cc2 Cr-Commit-Position: refs/heads/master@{#324318}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M net/spdy/spdy_session.cc View 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1074523002/1
5 years, 8 months ago (2015-04-08 18:47:25 UTC) #3
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 8 months ago (2015-04-08 18:47:27 UTC) #5
Ryan Hamilton
lgtm
5 years, 8 months ago (2015-04-08 21:37:04 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1074523002/1
5 years, 8 months ago (2015-04-08 21:37:57 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-04-09 00:44:00 UTC) #9
commit-bot: I haz the power
5 years, 8 months ago (2015-04-09 00:44:57 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/4ce665a95aeacb2bf1c83e110784226cc2536cc2
Cr-Commit-Position: refs/heads/master@{#324318}

Powered by Google App Engine
This is Rietveld 408576698