You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2019/10/04 02:05:11 UTC

[kudu-CR] KUDU 2069 p9: add timestamps to tserver states

Hello Alexey Serbin,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/14370

to review the following change.


Change subject: KUDU_2069 p9: add timestamps to tserver states
......................................................................

KUDU_2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted.

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
---
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
4 files changed, 26 insertions(+), 19 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14370/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................

KUDU-2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted. This patch plumbs this into
the Master's web UI's "Tablet Servers" page.

Here is an example of the Master's web UI with tservers of various
statuses:
https://imgur.com/kny5vHu

When the states are removed, the new section in the web UI does not show
up:
https://imgur.com/itWD0DB

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Reviewed-on: http://gerrit.cloudera.org:8080/14370
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/gutil/walltime.cc
M src/kudu/gutil/walltime.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
M www/tablet-servers.mustache
8 files changed, 132 insertions(+), 33 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14370

to look at the new patch set (#4).

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................

KUDU-2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted. This patch plumbs this into
the Master's web UI's "Tablet Servers" page.

Here is a an example of the Master's web UI with tservers of various
statuses:
https://imgur.com/kny5vHu

When the state are removed, the section in the web UI does not show up:
https://imgur.com/itWD0DB

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
---
M src/kudu/gutil/walltime.cc
M src/kudu/gutil/walltime.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
M www/tablet-servers.mustache
8 files changed, 131 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14370/4
-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG@12
PS2, Line 12: 
> I opted for web UI first because we already show the "started timestamp" fo
SGTM


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h
File src/kudu/gutil/walltime.h:

http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h@48
PS4, Line 48: string suitable for user display
nit: human-readable string for the current timezone


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h@48
PS4, Line 48: given timestamp
nit: please indicate that this is Epoch time.



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 09 Oct 2019 02:34:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-2069 p9: add timestamps to tserver states

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: WIP KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG@12
PS2, Line 12: display on the web UI
What about CLI tools?  Do we need to see the timestamp there as well?  Maybe, seeing that in the list of tablet servers along with the maintenance mode would be too much, but 'kudu tserver state get' might be a good spot to add that?



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Oct 2019 23:13:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14370

to look at the new patch set (#5).

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................

KUDU-2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted. This patch plumbs this into
the Master's web UI's "Tablet Servers" page.

Here is a an example of the Master's web UI with tservers of various
statuses:
https://imgur.com/kny5vHu

When the states are removed, the new section in the web UI does not show
up:
https://imgur.com/itWD0DB

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
---
M src/kudu/gutil/walltime.cc
M src/kudu/gutil/walltime.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
M www/tablet-servers.mustache
8 files changed, 132 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14370/5
-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/14370/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14370/4//COMMIT_MSG@17
PS4, Line 17: state
> Nit: states
Done


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h
File src/kudu/gutil/walltime.h:

http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h@48
PS4, Line 48: given timestamp
> nit: please indicate that this is Epoch time.
Done


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/gutil/walltime.h@48
PS4, Line 48: string suitable for user display
> nit: human-readable string for the current timezone
Done


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/master/master_path_handlers.cc@142
PS4, Line 142: uuid_and_state_with_timestmap
> uuid_and_state_with_timestamp
Done


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/master/master_path_handlers.cc@179
PS4, Line 179:       ts_json["target"] = std::move(webserver);
> warning: passing result of std::move() as a const reference argument; no mo
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 09 Oct 2019 04:40:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, Hannah Nguyen, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14370

to look at the new patch set (#6).

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................

KUDU-2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted. This patch plumbs this into
the Master's web UI's "Tablet Servers" page.

Here is an example of the Master's web UI with tservers of various
statuses:
https://imgur.com/kny5vHu

When the states are removed, the new section in the web UI does not show
up:
https://imgur.com/itWD0DB

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
---
M src/kudu/gutil/walltime.cc
M src/kudu/gutil/walltime.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
M www/tablet-servers.mustache
8 files changed, 132 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14370/6
-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14370/2//COMMIT_MSG@12
PS2, Line 12: 
> What about CLI tools?  Do we need to see the timestamp there as well?  Mayb
I opted for web UI first because we already show the "started timestamp" for each tserver, so having this here of all places seems like a good first step.

I'm going to hold off on adding tools until folks start asking for it. If anything, it's a good newbie task (though I'll hold off on filing that too, until someone asks for it).


http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/master.proto@232
PS2, Line 232:   // Timestamp, in seconds since the epoch, at which this state was set.
> What are the units?
Done


http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:

http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/ts_manager.cc@263
PS2, Line 263:     // may have been ignored while in maintenance mode are reprocessed. To do
> I think we prefer GetCurrentTimeMicros here.
We chatted about this on Slack -- to keep consistent with the current usage of timestamps in the master, we're keeping this.



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 09 Oct 2019 01:48:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-2069 p9: add timestamps to tserver states

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14370

to look at the new patch set (#2).

Change subject: WIP KUDU-2069 p9: add timestamps to tserver states
......................................................................

WIP KUDU-2069 p9: add timestamps to tserver states

This patch include timestamps to each tserver states so an operator can
know when a given tserver state was enacted.

TODO: display on the web UI

Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
---
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/ts_manager.cc
M src/kudu/master/ts_manager.h
4 files changed, 26 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14370/2
-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 09 Oct 2019 04:57:17 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2069 p9: add timestamps to tserver states

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14370/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14370/4//COMMIT_MSG@17
PS4, Line 17: state
Nit: states


http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/14370/4/src/kudu/master/master_path_handlers.cc@142
PS4, Line 142: uuid_and_state_with_timestmap
uuid_and_state_with_timestamp



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 09 Oct 2019 04:31:34 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-2069 p9: add timestamps to tserver states

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14370 )

Change subject: WIP KUDU-2069 p9: add timestamps to tserver states
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/master.proto@232
PS2, Line 232:   // Timestamp at which this state was set.
What are the units?


http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:

http://gerrit.cloudera.org:8080/#/c/14370/2/src/kudu/master/ts_manager.cc@263
PS2, Line 263:   int64_t timestamp = time(nullptr);
I think we prefer GetCurrentTimeMicros here.



-- 
To view, visit http://gerrit.cloudera.org:8080/14370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b437ad38ad6abcaae894a782ab6d52857c97741
Gerrit-Change-Number: 14370
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 05 Oct 2019 01:33:31 +0000
Gerrit-HasComments: Yes