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/01 03:38:22 UTC

[kudu-CR] KUDU-2279 (part 1). rolling log: respect logfile retention flag

Hello Will Berkeley, Mike Percy,

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

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

to review the following change.


Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................

KUDU-2279 (part 1). rolling_log: respect logfile retention flag

Previously we never deleted old metrics logs, making it somewhat
dangerous to enable this in production. Now we respect the same
retention gflag that we apply to our other glogs.

Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
---
M src/kudu/util/rolling_log-test.cc
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
3 files changed, 81 insertions(+), 38 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
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-2279 (part 1). rolling log: respect logfile retention flag

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

Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9175/1/src/kudu/util/rolling_log.h
File src/kudu/util/rolling_log.h:

http://gerrit.cloudera.org:8080/#/c/9175/1/src/kudu/util/rolling_log.h@68
PS1, Line 68:   void SetSizeLimitBytes(int64_t bytes);
> warning: function 'kudu::RollingLog::SetSizeLimitBytes' has a definition wi
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
Gerrit-PatchSet: 1
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, 01 Feb 2018 20:06:00 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2279 (part 1). rolling log: respect logfile retention flag

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

Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
Gerrit-PatchSet: 2
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, 02 Feb 2018 16:22:33 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2279 (part 1). rolling log: respect logfile retention flag

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

Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................


Patch Set 1: Code-Review+2

LGTM but might want to fix tidy bot's nit.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
Gerrit-PatchSet: 1
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: Thu, 01 Feb 2018 09:28:02 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2279 (part 1). rolling log: respect logfile retention flag

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

Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................

KUDU-2279 (part 1). rolling_log: respect logfile retention flag

Previously we never deleted old metrics logs, making it somewhat
dangerous to enable this in production. Now we respect the same
retention gflag that we apply to our other glogs.

Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Reviewed-on: http://gerrit.cloudera.org:8080/9175
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
M src/kudu/util/rolling_log-test.cc
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
3 files changed, 83 insertions(+), 39 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Will Berkeley: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
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>

[kudu-CR] KUDU-2279 (part 1). rolling log: respect logfile retention flag

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/9175

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

Change subject: KUDU-2279 (part 1). rolling_log: respect logfile retention flag
......................................................................

KUDU-2279 (part 1). rolling_log: respect logfile retention flag

Previously we never deleted old metrics logs, making it somewhat
dangerous to enable this in production. Now we respect the same
retention gflag that we apply to our other glogs.

Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
---
M src/kudu/util/rolling_log-test.cc
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
3 files changed, 83 insertions(+), 39 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b
Gerrit-Change-Number: 9175
Gerrit-PatchSet: 2
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>