You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2018/01/24 08:24:09 UTC
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9117
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
This change adds a new flag FLAGS_rpc_too_long_duration_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.
Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
---
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpcz_store.cc
2 files changed, 41 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/9117/1
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9117 )
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.
Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Reviewed-on: http://gerrit.cloudera.org:8080/9117
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpcz_store.cc
2 files changed, 9 insertions(+), 1 deletion(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9117 )
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9117/3/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/9117/3/src/kudu/rpc/rpc-test.cc@74
PS3, Line 74: DECLARE_int32(rpc_duration_too_long_ms);
Sorry, forgot to remove this line here. I can push another patch.
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:21:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9117
to look at the new patch set (#2).
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.
Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
---
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpcz_store.cc
2 files changed, 9 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/9117/2
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9117 )
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9117/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/9117/1/src/kudu/rpc/rpc-test.cc@592
PS1, Line 592: TEST_P(TestRpc, TestRpcLogging) {
hrm, I appreciate the effort of adding a test, but since the test doesn't actually assert anything, and it's just a pretty trivial substitution of a constant to a flag, I don't think it's necessary
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 18:57:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9117 )
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:06:59 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9117 )
Change subject: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9117/1/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/9117/1/src/kudu/rpc/rpc-test.cc@592
PS1, Line 592: TEST_P(TestRpc, TestRpcSidecar) {
> hrm, I appreciate the effort of adding a test, but since the test doesn't a
Removed. Yes, there doesn't seem to be a way to confirm whether something is logged (or not) from the test. I manually inspected the output of the test to confirm the flag is effective.
--
To view, visit http://gerrit.cloudera.org:8080/9117
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9117
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:06:05 +0000
Gerrit-HasComments: Yes