[kudu-CR] master: include TS address in log messages
Adar Dembo has submitted this change and it was merged. Change subject: master: include TS address in log messages .. master: include TS address in log messages When looking at master logs, it's quite annoying to have to translate back from UUIDs to actual hostnames, since the operator typically wants to ssh into that node to look at logs, etc. This patch adds TSDescriptor::ToString() and calls it from all the points in CatalogManager where log messages refer to an individual server. This also adds validation that TS registrations must include at least one HTTP and one RPC address. This has always been the case but wasn't verified. Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Reviewed-on: http://gerrit.cloudera.org:8080/4131 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/master/catalog_manager.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc 4 files changed, 66 insertions(+), 50 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] master: include TS address in log messages
Adar Dembo has posted comments on this change. Change subject: master: include TS address in log messages .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] master: include TS address in log messages
Todd Lipcon has posted comments on this change.
Change subject: master: include TS address in log messages
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4131/1//COMMIT_MSG
Commit Message:
Line 15: server.
> Thanks, yeah this is super useful while browsing logs with sclable nodes/ta
the error paths in that function are all for the case where we can't find or
register a TSDescriptor, so can't use the new function. We already use
RpcContext::requestor_string which includes the hostname, though
http://gerrit.cloudera.org:8080/#/c/4131/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3492: return Substitute("Master P $0: ",
> Hmm, I'm not so sure about this one. The Raft machinery will log as "T
yea good point, will revert this part.
--
To view, visit http://gerrit.cloudera.org:8080/4131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] master: include TS address in log messages
Hello Dinesh Bhat, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4131 to look at the new patch set (#2). Change subject: master: include TS address in log messages .. master: include TS address in log messages When looking at master logs, it's quite annoying to have to translate back from UUIDs to actual hostnames, since the operator typically wants to ssh into that node to look at logs, etc. This patch adds TSDescriptor::ToString() and calls it from all the points in CatalogManager where log messages refer to an individual server. This also adds validation that TS registrations must include at least one HTTP and one RPC address. This has always been the case but wasn't verified. Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc 4 files changed, 66 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/4131/2 -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] master: include TS address in log messages
Kudu Jenkins has posted comments on this change. Change subject: master: include TS address in log messages .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3093/ -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] master: include TS address in log messages
Adar Dembo has posted comments on this change.
Change subject: master: include TS address in log messages
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4131/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3492: return Substitute("Master P $0: ",
Hmm, I'm not so sure about this one. The Raft machinery will log as "T 0...
P ", so this is at least being consistent with that.
Can you scan the output of a multi-master integration test and see if this is
helpful?
--
To view, visit http://gerrit.cloudera.org:8080/4131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] master: include TS address in log messages
Dinesh Bhat has posted comments on this change. Change subject: master: include TS address in log messages .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4131/1//COMMIT_MSG Commit Message: Line 15: server. Thanks, yeah this is super useful while browsing logs with sclable nodes/tablets. Could you also see if there are places like MasterServiceImpl::TSHeartbeat() where it makes sense to add this ToString() from error paths. -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] master: include TS address in log messages
Hello Dan Burkert, Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4131 to review the following change. Change subject: master: include TS address in log messages .. master: include TS address in log messages When looking at master logs, it's quite annoying to have to translate back from UUIDs to actual hostnames, since the operator typically wants to ssh into that node to look at logs, etc. This patch adds TSDescriptor::ToString() and calls it from all the points in CatalogManager where log messages refer to an individual server. This also adds validation that TS registrations must include at least one HTTP and one RPC address. This has always been the case but wasn't verified. Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc 4 files changed, 45 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/4131/1 -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert
[kudu-CR] master: include TS address in log messages
Kudu Jenkins has posted comments on this change. Change subject: master: include TS address in log messages .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3086/ -- To view, visit http://gerrit.cloudera.org:8080/4131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic55fa7e818a115de70f9fc6aca12581c3b4779c7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
