You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/11/25 20:50:27 UTC

[kudu-CR] Remove/downgrade a few very verbose useless log statements

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................

Remove/downgrade a few very verbose useless log statements

When running tests that use a tablet server the log is
full of thread/reactor/maintenance manager statetments
that are useless. This is particularly annoying at VLOG(5)
level where we expect to see printouts of compaction
inputs/outputs. These get lost in a sea of useless statements.

This outright removes the "timer tick" reactor statement,
downgrades a few statements to VLOG(10) and makes the
maintenance_manager much quieter when it's disabled.

Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
---
M src/kudu/rpc/reactor.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/thread.cc
3 files changed, 8 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5226/2/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:

Line 208:     if (!FLAGS_enable_maintenance_manager) {
> why'd you move it from VLOG to INFO? doesn't this make it _more_ noisy? may
Done


http://gerrit.cloudera.org:8080/#/c/5226/2/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:

Line 546:   VLOG(2) << "Started thread " << t->tid()<< " - " << category << ":" << name;
> I actually find these info messages pretty useful at vlog=2, since we've ha
k will change this back.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: Yes

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 2:

> are these useful in your opinion? i know they can be reduced that
 > way but why keep typing that flag if they never provide value? for
 > instance if the maintenance manager if disabled, do we really need
 > to know about it every 100 msecs?

As I understand, when using the -vmodule flag, you can enable VLOG() logging only from specific places.  E.g., in you case it could be just set of files from which you wanted to collect VLOG() messages, all the rest of VLOG() instances would be silent.

E.g., some time ago I ran some tests like

  client-test --gtest_filter=.... -vmodule batcher=3

to get VLOG messages of level 0 to 3 from batcher.cc file.  All VLOG() messages from other places were silenced.

Would it be useful in your case?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: No

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................

Remove/downgrade a few very verbose useless log statements

When running tests that use a tablet server the log is
full of thread/reactor/maintenance manager statetments
that are useless. This is particularly annoying at VLOG(5)
level where we expect to see printouts of compaction
inputs/outputs. These get lost in a sea of useless statements.

This outright removes the "timer tick" reactor statement,
downgrades a few statements to VLOG(10) and makes the
maintenance_manager much quieter when it's disabled.

Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
---
M src/kudu/rpc/reactor.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/thread.cc
3 files changed, 9 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Remove/downgrade a few very verbose useless log statements

When running tests that use a tablet server the log is
full of thread/reactor/maintenance manager statetments
that are useless. This is particularly annoying at VLOG(5)
level where we expect to see printouts of compaction
inputs/outputs. These get lost in a sea of useless statements.

This outright removes the "timer tick" reactor statement,
downgrades another reactor statement to VLOG(10) and makes the
maintenance_manager much quieter when it's disabled.

Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Reviewed-on: http://gerrit.cloudera.org:8080/5226
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/reactor.cc
M src/kudu/util/maintenance_manager.cc
2 files changed, 8 insertions(+), 7 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 2:

are these useful in your opinion? i know they can be reduced that way but why keep typing that flag if they never provide value? for instance if the maintenance manager if disabled, do we really need to know about it every 100 msecs?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: No

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................

Remove/downgrade a few very verbose useless log statements

When running tests that use a tablet server the log is
full of thread/reactor/maintenance manager statetments
that are useless. This is particularly annoying at VLOG(5)
level where we expect to see printouts of compaction
inputs/outputs. These get lost in a sea of useless statements.

This outright removes the "timer tick" reactor statement,
downgrades another reactor statement to VLOG(10) and makes the
maintenance_manager much quieter when it's disabled.

Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
---
M src/kudu/rpc/reactor.cc
M src/kudu/util/maintenance_manager.cc
2 files changed, 8 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 2:

> Uploaded patch set 2.

Instead of that, why not to use '-vmodule' flag?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: No

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: No

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................

Remove/downgrade a few very verbose useless log statements

When running tests that use a tablet server the log is
full of thread/reactor/maintenance manager statetments
that are useless. This is particularly annoying at VLOG(5)
level where we expect to see printouts of compaction
inputs/outputs. These get lost in a sea of useless statements.

This outright removes the "timer tick" reactor statement,
downgrades another reactor statement to VLOG(10) and makes the
maintenance_manager much quieter when it's disabled.

Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
---
M src/kudu/rpc/reactor.cc
M src/kudu/util/maintenance_manager.cc
2 files changed, 7 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/5226/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5226
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Remove/downgrade a few very verbose useless log statements

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Remove/downgrade a few very verbose useless log statements
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5226/2/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:

Line 208:       LOG_EVERY_N(INFO, 10) << "Maintenance manager is disabled. Doing nothing";
why'd you move it from VLOG to INFO? doesn't this make it _more_ noisy? maybe use KLOG_EVERY_N_SECS for every 30 seconds or something instead, if you're going to boost to INFO level?

Also, should use the KLOG_* macro rather than LOG_* to avoid TSAN issues


http://gerrit.cloudera.org:8080/#/c/5226/2/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:

Line 546:   VLOG(10) << "Started thread " << t->tid()<< " - " << category << ":" << name;
I actually find these info messages pretty useful at vlog=2, since we've had a fair number of issues with things like threads starting/stopping more frequently than necessary, taking a long time to start, etc.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7863f067f49cc7432e8ca3b532a208d3c4005300
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@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-HasComments: Yes