[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Hello Will Berkeley, Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9326 to review the following change. Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 221 insertions(+), 71 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9326/1 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9326 to look at the new patch set (#3). Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 237 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9326/3 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 3: (2 comments) SGTM except nits. Release failure looks unrelated. The TSAN failure may be legit, though. http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@104 PS3, Line 104: Unable to collect metrics to diagnostics log Add a bit saying we'll retry in `kWaitBetweenFailures` seconds. http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@118 PS3, Line 118: this seems redundant with the "only include changed metrics" above Referring to `opts.only_modified_in_or_after_epoch` below? -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 20 Feb 2018 22:27:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.h File src/kudu/server/diagnostics_log.h: http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.h@39 PS3, Line 39: DiagnosticsLog(std::string log_dir, > warning: function 'kudu::server::DiagnosticsLog::DiagnosticsLog' has a defi Done http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@104 PS3, Line 104: Unable to collect metrics to diagnostics log > Add a bit saying we'll retry in `kWaitBetweenFailures` seconds. Done http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@118 PS3, Line 118: this seems redundant with the "only include changed metrics" above > Referring to `opts.only_modified_in_or_after_epoch` below? Done -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 20 Feb 2018 23:44:07 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9326 to look at the new patch set (#4). Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 240 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9326/4 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9326 to look at the new patch set (#5). Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 241 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9326/5 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 19:05:15 + Gerrit-HasComments: No
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 7: (1 comment) nice clean lifecycle http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc@76 PS7, Line 76: return Status::OK(); return s; -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 21:27:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc@96 PS7, Line 96: MonoDelta::FromSeconds(60) should this be configurable? -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 21:29:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc@76 PS7, Line 76: return Status::OK(); > return s; nice catch http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc@96 PS7, Line 96: MonoDelta::FromSeconds(60) > should this be configurable? eh I think it's rare enough that it's not really worth the flag (even a hidden one) -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 22 Feb 2018 00:06:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9326 to look at the new patch set (#8). Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 241 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9326/8 -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Todd Lipcon has removed a vote on this change. Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 8: Verified+1 IWYU failure from parent patch -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 23 Feb 2018 00:10:43 + Gerrit-HasComments: No
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. Patch Set 8: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9326/7/src/kudu/server/diagnostics_log.cc@96 PS7, Line 96: MonoDelta::FromSeconds(60) > eh I think it's rare enough that it's not really worth the flag (even a hid alright -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 23 Feb 2018 02:07:51 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9326 ) Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase .. KUDU-2297 (part 1): refactor metrics log out of ServerBase This is preparing for putting more information into the metrics log instead of just metrics snapshots. The logging code will get more complex as it gains features, so this makes a new class for it. This is slightly incompatible since I also changed the name on disk to 'diagnostics' instead of 'metrics' and updated the documentation, but I am not aware of people using this in the wild. So long as we release-note it, I think it's reasonable to expect any sophisticated operators to adjust their scripting accordingly. Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Reviewed-on: http://gerrit.cloudera.org:8080/9326 Tested-by: Todd Lipcon Reviewed-by: Mike Percy --- M docs/administration.adoc M src/kudu/server/CMakeLists.txt A src/kudu/server/diagnostics_log.cc A src/kudu/server/diagnostics_log.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 6 files changed, 241 insertions(+), 73 deletions(-) Approvals: Todd Lipcon: Verified Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d Gerrit-Change-Number: 9326 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
