You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/02/15 00:24:31 UTC

[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 <to...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 21:27:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 02:07:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 00:06:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 21:29:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 19:05:15 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:27:06 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Reviewed-by: Mike Percy <mp...@apache.org>
---
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 00:10:43 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:44:07 +0000
Gerrit-HasComments: Yes