kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: [mini_{master,tablet_server}] micro clean-up
Date Thu, 22 Jun 2017 22:37:21 GMT
Repository: kudu
Updated Branches:
  refs/heads/master d2b820594 -> d62394293


[mini_{master,tablet_server}] micro clean-up

A micro clean-up on mini_master and mini_tablet_server:
  * remove unused FsManager member from MiniTabletServer
  * use std::unique_ptr instead of gscoped_ptr
  * get rid of started_ member
  * removed redundant CHECK() statements
  * other unsorted micro-updates

Change-Id: I9fc34b6f8ff94098551e567f4aefa4e92a907f53
Reviewed-on: http://gerrit.cloudera.org:8080/7265
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <adar@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d6239429
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d6239429
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d6239429

Branch: refs/heads/master
Commit: d623942935e22aa2f22815a9b37397d84ffba46f
Parents: d2b8205
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Thu Jun 22 11:16:13 2017 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Thu Jun 22 22:35:17 2017 +0000

----------------------------------------------------------------------
 .../master_cert_authority-itest.cc              |  4 +-
 src/kudu/integration-tests/mini_cluster.cc      |  2 +-
 src/kudu/master/mini_master.cc                  | 36 ++++++-----------
 src/kudu/master/mini_master.h                   | 15 +++----
 src/kudu/tserver/mini_tablet_server.cc          | 42 +++++++++-----------
 src/kudu/tserver/mini_tablet_server.h           | 18 ++++-----
 6 files changed, 46 insertions(+), 71 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/integration-tests/master_cert_authority-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master_cert_authority-itest.cc b/src/kudu/integration-tests/master_cert_authority-itest.cc
index 6d6016a..48331c0 100644
--- a/src/kudu/integration-tests/master_cert_authority-itest.cc
+++ b/src/kudu/integration-tests/master_cert_authority-itest.cc
@@ -115,7 +115,7 @@ class MasterCertAuthorityTest : public KuduTest {
       req.mutable_registration()->CopyFrom(fake_reg);
 
       MiniMaster* m = cluster_->mini_master(i);
-      if (!m->is_running()) {
+      if (!m->is_started()) {
         continue;
       }
       MasterServiceProxy proxy(messenger_, m->bound_rpc_addr());
@@ -145,7 +145,7 @@ class MasterCertAuthorityTest : public KuduTest {
       req.set_csr_der(csr_str);
 
       MiniMaster* m = cluster_->mini_master(i);
-      if (!m->is_running()) {
+      if (!m->is_started()) {
         continue;
       }
       MasterServiceProxy proxy(messenger_, m->bound_rpc_addr());

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/integration-tests/mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster.cc b/src/kudu/integration-tests/mini_cluster.cc
index da57fb0..57d8ea4 100644
--- a/src/kudu/integration-tests/mini_cluster.cc
+++ b/src/kudu/integration-tests/mini_cluster.cc
@@ -306,7 +306,7 @@ Status MiniCluster::GetLeaderMasterIndex(int* idx) const {
   while (MonoTime::Now() < deadline) {
     for (int i = 0; i < num_masters(); i++) {
       master::MiniMaster* mm = mini_master(i);
-      if (!mm->is_running() || mm->master()->IsShutdown()) {
+      if (!mm->is_started() || mm->master()->IsShutdown()) {
         continue;
       }
       master::CatalogManager* catalog = mm->master()->catalog_manager();

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/master/mini_master.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/mini_master.cc b/src/kudu/master/mini_master.cc
index 63188b5..0862869 100644
--- a/src/kudu/master/mini_master.cc
+++ b/src/kudu/master/mini_master.cc
@@ -17,19 +17,24 @@
 
 #include "kudu/master/mini_master.h"
 
+#include <memory>
 #include <string>
+#include <vector>
 
 #include <glog/logging.h>
 
 #include "kudu/fs/fs_manager.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/strings/substitute.h"
+#include "kudu/master/master.h"
 #include "kudu/server/rpc_server.h"
 #include "kudu/server/webserver.h"
-#include "kudu/master/master.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/status.h"
 
+using std::string;
+using std::unique_ptr;
+using std::vector;
 using strings::Substitute;
 
 DECLARE_bool(enable_minidumps);
@@ -39,8 +44,7 @@ namespace kudu {
 namespace master {
 
 MiniMaster::MiniMaster(Env* env, string fs_root, uint16_t rpc_port)
-    : running_(false),
-      env_(env),
+    : env_(env),
       fs_root_(std::move(fs_root)),
       rpc_port_(rpc_port) {
   // Disable minidump handler (we allow only one per process).
@@ -52,32 +56,27 @@ MiniMaster::~MiniMaster() {
 }
 
 Status MiniMaster::Start() {
-  CHECK(!running_);
   FLAGS_rpc_server_allow_ephemeral_ports = true;
   RETURN_NOT_OK(StartOnPorts(rpc_port_, 0));
   return master_->WaitForCatalogManagerInit();
 }
 
 Status MiniMaster::StartDistributedMaster(const vector<uint16_t>& peer_ports) {
-  CHECK(!running_);
   return StartDistributedMasterOnPorts(rpc_port_, 0, peer_ports);
 }
 
 Status MiniMaster::Restart() {
-  CHECK(!running_);
   RETURN_NOT_OK(StartOnPorts(bound_rpc_.port(), bound_http_.port()));
-  CHECK(running_);
   return WaitForCatalogManagerInit();
 }
 
 void MiniMaster::Shutdown() {
-  if (running_) {
+  if (master_) {
     bound_rpc_ = bound_rpc_addr();
     bound_http_ = bound_http_addr();
     master_->Shutdown();
+    master_.reset();
   }
-  running_ = false;
-  master_.reset();
 }
 
 Status MiniMaster::WaitForCatalogManagerInit() const {
@@ -85,17 +84,14 @@ Status MiniMaster::WaitForCatalogManagerInit() const {
 }
 
 const Sockaddr MiniMaster::bound_rpc_addr() const {
-  CHECK(running_);
   return master_->first_rpc_address();
 }
 
 const Sockaddr MiniMaster::bound_http_addr() const {
-  CHECK(running_);
   return master_->first_http_address();
 }
 
 std::string MiniMaster::permanent_uuid() const {
-  CHECK(master_);
   return DCHECK_NOTNULL(master_->fs_manager())->uuid();
 }
 
@@ -104,42 +100,34 @@ std::string MiniMaster::bound_rpc_addr_str() const {
 }
 
 Status MiniMaster::StartOnPorts(uint16_t rpc_port, uint16_t web_port) {
-  CHECK(!running_);
-  CHECK(!master_);
-
   MasterOptions opts;
   return StartOnPorts(rpc_port, web_port, &opts);
 }
 
 Status MiniMaster::StartOnPorts(uint16_t rpc_port, uint16_t web_port,
                                 MasterOptions* opts) {
+  CHECK(!master_);
   opts->rpc_opts.rpc_bind_addresses = Substitute("127.0.0.1:$0", rpc_port);
   opts->webserver_opts.port = web_port;
   opts->fs_opts.wal_path = fs_root_;
   opts->fs_opts.data_paths = { fs_root_ };
 
-  gscoped_ptr<Master> server(new Master(*opts));
+  unique_ptr<Master> server(new Master(*opts));
   RETURN_NOT_OK(server->Init());
   RETURN_NOT_OK(server->StartAsync());
-
   master_.swap(server);
-  running_ = true;
 
   return Status::OK();
 }
 
 Status MiniMaster::StartDistributedMasterOnPorts(uint16_t rpc_port, uint16_t web_port,
                                                  const vector<uint16_t>& peer_ports)
{
-  CHECK(!running_);
-  CHECK(!master_);
-
-  MasterOptions opts;
-
   vector<HostPort> peer_addresses;
   for (uint16_t peer_port : peer_ports) {
     HostPort peer_address("127.0.0.1", peer_port);
     peer_addresses.push_back(peer_address);
   }
+  MasterOptions opts;
   opts.master_addresses = peer_addresses;
 
   return StartOnPorts(rpc_port, web_port, &opts);

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/master/mini_master.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/mini_master.h b/src/kudu/master/mini_master.h
index fe9777e..b509b3b 100644
--- a/src/kudu/master/mini_master.h
+++ b/src/kudu/master/mini_master.h
@@ -14,9 +14,10 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_MASTER_MINI_MASTER_H
-#define KUDU_MASTER_MINI_MASTER_H
 
+#pragma once
+
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -60,7 +61,7 @@ class MiniMaster {
 
   Status WaitForCatalogManagerInit() const;
 
-  bool is_running() const { return running_; }
+  bool is_started() const { return master_ ? true : false; }
 
   const Sockaddr bound_rpc_addr() const;
   const Sockaddr bound_http_addr() const;
@@ -77,22 +78,18 @@ class MiniMaster {
   Status StartOnPorts(uint16_t rpc_port, uint16_t web_port);
 
   Status StartOnPorts(uint16_t rpc_port, uint16_t web_port,
-                      MasterOptions* options);
+                      MasterOptions* opts);
 
   Status StartDistributedMasterOnPorts(uint16_t rpc_port, uint16_t web_port,
                                        const std::vector<uint16_t>& peer_ports);
 
-  bool running_;
-
   ATTRIBUTE_MEMBER_UNUSED Env* const env_;
   const std::string fs_root_;
   const uint16_t rpc_port_;
   Sockaddr bound_rpc_;
   Sockaddr bound_http_;
-  gscoped_ptr<Master> master_;
+  std::unique_ptr<Master> master_;
 };
 
 } // namespace master
 } // namespace kudu
-
-#endif /* KUDU_MASTER_MINI_MASTER_H */

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/tserver/mini_tablet_server.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/mini_tablet_server.cc b/src/kudu/tserver/mini_tablet_server.cc
index 67a2734..9ee60b4 100644
--- a/src/kudu/tserver/mini_tablet_server.cc
+++ b/src/kudu/tserver/mini_tablet_server.cc
@@ -17,8 +17,11 @@
 
 #include "kudu/tserver/mini_tablet_server.h"
 
+#include <string>
 #include <utility>
 
+#include <gflags/gflags_declare.h>
+
 #include "kudu/consensus/metadata.pb.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tablet/tablet-test-util.h"
@@ -27,22 +30,21 @@
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/status.h"
 
-using std::pair;
+DECLARE_bool(enable_minidumps);
+DECLARE_bool(rpc_server_allow_ephemeral_ports);
 
-using kudu::consensus::RaftPeerPB;
 using kudu::consensus::RaftConfigPB;
+using kudu::consensus::RaftPeerPB;
+using std::pair;
+using std::string;
+using std::unique_ptr;
 using strings::Substitute;
 
-DECLARE_bool(enable_minidumps);
-DECLARE_bool(rpc_server_allow_ephemeral_ports);
-
 namespace kudu {
 namespace tserver {
 
 MiniTabletServer::MiniTabletServer(const string& fs_root,
-                                   uint16_t rpc_port)
-  : started_(false) {
-
+                                   uint16_t rpc_port) {
   // Disable minidump handler (we allow only one per process).
   FLAGS_enable_minidumps = false;
   // Start RPC server on loopback.
@@ -58,14 +60,13 @@ MiniTabletServer::~MiniTabletServer() {
 }
 
 Status MiniTabletServer::Start() {
-  CHECK(!started_);
+  CHECK(!server_);
 
-  gscoped_ptr<TabletServer> server(new TabletServer(opts_));
+  unique_ptr<TabletServer> server(new TabletServer(opts_));
   RETURN_NOT_OK(server->Init());
   RETURN_NOT_OK(server->Start());
-
   server_.swap(server);
-  started_ = true;
+
   return Status::OK();
 }
 
@@ -74,7 +75,7 @@ Status MiniTabletServer::WaitStarted() {
 }
 
 void MiniTabletServer::Shutdown() {
-  if (started_) {
+  if (server_) {
     // Save the bound ports back into the options structure so that, if we restart the
     // server, it will come back on the same address. This is necessary since we don't
     // currently support tablet servers re-registering on different ports (KUDU-418).
@@ -83,17 +84,14 @@ void MiniTabletServer::Shutdown() {
     server_->Shutdown();
     server_.reset();
   }
-  started_ = false;
 }
 
 Status MiniTabletServer::Restart() {
-  CHECK(!started_);
-  RETURN_NOT_OK(Start());
-  return Status::OK();
+  return Start();
 }
 
 RaftConfigPB MiniTabletServer::CreateLocalConfig() const {
-  CHECK(started_) << "Must Start()";
+  CHECK(server_) << "must call Start() first";
   RaftConfigPB config;
   RaftPeerPB* peer = config.add_peers();
   peer->set_permanent_uuid(server_->instance_pb().permanent_uuid());
@@ -113,13 +111,12 @@ Status MiniTabletServer::AddTestTablet(const std::string& table_id,
                                        const std::string& tablet_id,
                                        const Schema& schema,
                                        const RaftConfigPB& config) {
-  CHECK(started_) << "Must Start()";
   Schema schema_with_ids = SchemaBuilder(schema).Build();
   pair<PartitionSchema, Partition> partition = tablet::CreateDefaultPartition(schema_with_ids);
 
   return server_->tablet_manager()->CreateNewTablet(
-    table_id, tablet_id, partition.second, table_id,
-    schema_with_ids, partition.first, config, nullptr);
+      table_id, tablet_id, partition.second, table_id,
+      schema_with_ids, partition.first, config, nullptr);
 }
 
 void MiniTabletServer::FailHeartbeats() {
@@ -127,17 +124,14 @@ void MiniTabletServer::FailHeartbeats() {
 }
 
 const Sockaddr MiniTabletServer::bound_rpc_addr() const {
-  CHECK(started_);
   return server_->first_rpc_address();
 }
 
 const Sockaddr MiniTabletServer::bound_http_addr() const {
-  CHECK(started_);
   return server_->first_http_address();
 }
 
 const string& MiniTabletServer::uuid() const {
-  CHECK(started_);
   return server_->fs_manager()->uuid();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6239429/src/kudu/tserver/mini_tablet_server.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/mini_tablet_server.h b/src/kudu/tserver/mini_tablet_server.h
index 34ed945..ffa8b59 100644
--- a/src/kudu/tserver/mini_tablet_server.h
+++ b/src/kudu/tserver/mini_tablet_server.h
@@ -14,8 +14,11 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_TSERVER_MINI_TABLET_SERVER_H
-#define KUDU_TSERVER_MINI_TABLET_SERVER_H
+
+#pragma once
+
+#include <memory>
+#include <string>
 
 #include "kudu/common/schema.h"
 #include "kudu/gutil/macros.h"
@@ -23,8 +26,6 @@
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/status.h"
 
-#include <string>
-
 namespace kudu {
 
 class FsManager;
@@ -90,19 +91,14 @@ class MiniTabletServer {
   // Return TS uuid.
   const std::string& uuid() const;
 
-  bool is_started() const { return started_; }
+  bool is_started() const { return server_ ? true : false; }
 
   void FailHeartbeats();
 
  private:
-  bool started_;
-
   TabletServerOptions opts_;
-
-  gscoped_ptr<FsManager> fs_manager_;
-  gscoped_ptr<TabletServer> server_;
+  std::unique_ptr<TabletServer> server_;
 };
 
 } // namespace tserver
 } // namespace kudu
-#endif


Mime
View raw message