You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2021/08/20 17:30:23 UTC

[kudu] branch master updated: [tablet] small clean-up

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 888f42a  [tablet] small clean-up
888f42a is described below

commit 888f42a7760377c21237643a0c2396870663f79c
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Aug 19 23:43:30 2021 -0700

    [tablet] small clean-up
    
    While making changes in TabletMetadata's code related to KUDU-2671,
    I found it's possible to improve the code around:
      * LogPrefix() now returns const reference instead of copy of a string
      * TabletMetadata's destructor can be trivial
      * TabletBootstrap::SetStatusMessage() uses move semantics
    
    This changelist does not contain any significant functional changes.
    
    Change-Id: I8ebe39d483605840b22e6ec2d9eb66b1f4fe9b07
    Reviewed-on: http://gerrit.cloudera.org:8080/17797
    Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/tablet/tablet_bootstrap.cc         | 30 +++++++++++--------
 src/kudu/tablet/tablet_metadata.cc          | 38 ++++++++++--------------
 src/kudu/tablet/tablet_metadata.h           | 46 ++++++++++++++++-------------
 src/kudu/tablet/tablet_replica.cc           | 10 ++-----
 src/kudu/tablet/tablet_replica.h            | 15 +++++-----
 src/kudu/tserver/tablet_copy-test-base.h    |  4 +--
 src/kudu/tserver/tablet_copy_client-test.cc | 44 ++++++++++++++-------------
 7 files changed, 94 insertions(+), 93 deletions(-)

diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc
index fb2a4fc..4d52e80 100644
--- a/src/kudu/tablet/tablet_bootstrap.cc
+++ b/src/kudu/tablet/tablet_bootstrap.cc
@@ -38,6 +38,7 @@
 #include "kudu/clock/hybrid_clock.h"
 #include "kudu/common/common.pb.h"
 #include "kudu/common/row_operations.h"
+#include "kudu/common/row_operations.pb.h"
 #include "kudu/common/schema.h"
 #include "kudu/common/timestamp.h"
 #include "kudu/common/wire_protocol.h"
@@ -141,6 +142,7 @@ using std::shared_ptr;
 using std::string;
 using std::unique_ptr;
 using std::unordered_map;
+using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -393,12 +395,15 @@ class TabletBootstrap {
   Status UpdateClock(uint64_t timestamp);
 
   // Return a log prefix string in the standard "T xxx P yyy" format.
-  string LogPrefix() const;
+  const string& LogPrefix() const {
+    return log_prefix_;
+  }
 
   // Log a status message and set the TabletReplica's status as well.
-  void SetStatusMessage(const string& status);
+  void SetStatusMessage(string status);
 
   const scoped_refptr<TabletMetadata> tablet_meta_;
+  const string log_prefix_;
   const RaftConfigPB committed_raft_config_;
   Clock* clock_;
   shared_ptr<MemTracker> mem_tracker_;
@@ -409,7 +414,7 @@ class TabletBootstrap {
   unique_ptr<tablet::Tablet> tablet_;
   const scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_;
   scoped_refptr<log::Log> log_;
-  std::shared_ptr<log::LogReader> log_reader_;
+  shared_ptr<log::LogReader> log_reader_;
 
   // Statistics on the replay of entries in the log.
   struct Stats {
@@ -463,18 +468,20 @@ class TabletBootstrap {
 
   // Transactions that were persisted as being in-flight (neither finalized or
   // aborted) and completed prior to restart.
-  std::unordered_set<int64_t> in_flight_txn_ids_;
-  std::unordered_set<int64_t> terminal_txn_ids_;
+  unordered_set<int64_t> in_flight_txn_ids_;
+  unordered_set<int64_t> terminal_txn_ids_;
 
   // Transactions (committed or not) that have active MemRowSets.
-  std::unordered_set<int64_t> mrs_txn_ids_;
+  unordered_set<int64_t> mrs_txn_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(TabletBootstrap);
 };
 
-void TabletBootstrap::SetStatusMessage(const string& status) {
+void TabletBootstrap::SetStatusMessage(string status) {
   LOG_WITH_PREFIX(INFO) << status;
-  if (tablet_replica_) tablet_replica_->SetStatusMessage(status);
+  if (tablet_replica_) {
+    tablet_replica_->SetStatusMessage(std::move(status));
+  }
 }
 
 Status BootstrapTablet(scoped_refptr<TabletMetadata> tablet_meta,
@@ -538,6 +545,9 @@ TabletBootstrap::TabletBootstrap(
     scoped_refptr<TabletReplica> tablet_replica,
     scoped_refptr<LogAnchorRegistry> log_anchor_registry)
     : tablet_meta_(std::move(tablet_meta)),
+      log_prefix_(Substitute("T $0 P $1: ",
+                             tablet_meta_->tablet_id(),
+                             tablet_meta_->fs_manager()->uuid())),
       committed_raft_config_(std::move(committed_raft_config)),
       clock_(clock),
       mem_tracker_(std::move(mem_tracker)),
@@ -1786,10 +1796,6 @@ Status TabletBootstrap::UpdateClock(uint64_t timestamp) {
   return clock_->Update(Timestamp(timestamp));
 }
 
-string TabletBootstrap::LogPrefix() const {
-  return Substitute("T $0 P $1: ", tablet_meta_->tablet_id(), tablet_meta_->fs_manager()->uuid());
-}
-
 Status FlushedStoresSnapshot::InitFrom(const TabletMetadata& tablet_meta) {
   CHECK(flushed_dms_by_drs_id_.empty()) << "already initted";
   last_durable_mrs_id_ = tablet_meta.last_durable_mrs_id();
diff --git a/src/kudu/tablet/tablet_metadata.cc b/src/kudu/tablet/tablet_metadata.cc
index 809a5c9..4e4e244 100644
--- a/src/kudu/tablet/tablet_metadata.cc
+++ b/src/kudu/tablet/tablet_metadata.cc
@@ -45,7 +45,6 @@
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
-#include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tablet/rowset_metadata.h"
 #include "kudu/tablet/txn_metadata.h"
@@ -69,25 +68,23 @@ TAG_FLAG(enable_tablet_orphaned_block_deletion, runtime);
 using base::subtle::Barrier_AtomicIncrement;
 using kudu::consensus::MinimumOpId;
 using kudu::consensus::OpId;
-using kudu::fs::BlockManager;
 using kudu::fs::BlockDeletionTransaction;
+using kudu::fs::BlockManager;
 using kudu::log::MinLogIndexAnchorer;
 using kudu::pb_util::SecureDebugString;
 using kudu::pb_util::SecureShortDebugString;
 using std::memory_order_relaxed;
 using std::shared_ptr;
 using std::string;
+using std::unique_ptr;
 using std::unordered_map;
 using std::unordered_set;
-using std::unique_ptr;
 using std::vector;
 using strings::Substitute;
 
 namespace kudu {
 namespace tablet {
 
-const int64_t kNoDurableMemStore = -1;
-
 // ============================================================================
 //  Tablet Metadata
 // ============================================================================
@@ -305,9 +302,11 @@ TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id,
       table_id_(std::move(table_id)),
       partition_(std::move(partition)),
       fs_manager_(fs_manager),
+      log_prefix_(Substitute("T $0 P $1: ", tablet_id_, fs_manager_->uuid())),
       next_rowset_idx_(0),
       last_durable_mrs_id_(kNoDurableMemStore),
       schema_(new Schema(schema)),
+      schema_ptr_(schema_.get()),
       schema_version_(0),
       table_name_(std::move(table_name)),
       partition_schema_(std::move(partition_schema)),
@@ -326,8 +325,6 @@ TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id,
 }
 
 TabletMetadata::~TabletMetadata() {
-  STLDeleteElements(&old_schemas_);
-  delete schema_;
 }
 
 TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id)
@@ -335,7 +332,7 @@ TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id)
       tablet_id_(std::move(tablet_id)),
       fs_manager_(fs_manager),
       next_rowset_idx_(0),
-      schema_(nullptr),
+      schema_ptr_(nullptr),
       num_flush_pins_(0),
       needs_flush_(false),
       flush_count_for_tests_(0),
@@ -414,7 +411,7 @@ Status TabletMetadata::LoadFromSuperBlock(const TabletSuperBlockPB& superblock)
       CHECK_EQ(table_id_, superblock.table_id());
       PartitionSchema partition_schema;
       RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(),
-                                            *schema_, &partition_schema));
+                                            *schema_ptr_, &partition_schema));
       CHECK(partition_schema_ == partition_schema);
 
       Partition partition;
@@ -738,7 +735,7 @@ Status TabletMetadata::ToSuperBlockUnlocked(TabletSuperBlockPB* super_block,
   partition_.ToPB(pb.mutable_partition());
   pb.set_last_durable_mrs_id(last_durable_mrs_id_);
   pb.set_schema_version(schema_version_);
-  RETURN_NOT_OK(partition_schema_.ToPB(*schema_, pb.mutable_partition_schema()));
+  RETURN_NOT_OK(partition_schema_.ToPB(*schema_ptr_, pb.mutable_partition_schema()));
   pb.set_table_name(table_name_);
 
   for (const shared_ptr<RowSetMetadata>& meta : rowsets) {
@@ -751,8 +748,8 @@ Status TabletMetadata::ToSuperBlockUnlocked(TabletSuperBlockPB* super_block,
     InsertOrDie(pb.mutable_txn_metadata(), txn_id_and_metadata.first, meta_pb);
   }
 
-  DCHECK(schema_->has_column_ids());
-  RETURN_NOT_OK_PREPEND(SchemaToPB(*schema_, pb.mutable_schema()),
+  DCHECK(schema_ptr_->has_column_ids());
+  RETURN_NOT_OK_PREPEND(SchemaToPB(*schema_ptr_, pb.mutable_schema()),
                         "Couldn't serialize schema into superblock");
 
   pb.set_tablet_data_state(tablet_data_state_);
@@ -938,17 +935,16 @@ void TabletMetadata::SetSchema(const Schema& schema, uint32_t version) {
   SetSchemaUnlocked(std::move(new_schema), version);
 }
 
-void TabletMetadata::SetSchemaUnlocked(unique_ptr<Schema> new_schema, uint32_t version) {
+void TabletMetadata::SetSchemaUnlocked(
+    unique_ptr<Schema> new_schema, uint32_t version) {
   DCHECK(new_schema->has_column_ids());
 
-  Schema* old_schema = schema_;
   // "Release" barrier ensures that, when we publish the new Schema object,
   // all of its initialization is also visible.
-  base::subtle::Release_Store(reinterpret_cast<AtomicWord*>(&schema_),
-                              reinterpret_cast<AtomicWord>(new_schema.release()));
-  if (PREDICT_TRUE(old_schema)) {
-    old_schemas_.push_back(old_schema);
-  }
+  base::subtle::Release_Store(reinterpret_cast<AtomicWord*>(&schema_ptr_),
+                              reinterpret_cast<AtomicWord>(new_schema.get()));
+  std::swap(schema_, new_schema);
+  old_schemas_.emplace_back(std::move(new_schema));
   schema_version_ = version;
 }
 
@@ -977,10 +973,6 @@ void TabletMetadata::set_tablet_data_state(TabletDataState state) {
   tablet_data_state_ = state;
 }
 
-string TabletMetadata::LogPrefix() const {
-  return Substitute("T $0 P $1: ", tablet_id_, fs_manager_->uuid());
-}
-
 TabletDataState TabletMetadata::tablet_data_state() const {
   std::lock_guard<LockType> l(data_lock_);
   return tablet_data_state_;
diff --git a/src/kudu/tablet/tablet_metadata.h b/src/kudu/tablet/tablet_metadata.h
index 55053b9..f545dfa 100644
--- a/src/kudu/tablet/tablet_metadata.h
+++ b/src/kudu/tablet/tablet_metadata.h
@@ -60,7 +60,7 @@ class RowSetMetadata;
 class TxnMetadata;
 enum TxnState : int8_t;
 
-typedef std::vector<std::shared_ptr<RowSetMetadata> > RowSetMetadataVector;
+typedef std::vector<std::shared_ptr<RowSetMetadata>> RowSetMetadataVector;
 typedef std::unordered_set<int64_t> RowSetMetadataIds;
 
 // A transaction ID whose MRS is being flushed.
@@ -69,7 +69,7 @@ typedef std::unordered_set<int64_t> RowSetMetadataIds;
 // 'last_durable_mrs_id' per transaction.
 typedef int64_t TxnInfoBeingFlushed;
 
-extern const int64_t kNoDurableMemStore;
+constexpr int64_t kNoDurableMemStore = -1;
 
 // Manages the "blocks tracking" for the specified tablet.
 //
@@ -161,7 +161,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   // even if the schema is changed.
   const Schema& schema() const {
     const Schema* s = reinterpret_cast<const Schema*>(
-        base::subtle::Acquire_Load(reinterpret_cast<const AtomicWord*>(&schema_)));
+        base::subtle::Acquire_Load(reinterpret_cast<const AtomicWord*>(&schema_ptr_)));
     return *s;
   }
 
@@ -290,7 +290,8 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
 
   const RowSetMetadataVector& rowsets() const { return rowsets_; }
 
-  FsManager *fs_manager() const { return fs_manager_; }
+  const FsManager* fs_manager() const { return fs_manager_; }
+  FsManager* fs_manager() { return fs_manager_; }
 
   int64_t last_durable_mrs_id() const { return last_durable_mrs_id_; }
 
@@ -323,18 +324,23 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
     return on_disk_size_.load(std::memory_order_relaxed);
   }
 
+  bool supports_live_row_count() const {
+    return supports_live_row_count_;
+  }
+
+  // Return standard "T xxx P yyy" log prefix.
+  const std::string& LogPrefix() const {
+    return log_prefix_;
+  }
+
   // ==========================================================================
   // Stuff used by the tests
   // ==========================================================================
-  const RowSetMetadata *GetRowSetForTests(int64_t id) const;
-
-  RowSetMetadata *GetRowSetForTests(int64_t id);
+  const RowSetMetadata* GetRowSetForTests(int64_t id) const;
+  RowSetMetadata* GetRowSetForTests(int64_t id);
 
   std::unordered_map<int64_t, scoped_refptr<TxnMetadata>> GetTxnMetadata() const;
 
-  // Return standard "T xxx P yyy" log prefix.
-  std::string LogPrefix() const;
-
   int flush_count_for_tests() const {
     return flush_count_for_tests_;
   }
@@ -344,14 +350,9 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
     supports_live_row_count_ = supports_live_row_count;
   }
 
-  bool supports_live_row_count() const {
-    return supports_live_row_count_;
-  }
-
  private:
-  friend class RefCountedThreadSafe<TabletMetadata>;
   friend class MetadataTest;
-  friend class TestTabletMetadataBenchmark;
+  friend class RefCountedThreadSafe<TabletMetadata>;
 
   // Compile time assert that no one deletes TabletMetadata objects.
   ~TabletMetadata();
@@ -383,7 +384,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
 
   // Fully replace superblock.
   // Requires 'flush_lock_'.
-  Status ReplaceSuperBlockUnlocked(const TabletSuperBlockPB &pb);
+  Status ReplaceSuperBlockUnlocked(const TabletSuperBlockPB& pb);
 
   // Requires 'data_lock_'.
   Status UpdateUnlocked(const RowSetMetadataIds& to_remove,
@@ -427,6 +428,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   Partition partition_;
 
   FsManager* const fs_manager_;
+  const std::string log_prefix_;
   RowSetMetadataVector rowsets_;
 
   base::subtle::Atomic64 next_rowset_idx_;
@@ -440,9 +442,11 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   // merged with main state.
   std::unordered_map<int64_t, scoped_refptr<TxnMetadata>> txn_metadata_by_txn_id_;;
 
-  // The current schema version. This is owned by this class.
-  // We don't use unique_ptr so that we can do an atomic swap.
-  Schema* schema_;
+  // The current schema version.
+  std::unique_ptr<Schema> schema_;
+  // The raw pointer to the schema for an atomic swap.
+  Schema* schema_ptr_;
+
   uint32_t schema_version_;
   std::string table_name_;
   PartitionSchema partition_schema_;
@@ -452,7 +456,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   // a given tablet won't have thousands of "alter table" calls.
   // They are kept alive so that callers of schema() don't need to
   // worry about reference counting or locking.
-  std::vector<Schema*> old_schemas_;
+  std::vector<std::unique_ptr<Schema>> old_schemas_;
 
   // Protected by 'data_lock_'.
   BlockIdSet orphaned_blocks_;
diff --git a/src/kudu/tablet/tablet_replica.cc b/src/kudu/tablet/tablet_replica.cc
index a50bdb4..2f037ed 100644
--- a/src/kudu/tablet/tablet_replica.cc
+++ b/src/kudu/tablet/tablet_replica.cc
@@ -302,7 +302,7 @@ Status TabletReplica::Start(
   return Status::OK();
 }
 
-string TabletReplica::StateName() const {
+const string& TabletReplica::StateName() const {
   return TabletStatePB_Name(state());
 }
 
@@ -446,10 +446,6 @@ void TabletReplica::TxnStatusReplicaStateChanged(const string& tablet_id, const
   }
 }
 
-string TabletReplica::LogPrefix() const {
-  return meta_->LogPrefix();
-}
-
 void TabletReplica::set_state(TabletStatePB new_state) {
   switch (new_state) {
     case NOT_INITIALIZED:
@@ -666,9 +662,9 @@ void TabletReplica::SetBootstrapping() {
   set_state(BOOTSTRAPPING);
 }
 
-void TabletReplica::SetStatusMessage(const std::string& status) {
+void TabletReplica::SetStatusMessage(string status) {
   std::lock_guard<simple_spinlock> lock(lock_);
-  last_status_ = status;
+  last_status_ = std::move(status);
 }
 
 string TabletReplica::last_status() const {
diff --git a/src/kudu/tablet/tablet_replica.h b/src/kudu/tablet/tablet_replica.h
index bfcf7a4..c7f4505 100644
--- a/src/kudu/tablet/tablet_replica.h
+++ b/src/kudu/tablet/tablet_replica.h
@@ -229,14 +229,14 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
     return tablet_;
   }
 
-  const TabletStatePB state() const {
+  TabletStatePB state() const {
     std::lock_guard<simple_spinlock> lock(lock_);
     return state_;
   }
 
-  std::string StateName() const;
+  const std::string& StateName() const;
 
-  const TabletDataState data_state() const {
+  TabletDataState data_state() const {
     std::lock_guard<simple_spinlock> lock(lock_);
     return meta_->tablet_data_state();
   }
@@ -249,7 +249,7 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
 
   // Set a user-readable status message about the tablet. This may appear on
   // the Web UI, for example.
-  void SetStatusMessage(const std::string& status);
+  void SetStatusMessage(std::string status);
 
   // Retrieve the last human-readable status of this tablet replica.
   std::string last_status() const;
@@ -513,7 +513,10 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
   void TxnStatusReplicaStateChanged(const std::string& tablet_id,
                                     const std::string& reason);
 
-  std::string LogPrefix() const;
+  const std::string& LogPrefix() const {
+    return meta_->LogPrefix();
+  }
+
   // Transition to another state. Requires that the caller hold 'lock_' if the
   // object has already published to other threads. See tablet/metadata.proto
   // for state descriptions and legal state transitions.
@@ -628,7 +631,5 @@ class FlushInflightsToLogCallback : public RefCountedThreadSafe<FlushInflightsTo
   scoped_refptr<log::Log> log_;
 };
 
-
 }  // namespace tablet
 }  // namespace kudu
-
diff --git a/src/kudu/tserver/tablet_copy-test-base.h b/src/kudu/tserver/tablet_copy-test-base.h
index 0b33b06..0d5221a 100644
--- a/src/kudu/tserver/tablet_copy-test-base.h
+++ b/src/kudu/tserver/tablet_copy-test-base.h
@@ -41,7 +41,7 @@ static const int kNumLogRolls = 2;
 
 class TabletCopyTest : public TabletServerTestBase {
  public:
-  virtual void SetUp() OVERRIDE {
+  void SetUp() override {
     NO_FATALS(TabletServerTestBase::SetUp());
     // Create a tablet server with multiple data dirs. In most cases, this is
     // unimportant, but in some cases can be helpful to test multi-disk
@@ -56,7 +56,7 @@ class TabletCopyTest : public TabletServerTestBase {
     NO_FATALS(GenerateTestData());
   }
 
-  virtual void TearDown() OVERRIDE {
+  void TearDown() override {
     if (tablet_replica_) {
       ASSERT_OK(tablet_replica_->log_anchor_registry()->Unregister(&anchor_));
     }
diff --git a/src/kudu/tserver/tablet_copy_client-test.cc b/src/kudu/tserver/tablet_copy_client-test.cc
index faa10b0..e6c1b9c 100644
--- a/src/kudu/tserver/tablet_copy_client-test.cc
+++ b/src/kudu/tserver/tablet_copy_client-test.cc
@@ -44,7 +44,6 @@
 #include "kudu/fs/data_dirs.h"
 #include "kudu/fs/fs_manager.h"
 #include "kudu/gutil/casts.h"
-#include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/fastmem.h"
 #include "kudu/gutil/strings/join.h"
@@ -69,11 +68,6 @@
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
-using std::shared_ptr;
-using std::string;
-using std::thread;
-using std::vector;
-
 DECLARE_double(env_inject_eio);
 DECLARE_double(tablet_copy_fault_crash_during_download_block);
 DECLARE_double(tablet_copy_fault_crash_during_download_wal);
@@ -83,22 +77,30 @@ DECLARE_string(env_inject_eio_globs);
 
 METRIC_DECLARE_counter(block_manager_total_disk_sync);
 
-namespace kudu {
-namespace tserver {
-
-using consensus::ConsensusMetadataManager;
-using consensus::ConsensusStatePB;
-using consensus::GetRaftConfigLeader;
-using consensus::RaftPeerPB;
-using fs::DataDirManager;
+using kudu::consensus::ConsensusMetadataManager;
+using kudu::consensus::ConsensusStatePB;
+using kudu::consensus::GetRaftConfigLeader;
+using kudu::consensus::RaftPeerPB;
+using kudu::fs::DataDirManager;
+using kudu::tablet::TabletMetadata;
+using std::shared_ptr;
+using std::string;
+using std::thread;
 using std::tuple;
 using std::unique_ptr;
+using std::vector;
 using strings::Substitute;
-using tablet::TabletMetadata;
+
+namespace kudu {
+namespace tserver {
 
 class TabletCopyClientTest : public TabletCopyTest {
  public:
-  virtual void SetUp() OVERRIDE {
+  TabletCopyClientTest()
+      : rand_(SeedRandom()) {
+  }
+
+  void SetUp() override {
     NO_FATALS(TabletCopyTest::SetUp());
 
     // To be a bit more flexible in testing, create a FS layout with multiple disks.
@@ -147,14 +149,14 @@ class TabletCopyClientTest : public TabletCopyTest {
  protected:
   Status CompareFileContents(const string& path1, const string& path2);
 
-  // Injection of 'supports_live_row_count' modifiers through polymorphic characteristic.
+  // Injection of 'supports_live_row_count' modifiers.
   void GenerateTestData() override {
-    Random rand(SeedRandom());
-    NO_FATALS(tablet_replica_->tablet_metadata()->
-        set_supports_live_row_count_for_tests(rand.Next() % 2));
+    tablet_replica_->tablet_metadata()->set_supports_live_row_count_for_tests(
+        rand_.Next() % 2);
     NO_FATALS(TabletCopyTest::GenerateTestData());
   }
 
+  Random rand_;
   MetricRegistry metric_registry_;
   scoped_refptr<MetricEntity> metric_entity_;
   unique_ptr<FsManager> fs_manager_;
@@ -464,7 +466,7 @@ class TabletCopyClientAbortTest : public TabletCopyClientTest,
                                   public ::testing::WithParamInterface<
                                       tuple<DownloadBlocks, DeleteTrigger>> {
  public:
-  virtual void SetUp() override {
+  void SetUp() override {
     TabletCopyClientTest::SetUp();
     ASSERT_OK(StartCopy());
   }