You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/10/09 00:24:49 UTC
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Hello Alexey Serbin, Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/14395
to review the following change.
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
KUDU-2965: cut down on task-related log spew in the catalog manager
- All "sending RPC" messages are relegated to VLOG. These aren't terribly
useful in troubleshooting (i.e. the tserver's log is the source of truth
for what RPCs were actually received), and it was already the case for
some tasks, so it made sense to do it across the board.
- Failure messages that don't end a task are throttled, because they're very
likely to repeat. I toyed with the idea of throttling across a particular
facet (e.g. per tserver) but the complexity outweighs the usefulness.
- Generic retry messages are relegated to VLOG. Sure it was nice to see the
exponential backoff get logged, but after years of that, I think we know
that the math works as expected.
- A few other cosmetic changes, including the removal of a function that had
no effect since the dawn of time.
Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 43 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/14395/1
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14395 )
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@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 04:44:56 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14395 )
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc@3252
PS1, Line 3252: 1
nit: maybe, 60 seconds is a better option?
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 1
Gerrit-Owner: 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:29:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14395 )
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14395/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14395/1//COMMIT_MSG@14
PS1, Line 14: throttling across a particular
: facet (e.g. per tserver) but the complexity outweighs the usefulness.
While I agree here, this kind of throttling would be super useful for per-replica throttling on tservers.
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc@3252
PS1, Line 3252: 1
> nit: maybe, 60 seconds is a better option?
Yeah, I've never found a good formula for how frequently we should update these. On the one hand, 1 second over a long period will still fill the logs. On the other, 60 seconds might be too coarse-grained to pinpoint something useful. I think I'd be fine with either; this already seems like an improvement over what there was before.
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 1
Gerrit-Owner: 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:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14395
to look at the new patch set (#2).
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
KUDU-2965: cut down on task-related log spew in the catalog manager
- All "sending RPC" messages are relegated to VLOG. These aren't terribly
useful in troubleshooting (i.e. the tserver's log is the source of truth
for what RPCs were actually received), and it was already the case for
some tasks, so it made sense to do it across the board.
- Failure messages that don't end a task are throttled because they're very
likely to repeat. I thought about throttling across a particular facet
(e.g. per tserver) but that's more work than what I want to do right now.
- Generic retry messages are relegated to VLOG. Sure it was nice to see the
exponential backoff get logged, but after years of that, I think we know
that the math works as expected.
- A few other cosmetic changes, including the removal of a function that had
no effect since the dawn of time.
Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 43 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/14395/2
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@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)
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14395 )
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
KUDU-2965: cut down on task-related log spew in the catalog manager
- All "sending RPC" messages are relegated to VLOG. These aren't terribly
useful in troubleshooting (i.e. the tserver's log is the source of truth
for what RPCs were actually received), and it was already the case for
some tasks, so it made sense to do it across the board.
- Failure messages that don't end a task are throttled because they're very
likely to repeat. I thought about throttling across a particular facet
(e.g. per tserver) but that's more work than what I want to do right now.
- Generic retry messages are relegated to VLOG. Sure it was nice to see the
exponential backoff get logged, but after years of that, I think we know
that the math works as expected.
- A few other cosmetic changes, including the removal of a function that had
no effect since the dawn of time.
Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Reviewed-on: http://gerrit.cloudera.org:8080/14395
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 43 insertions(+), 51 deletions(-)
Approvals:
Andrew Wong: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@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)
[kudu-CR] KUDU-2965: cut down on task-related log spew in the catalog manager
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14395 )
Change subject: KUDU-2965: cut down on task-related log spew in the catalog manager
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14395/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14395/1//COMMIT_MSG@14
PS1, Line 14: throttling across a particular
: facet (e.g. per tserver) but the complexity outweighs the usefulness.
> While I agree here, this kind of throttling would be super useful for per-r
Yeah maybe I should rephrase: it was complex enough that I didn't feel like implementing it right now. :)
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14395/1/src/kudu/master/catalog_manager.cc@3252
PS1, Line 3252: 1
> Yeah, I've never found a good formula for how frequently we should update t
Yeah I want to be conservative and start with the minimal amount of throttling (i.e. 1 second). We can always throttle further if the trickle of one log per second ends up being a torrent.
--
To view, visit http://gerrit.cloudera.org:8080/14395
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d852eefdcb7903f6cfa330a423744a0ca5b7e80
Gerrit-Change-Number: 14395
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@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 04:24:38 +0000
Gerrit-HasComments: Yes