You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2019/01/30 06:32:17 UTC
[kudu-CR] [server] Add start time in server description
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12304
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
11 files changed, 61 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/1
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed Dan Burkert from this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Removed reviewer Dan Burkert.
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Reviewed-on: http://gerrit.cloudera.org:8080/12304
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
17 files changed, 109 insertions(+), 12 deletions(-)
Approvals:
Kudu Jenkins: Verified
Todd Lipcon: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 12
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#2).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
11 files changed, 63 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/2
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#11).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
17 files changed, 109 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/11
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/common/wire_protocol.proto@96
PS2, Line 96: optional int64 start_time = 5;
> Do you feel strongly about storing this as a string instead of a "seconds s
Done
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/server/server_base.cc@683
PS2, Line 683: start_time_ = WallTime_Now();
> using local time here is a bit confusing since servers eventaully may run i
Done
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 10:47:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed Tidy Bot from this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Removed reviewer Tidy Bot.
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#8).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
16 files changed, 113 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/8
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 10:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_master.cc@149
PS10, Line 149: std::move
no need for 'std::move' here since the return value of a function is already a temporary (technically I think it's called a "prvalue", and so the rvalue reference argument to emplace_back will already bind to it directly, see https://en.cppreference.com/w/cpp/language/value_category )
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_tserver.cc@148
PS10, Line 148: std::move
same
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/CMakeLists.txt@259
PS10, Line 259: wire_protocol_proto
see comment elsewhere: this is a circular reference between modules that we don't want
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h@39
PS10, Line 39: } // namespace kudu
I don't think this belongs in util/pb_util.h. util/ is meant to be very generic utility code that could be shared between projects, so a backward dependency up into kudu server specific stuff like this isn't clean. (In fact, Impala pulls in the util/ directory since it shares our krpc stack)
Can you move this utility function somewhere else like common/wire_protocol.h since the PB is in wire_protocol.proto in that same package?
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h@124
PS10, Line 124: std::string ParseStartTime(const ServerRegistrationPB& reg);
nit: better to name this "StartTimeToString" rather than "Parse", since imo "parse" implies going _from_ a string format rather thna _to_ one
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 06:06:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#4).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M www/masters.mustache
M www/tablet-servers.mustache
13 files changed, 102 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/4
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 11:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_master.cc@149
PS10, Line 149:
> no need for 'std::move' here since the return value of a function is alread
Done
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/tools/tool_action_tserver.cc@148
PS10, Line 148:
> same
Done
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/CMakeLists.txt@259
PS10, Line 259: zlib)
> see comment elsewhere: this is a circular reference between modules that we
Done
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h@39
PS10, Line 39: class DescriptorPool;
> I don't think this belongs in util/pb_util.h. util/ is meant to be very gen
Done
http://gerrit.cloudera.org:8080/#/c/12304/10/src/kudu/util/pb_util.h@124
PS10, Line 124: //
> nit: better to name this "StartTimeToString" rather than "Parse", since imo
Done
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 08:06:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webserver] Add table name filter to JSON metrics
Posted by "Zhang Yifan (Code Review)" <ge...@cloudera.org>.
Zhang Yifan has uploaded a new patch set (#9) to the change originally created by Yingchun Lai. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [webserver] Add table name filter to JSON metrics
......................................................................
[webserver] Add table name filter to JSON metrics
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/diagnostics_log.cc
M src/kudu/tablet/tablet-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
6 files changed, 122 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/9
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Zhang Yifan <zh...@xiaomi.com>
[kudu-CR] [server] Add start time in server description
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 4:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/common/wire_protocol.proto@96
PS4, Line 96: optional int64 start_time = 5;
can you add a comment which indicates the unit and reference point? ie "seconds since the epoch" or "milliseconds since the epoch" or something?
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@138
PS4, Line 138: (time_t)
nit: we prefer using C++ syntax for casts: static_cast<time_t>(...)
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@479
PS4, Line 479: // Convert epoch time to localtime.
need to handle the case where 'reg.has_start_time()' is false (eg post upgrade) to not set it in the output JSON
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@663
PS4, Line 663: jw.String("start_time");
same
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc@152
PS4, Line 152: StringAppendStrftime(&time_str, "%Y-%m-%d %H:%M:%S %Z",
I think you need to wrap this block in something like:
if (registration().has_start_time()) {
...
} else {
time_str = "<unknown>";
}
since during a rolling upgrade from an earlier release we won't have a time set, and we don't want to show it as 1970-01-01
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc@154
PS4, Line 154: values.emplace_back(time_str);
nit: can std::move() this
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc@150
PS4, Line 150: string time_str;
same comment as in tool_action_master.cc
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc@153
PS4, Line 153: values.emplace_back(time_str);
nit: std::move
http://gerrit.cloudera.org:8080/#/c/12304/4/www/tablet-servers.mustache
File www/tablet-servers.mustache:
http://gerrit.cloudera.org:8080/#/c/12304/4/www/tablet-servers.mustache@58
PS4, Line 58: <td><pre><code>{{registration}} start_time: {{start_time}}</code></pre></td>
maybe breaking out the start time to a separate column would be good?
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 05:35:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] Add start time in server description
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 11: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12304/11/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
http://gerrit.cloudera.org:8080/#/c/12304/11/src/kudu/common/wire_protocol.cc@957
PS11, Line 957: if (reg.has_start_time()) {
> error: no member named 'has_start_time' in 'kudu::ServerRegistrationPB' [cl
not sure what's going on with tidybot, seems to be a buidl issue, can ignore these
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 19:40:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Zhang Yifan, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#10).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
16 files changed, 113 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/10
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Zhang Yifan <zh...@xiaomi.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#5).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
15 files changed, 110 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/5
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#6).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
15 files changed, 112 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/6
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#7).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
M www/masters.mustache
M www/tablet-servers.mustache
16 files changed, 113 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/7
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [server] Add start time in server description
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/common/wire_protocol.proto@96
PS2, Line 96: optional string start_time = 5;
Do you feel strongly about storing this as a string instead of a "seconds since epoch" timestamp? As the protobuf is generally meant to be a machine-readable format, I think the "seconds since epoch" format will be easier to deal with, and we can always format it in UTC time or localtime when displaying it to a user.
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/12304/2/src/kudu/server/server_base.cc@683
PS2, Line 683: start_time_ = LocalTimeAsString();
using local time here is a bit confusing since servers eventaully may run in different time zones (eg if we deploy in a multi-AZ setup). Using an epoch time avoids that issue, and we can convert to localtime when we display it on the web page or CLI output
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 04 Feb 2019 19:22:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12304
to look at the new patch set (#3).
Change subject: [server] Add start_time in server description
......................................................................
[server] Add start_time in server description
Add `start_time` to describe the start time of a server in
localtime format, like `2019-01-01 00:00:00 UTC`. We can use
this information to acknowledge the last start time of a server,
both master and tserver, to check whether there is an unexpect
restarting, the server joined time, etc. We can get `start_time`
of servers from both web-ui and command line `kudu tserver/master
list`.
Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/ts_descriptor-test.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tserver/heartbeater.cc
M www/masters.mustache
M www/tablet-servers.mustache
13 files changed, 89 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/12304/3
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [server] Add start time in server description
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/12304 )
Change subject: [server] Add start_time in server description
......................................................................
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/common/wire_protocol.proto@96
PS4, Line 96:
> can you add a comment which indicates the unit and reference point? ie "sec
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@138
PS4, Line 138: desc->l
> nit: we prefer using C++ syntax for casts: static_cast<time_t>(...)
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@479
PS4, Line 479: }
> need to handle the case where 'reg.has_start_time()' is false (eg post upgr
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/master/master_path_handlers.cc@663
PS4, Line 663: }
> same
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc@152
PS4, Line 152: values.emplace_back(std::move(pb_util::ParseStartTime(master.registration())));
> I think you need to wrap this block in something like:
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_master.cc@154
PS4, Line 154: } else {
> nit: can std::move() this
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc@150
PS4, Line 150: for (const auto& server : servers) {
> same comment as in tool_action_master.cc
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/src/kudu/tools/tool_action_tserver.cc@153
PS4, Line 153: } else {
> nit: std::move
Done
http://gerrit.cloudera.org:8080/#/c/12304/4/www/tablet-servers.mustache
File www/tablet-servers.mustache:
http://gerrit.cloudera.org:8080/#/c/12304/4/www/tablet-servers.mustache@58
PS4, Line 58: <td>{{time_since_hb}}</td>
> maybe breaking out the start time to a separate column would be good?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I041467014499ad00c07398ad8f61c6d6ca1ceeca
Gerrit-Change-Number: 12304
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 15:58:08 +0000
Gerrit-HasComments: Yes