Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(770)

Issue 556730043: Sends the agent recorded list bytes in the pulse message. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 10 months ago by alanmorales
Modified:
4 years, 10 months ago
Reviewers:
akaiser
Visibility:
Public.

Description

Sends the agent recorded list bytes in the pulse message.

Patch Set 1 #

Total comments: 36

Patch Set 2 : Addressing comments from patch 1 #

Total comments: 2

Patch Set 3 : Addressing second round of comments #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+260 lines, -140 lines) Patch
M agent/control/pulse.go View 1 2 2 chunks +5 lines, -3 lines 0 comments Download
M agent/control/pulse_test.go View 1 1 chunk +3 lines, -3 lines 0 comments Download
M agent/stats/log_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M agent/stats/stats.go View 1 2 9 chunks +114 lines, -45 lines 2 comments Download
M agent/stats/stats_test.go View 1 2 4 chunks +32 lines, -22 lines 2 comments Download
M agent/tasks/list/depthfirstlist.go View 1 4 chunks +7 lines, -3 lines 0 comments Download
M agent/tasks/list/depthfirstlist_test.go View 8 chunks +20 lines, -11 lines 0 comments Download
M agent/tasks/list/listv3.go View 1 5 chunks +9 lines, -3 lines 0 comments Download
M agent/tasks/list/listv3_test.go View 10 chunks +28 lines, -18 lines 0 comments Download
M agent/tasks/taskprocessor.go View 1 chunk +1 line, -1 line 0 comments Download
M proto/pulse.proto View 1 chunk +1 line, -0 lines 0 comments Download
M proto/pulse_go_proto/pulse.pb.go View 3 chunks +39 lines, -30 lines 0 comments Download

Messages

Total messages: 8
alanmorales
Please take a look! thank you
4 years, 10 months ago (2019-05-14 17:25:55 UTC) #1
akaiser
Alan, you're off to a great start here! You're 90% headed in the right direction. ...
4 years, 10 months ago (2019-05-14 17:58:34 UTC) #2
alanmorales
Please take another look! https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go File agent/control/pulse.go (right): https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go#newcode120 agent/control/pulse.go:120: if ps.statsTracker != nil { ...
4 years, 10 months ago (2019-05-14 20:45:48 UTC) #3
alanmorales
https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go File agent/control/pulse.go (right): https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go#newcode120 agent/control/pulse.go:120: if ps.statsTracker != nil { On 2019/05/14 20:45:47, alanmorales ...
4 years, 10 months ago (2019-05-14 20:55:35 UTC) #4
akaiser
https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go File agent/control/pulse.go (right): https://codereview.appspot.com/556730043/diff/562770043/agent/control/pulse.go#newcode120 agent/control/pulse.go:120: if ps.statsTracker != nil { On 2019/05/14 20:55:35, alanmorales ...
4 years, 10 months ago (2019-05-14 21:04:17 UTC) #5
alanmorales
Please take another look! https://codereview.appspot.com/556730043/diff/562770043/agent/tasks/list/depthfirstlist_test.go File agent/tasks/list/depthfirstlist_test.go (right): https://codereview.appspot.com/556730043/diff/562770043/agent/tasks/list/depthfirstlist_test.go#newcode136 agent/tasks/list/depthfirstlist_test.go:136: st := stats.NewTracker(ctx) On 2019/05/14 ...
4 years, 10 months ago (2019-05-14 22:12:04 UTC) #6
akaiser
2 nits LGTM Thanks for doing this Alan! I really like how this turned out. ...
4 years, 10 months ago (2019-05-14 23:25:12 UTC) #7
alanmorales
4 years, 10 months ago (2019-05-14 23:56:16 UTC) #8
https://codereview.appspot.com/556730043/diff/566630043/agent/stats/stats.go
File agent/stats/stats.go (right):

https://codereview.appspot.com/556730043/diff/566630043/agent/stats/stats.go#...
agent/stats/stats.go:88: CopyBytes accumulator
On 2019/05/14 23:25:12, akaiser wrote:
> Don't export this, make it lowercase "copyBytes". Same with listBytes.

Done.

https://codereview.appspot.com/556730043/diff/566630043/agent/stats/stats_tes...
File agent/stats/stats_test.go (right):

https://codereview.appspot.com/556730043/diff/566630043/agent/stats/stats_tes...
agent/stats/stats_test.go:107: wg.Add(1) // Additional wg.Add(1) needed for
second record to chan
On 2019/05/14 23:25:12, akaiser wrote:
> chan.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b