[kudu-CR] kserver: introduce KuduServer class
Todd Lipcon has submitted this change and it was merged. Change subject: kserver: introduce KuduServer class .. kserver: introduce KuduServer class The kserver module (with the KuduServer class) is an abstraction that sits between ServerBase and Master/TabletServer. The idea is for it to encapsulate all Kudu-specific knowledge that should be shared by Masters and Tablet Servers without polluting ServerBase, which has no Kudu-specific knowledge and could be reused by any C++ application. This patch introduces the new class (just lifecycle methods for now) and wires it into Master/TabletServer. There's no new functionality. Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Reviewed-on: http://gerrit.cloudera.org:8080/7239 Reviewed-by: Todd LipconTested-by: Kudu Jenkins --- M CMakeLists.txt A src/kudu/kserver/CMakeLists.txt A src/kudu/kserver/kserver.cc A src/kudu/kserver/kserver.h M src/kudu/master/CMakeLists.txt M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/server_base.h M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 11 files changed, 192 insertions(+), 43 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] kserver: introduce KuduServer class
Todd Lipcon has posted comments on this change. Change subject: kserver: introduce KuduServer class .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] kserver: introduce KuduServer class
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7239 to look at the new patch set (#2). Change subject: kserver: introduce KuduServer class .. kserver: introduce KuduServer class The kserver module (with the KuduServer class) is an abstraction that sits between ServerBase and Master/TabletServer. The idea is for it to encapsulate all Kudu-specific knowledge that should be shared by Masters and Tablet Servers without polluting ServerBase, which has no Kudu-specific knowledge and could be reused by any C++ application. This patch introduces the new class (just lifecycle methods for now) and wires it into Master/TabletServer. There's no new functionality. Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 --- M CMakeLists.txt A src/kudu/kserver/CMakeLists.txt A src/kudu/kserver/kserver.cc A src/kudu/kserver/kserver.h M src/kudu/master/CMakeLists.txt M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/server_base.h M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 11 files changed, 192 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/7239/2 -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] kserver: introduce KuduServer class
Todd Lipcon has posted comments on this change. Change subject: kserver: introduce KuduServer class .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7239/1/src/kudu/kserver/kserver.h File src/kudu/kserver/kserver.h: Line 49: Status Init(); > Yeah. I don't think it's a huge deal (since the instances are always of typ yea, given it's not a perf hotspot or anything, though, let's make it virtual so it's less error prone if we ever end up with some utility code that tries to call the superclass -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] kserver: introduce KuduServer class
Adar Dembo has posted comments on this change. Change subject: kserver: introduce KuduServer class .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7239/1/src/kudu/kserver/kserver.h File src/kudu/kserver/kserver.h: Line 49: Status Init(); > should these methods be virtual and override? Yeah. I don't think it's a huge deal (since the instances are always of type Master/TabletServer and we explicitly call ServerBase::Foo and KuduServer:Foo), but it's certainly confusing. -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] kserver: introduce KuduServer class
Todd Lipcon has posted comments on this change. Change subject: kserver: introduce KuduServer class .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7239/1/src/kudu/kserver/kserver.h File src/kudu/kserver/kserver.h: Line 49: Status Init(); should these methods be virtual and override? -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] kserver: introduce KuduServer class
Adar Dembo has posted comments on this change. Change subject: kserver: introduce KuduServer class .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7239/1/src/kudu/master/master.h File src/kudu/master/master.h: Line 29: #include "kudu/master/master_options.h" > warning: #includes are not sorted properly [llvm-include-order] Done http://gerrit.cloudera.org:8080/#/c/7239/1/src/kudu/tserver/tablet_server.h File src/kudu/tserver/tablet_server.h: Line 47: // TODO: move this out of this header, since clients want to use this > warning: missing username/bug in TODO [google-readability-todo] Done -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] kserver: introduce KuduServer class
Hello David Ribeiro Alves, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7239 to review the following change. Change subject: kserver: introduce KuduServer class .. kserver: introduce KuduServer class The kserver module (with the KuduServer class) is an abstraction that sits between ServerBase and Master/TabletServer. The idea is for it to encapsulate all Kudu-specific knowledge that should be shared by Masters and Tablet Servers without polluting ServerBase, which has no Kudu-specific knowledge and could be reused by any C++ application. This patch introduces the new class (just lifecycle methods for now) and wires it into Master/TabletServer. There's no new functionality. Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 --- M CMakeLists.txt A src/kudu/kserver/CMakeLists.txt A src/kudu/kserver/kserver.cc A src/kudu/kserver/kserver.h M src/kudu/master/CMakeLists.txt M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 10 files changed, 179 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/7239/1 -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Todd Lipcon