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 2019/10/15 17:17:57 UTC

[kudu] branch branch-1.11.x updated (34ee1ba -> 2b5119e)

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

alexey pushed a change to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git.


    from 34ee1ba  [docs][examples] fix RAT report
     new 6347c66  [tablet] remove used method in Mutation
     new 266589c  [docs] a small update on RELEASING.adoc
     new 1589893  util: gscoped_ptr<ThreadPool> -> unique_ptr<ThreadPool>
     new 8c4f72b  [build] Fix a bug for memory_gc-itest
     new 8b7e939  [test] deflake memory_gc-itest
     new 781cc0b  net_util: tag DNS resolution as a blocking operation
     new 2dadfee  [cfile] KUDU-2852 Push predicate evaluation for int type RLE decoder
     new 2b5119e  KUDU-2800 Test long bootstrapping tablet replicas

The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 RELEASING.adoc                                     |  14 +-
 src/kudu/cfile/rle_block.h                         |  49 ++++
 src/kudu/client/client-internal.cc                 |   4 +-
 src/kudu/codegen/compilation_manager.h             |   8 +-
 src/kudu/common/rowblock.h                         |   7 +-
 src/kudu/consensus/consensus-test-util.h           |   6 +-
 src/kudu/consensus/consensus_peers-test.cc         |   2 +-
 src/kudu/consensus/consensus_queue-test.cc         |   4 +-
 src/kudu/consensus/leader_election-test.cc         |   3 +-
 src/kudu/consensus/log.cc                          |   2 +-
 src/kudu/consensus/log.h                           |   9 +-
 src/kudu/consensus/raft_consensus_quorum-test.cc   |   2 +-
 src/kudu/fs/data_dirs.cc                           |   3 +-
 src/kudu/integration-tests/CMakeLists.txt          |   2 +-
 src/kudu/integration-tests/memory_gc-itest.cc      |  26 +-
 .../integration-tests/raft_config_change-itest.cc  | 152 ++++++++++++
 src/kudu/kserver/kserver.h                         |   7 +-
 src/kudu/master/catalog_manager.cc                 |   8 +
 src/kudu/master/catalog_manager.h                  |   7 +-
 src/kudu/master/master.h                           |  13 +-
 src/kudu/rpc/messenger.h                           |   8 +-
 src/kudu/tablet/all_types-scan-correctness-test.cc | 271 +++++++++++++++++----
 src/kudu/tablet/mutation.h                         |   6 +-
 src/kudu/tablet/tablet_replica-test.cc             |   6 +-
 src/kudu/thrift/client.h                           |   3 +-
 src/kudu/tools/ksck.cc                             |   1 -
 src/kudu/tools/ksck.h                              |  10 +-
 src/kudu/tools/ksck_checksum.h                     |   3 +-
 src/kudu/tools/table_scanner.h                     |   3 +-
 .../tserver/tablet_copy_source_session-test.cc     |   6 +-
 src/kudu/tserver/ts_tablet_manager.cc              |  11 +
 src/kudu/tserver/ts_tablet_manager.h               |   8 +-
 src/kudu/util/countdown_latch-test.cc              |   5 +-
 src/kudu/util/curl_util-test.cc                    |   5 +-
 src/kudu/util/maintenance_manager.h                |   3 +-
 src/kudu/util/net/dns_resolver.h                   |   3 +-
 src/kudu/util/net/net_util.cc                      |   2 +
 src/kudu/util/threadpool-test.cc                   |   5 +-
 src/kudu/util/threadpool.cc                        |   5 +-
 src/kudu/util/threadpool.h                         |   9 +-
 src/kudu/util/trace.h                              |   7 +-
 41 files changed, 539 insertions(+), 169 deletions(-)


[kudu] 08/08: KUDU-2800 Test long bootstrapping tablet replicas

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2b5119e53b61629cc510e0ef07a1314911efe7f2
Author: Volodymyr Verovkin <ve...@cloudera.com>
AuthorDate: Tue Sep 24 16:27:43 2019 -0700

    KUDU-2800 Test long bootstrapping tablet replicas
    
    These tests cover change of consensus state in case of long
    bootstrapping of a tablet replica.
    The following cases are covered:
    
    1) A tablet replica bootstraps long time. No data modications happen
    during that time. The replica is not evicted from the tablet Raft
    configuration and joins the quorum after the bootstrapping process is
    finished.
    
    2) A tablet replica is shut down for some time. During this time a lot
    of data modifications happen, log rolls, the replica is replaced (with
    re-replication). The replica restarts and finds out that is has been
    evicted from the tablet Raft configuration.
    
    3) A tablet replica bootstraps long time. During this time a lot
    of data modifications happen, log rolls, the replica is replaced (with
    re-replication). The replica eventually finishes boostrap and finds out
    that is has been evicted from the tablet Raft configuration.
    
    Change-Id: Ie1fee42053194f51d7a869ce14788095d6627ed9
    Reviewed-on: http://gerrit.cloudera.org:8080/14300
    Tested-by: Alexey Serbin <as...@cloudera.com>
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    (cherry picked from commit 90ec2715171bbf3fca294d5b1d230f951c71e75c)
    Reviewed-on: http://gerrit.cloudera.org:8080/14452
    Tested-by: Kudu Jenkins
---
 .../integration-tests/raft_config_change-itest.cc  | 152 +++++++++++++++++++++
 src/kudu/tserver/ts_tablet_manager.cc              |  11 ++
 2 files changed, 163 insertions(+)

diff --git a/src/kudu/integration-tests/raft_config_change-itest.cc b/src/kudu/integration-tests/raft_config_change-itest.cc
index 9dfb4a1..38b881c 100644
--- a/src/kudu/integration-tests/raft_config_change-itest.cc
+++ b/src/kudu/integration-tests/raft_config_change-itest.cc
@@ -25,6 +25,7 @@
 #include <vector>
 
 #include <boost/optional/optional.hpp>
+#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
@@ -33,14 +34,17 @@
 #include "kudu/consensus/consensus.pb.h"
 #include "kudu/consensus/metadata.pb.h"
 #include "kudu/consensus/quorum_util.h"
+#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
 #include "kudu/integration-tests/mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/raft_consensus-itest-base.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/tserver/tablet_server-test-base.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/status.h"
@@ -56,9 +60,12 @@ using kudu::consensus::MODIFY_PEER;
 using kudu::consensus::RaftPeerAttrsPB;
 using kudu::consensus::RaftPeerPB;
 using kudu::consensus::REMOVE_PEER;
+using kudu::consensus::EXCLUDE_HEALTH_REPORT;
 using kudu::itest::BulkChangeConfig;
 using kudu::itest::GetTableLocations;
 using kudu::itest::TServerDetails;
+using kudu::itest::GetConsensusState;
+using kudu::tserver::RaftConsensusITestBase;
 using kudu::master::VOTER_REPLICA;
 using kudu::pb_util::SecureShortDebugString;
 using std::string;
@@ -66,6 +73,9 @@ using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
+DECLARE_int32(num_replicas);
+DECLARE_int32(num_tablet_servers);
+
 namespace kudu {
 
 class RaftConfigChangeITest : public ExternalMiniClusterITestBase {
@@ -447,4 +457,146 @@ TEST_F(RaftConfigChangeITest, TestBulkChangeConfig) {
   ASSERT_STR_MATCHES(s.ToString(), "only one change allowed per peer");
 }
 
+// KUDU-2800
+// Check re-replication during slow tablet replica bootstrap.
+class SlowTabletBootstrapTest : public RaftConsensusITestBase {
+ protected:
+  const MonoDelta kTimeout = MonoDelta::FromSeconds(30);
+  // Check that 'added_replica_uuid' was added to the consensus
+  // and 'removed_replica_uuid' was removed from consensus.
+  void ValidateConsensusStateChanged(const string& added_replica_uuid,
+                                     const string& removed_replica_uuid,
+                                     const MonoDelta& timeout) {
+    TServerDetails* leader = nullptr;
+    ASSERT_OK(GetLeaderReplicaWithRetries(tablet_id_, &leader));
+    consensus::ConsensusStatePB cstate;
+    ASSERT_OK(GetConsensusState(leader, tablet_id_, timeout,
+                                EXCLUDE_HEALTH_REPORT, &cstate));
+    ASSERT_TRUE(IsRaftConfigMember(added_replica_uuid, cstate.committed_config()));
+    ASSERT_FALSE(IsRaftConfigMember(removed_replica_uuid, cstate.committed_config()));
+  }
+  // Create and start cluster.
+  // Returns 'any_replica_sever' - UUID of first server with tablet replica,
+  // and 'no_replica_server' - UUID of server without tablet replica.
+  void SetUpCluster(string* any_replica_server,
+                    string* no_replica_server) {
+    vector<string> ts_flags {
+      // The default value 5 minutes is very long.
+      // So we set timeout 3 seconds in order to quickly
+      // remove non-responding replica from consensus
+      "--follower_unavailable_considered_failed_sec=3"
+    };
+
+    AddFlagsForLogRolls(&ts_flags);
+    FLAGS_num_tablet_servers = 4;
+    FLAGS_num_replicas = 3;
+    NO_FATALS(BuildAndStart(ts_flags, {}));
+
+    ASSERT_EQ(4, tablet_servers_.size());
+
+    // Extra sanity checks.
+    vector<string> replica_servers = GetServersWithReplica(tablet_id_);
+    ASSERT_EQ(3, replica_servers.size());
+    vector<string> no_replica_servers = GetServersWithoutReplica(tablet_id_);
+    ASSERT_EQ(1, no_replica_servers.size());
+    *any_replica_server = replica_servers.front();
+    *no_replica_server = no_replica_servers.front();
+  }
+};
+
+// Slow tablet replica is not evicted while bootstrapping
+// as long as there are no data modifications in the consensus.
+TEST_F(SlowTabletBootstrapTest, TestSlowBootstrap) {
+  SKIP_IF_SLOW_NOT_ALLOWED();
+
+  string any_replica_server, no_replica_server;
+  NO_FATALS(SetUpCluster(&any_replica_server, &no_replica_server));
+
+  // Shutdown any tablet server with tablet's replica.
+  auto ts = cluster_->tablet_server_by_uuid(any_replica_server);
+  ASSERT_NE(nullptr, ts);
+
+  ts->mutable_flags()->emplace_back("--tablet_bootstrap_inject_latency_ms=4000");
+  ts->Shutdown();
+  ASSERT_OK(ts->Restart());
+  SleepFor(MonoDelta::FromSeconds(7));
+
+  ASSERT_EVENTUALLY([&]() {
+    ValidateConsensusStateChanged(any_replica_server, no_replica_server, kTimeout);
+  });
+}
+
+// If replica restarts after many data modifications,
+// it falls behind and is removed from consensus.
+TEST_F(SlowTabletBootstrapTest, TestFallBehind) {
+  SKIP_IF_SLOW_NOT_ALLOWED();
+
+  string any_replica_server, no_replica_server;
+  NO_FATALS(SetUpCluster(&any_replica_server, &no_replica_server));
+
+  // Shutdown any tablet server with tablet's replica,
+  // add data, then restart and cause it to fall behind.
+  NO_FATALS(CauseSpecificFollowerToFallBehindLogGC(tablet_servers_,
+      any_replica_server, nullptr, nullptr,
+      BehindWalGcBehavior::SHUTDOWN_RESTART));
+
+  ASSERT_EVENTUALLY([&]() {
+    ValidateConsensusStateChanged(no_replica_server, any_replica_server, kTimeout);
+  });
+}
+
+// If there many data modifications during slow replica bootstrap,
+// it falls behind and is removed from consensus.
+TEST_F(SlowTabletBootstrapTest, TestFallBehindSlowBootstrap) {
+  SKIP_IF_SLOW_NOT_ALLOWED();
+
+  string any_replica_server, no_replica_server;
+  NO_FATALS(SetUpCluster(&any_replica_server, &no_replica_server));
+
+  // Shutdown any tablet server with tablet replica.
+  auto ts = cluster_->tablet_server_by_uuid(any_replica_server);
+  ASSERT_NE(nullptr, ts);
+
+  // Inject delay into next tablet replica bootstrap.
+  // When replica finish bootstrapping, it will find that it was left behind
+  // and was removed from consensus.
+  ts->mutable_flags()->emplace_back("--tablet_bootstrap_inject_latency_ms=3000");
+
+  TServerDetails* replica = FindOrDie(tablet_servers_, any_replica_server);
+  ts->Shutdown();
+  ASSERT_OK(ts->Restart());
+
+  // Find a leader.
+  TServerDetails* leader = nullptr;
+  ASSERT_OK(GetLeaderReplicaWithRetries(tablet_id_, &leader));
+  ASSERT_NE(leader, nullptr);
+  ASSERT_NE(leader, replica);
+  int leader_index = cluster_->tablet_server_index_by_uuid(leader->uuid());
+
+  TestWorkload workload(cluster_.get());
+  workload.set_table_name(kTableId);
+  workload.set_timeout_allowed(true);
+  workload.set_payload_bytes(128 * 1024); // Write ops of size 128KB.
+  workload.set_write_batch_size(1);
+  workload.set_num_write_threads(4);
+  workload.Setup();
+  workload.Start();
+
+  LOG(INFO) << "Waiting until we've written at least 4MB...";
+  while (workload.rows_inserted() < 8 * 4) {
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  workload.StopAndJoin();
+
+  LOG(INFO) << "Waiting for log GC on " << leader->uuid();
+  // Some WAL segments must exist, but wal segment 1 must not exist.
+  ASSERT_OK(inspect_->WaitForFilePatternInTabletWalDirOnTs(
+      leader_index, tablet_id_, { "wal-" }, { "wal-000000001" }));
+  LOG(INFO) << "Log GC complete on " << leader->uuid();
+
+  ASSERT_EVENTUALLY([&]() {
+    ValidateConsensusStateChanged(no_replica_server, any_replica_server, kTimeout);
+  });
+}
+
 } // namespace kudu
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index 97a970a..78b6e02 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -138,6 +138,11 @@ DEFINE_int32(update_tablet_stats_interval_ms, 5000,
              "Should be greater than 'heartbeat_interval_ms'");
 TAG_FLAG(update_tablet_stats_interval_ms, advanced);
 
+DEFINE_int32(tablet_bootstrap_inject_latency_ms, 0,
+             "Injects latency into the tablet bootstrapping. "
+             "For use in tests only.");
+TAG_FLAG(tablet_bootstrap_inject_latency_ms, unsafe);
+
 DECLARE_bool(raft_prepare_replacement_before_eviction);
 
 METRIC_DEFINE_gauge_int32(server, tablets_num_not_initialized,
@@ -1068,6 +1073,12 @@ void TSTabletManager::OpenTablet(const scoped_refptr<TabletReplica>& replica,
   VLOG(1) << LogPrefix(tablet_id) << "Bootstrapping tablet";
   TRACE("Bootstrapping tablet");
 
+  if (FLAGS_tablet_bootstrap_inject_latency_ms > 0) {
+    LOG(WARNING) << "Injecting " << FLAGS_tablet_bootstrap_inject_latency_ms
+                     << "ms delay in tablet bootstrapping";
+    SleepFor(MonoDelta::FromMilliseconds(FLAGS_tablet_bootstrap_inject_latency_ms));
+  }
+
   scoped_refptr<ConsensusMetadata> cmeta;
   Status s = cmeta_manager_->Load(replica->tablet_id(), &cmeta);
   auto fail_tablet = MakeScopedCleanup([&]() {


[kudu] 05/08: [test] deflake memory_gc-itest

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8b7e9398a2e650b2bd22e489aaba30b2a0080c28
Author: Yingchun Lai <40...@qq.com>
AuthorDate: Tue Oct 15 10:44:09 2019 +0800

    [test] deflake memory_gc-itest
    
    Produce more rows for test table, and disable periodical tcmalloc
    memory GC before main memory consuming for mini-tservers, to avoid
    memory GC before we check it.
    
    Change-Id: Ied3c38a0e40f0586e929f5102b99c1bf3a6a91c5
    Reviewed-on: http://gerrit.cloudera.org:8080/14437
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    (cherry picked from commit 8d3d3784ff26292f4aedb4c3a7fc2303bc9958b0)
    Reviewed-on: http://gerrit.cloudera.org:8080/14442
    Reviewed-by: Yingchun Lai <40...@qq.com>
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/integration-tests/memory_gc-itest.cc | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/kudu/integration-tests/memory_gc-itest.cc b/src/kudu/integration-tests/memory_gc-itest.cc
index 761756f..eb9c963 100644
--- a/src/kudu/integration-tests/memory_gc-itest.cc
+++ b/src/kudu/integration-tests/memory_gc-itest.cc
@@ -67,13 +67,21 @@ class MemoryGcITest : public ExternalMiniClusterITestBase {
 TEST_F(MemoryGcITest, TestPeriodicGc) {
   vector<string> ts_flags;
   // Set GC interval seconeds short enough, so the test case could compelte sooner.
-  ts_flags.emplace_back("--gc_tcmalloc_memory_interval_seconds=1");
+  ts_flags.emplace_back("--gc_tcmalloc_memory_interval_seconds=5");
 
   ExternalMiniClusterOptions opts;
   opts.extra_tserver_flags = std::move(ts_flags);
   opts.num_tablet_servers = 3;
   NO_FATALS(StartClusterWithOpts(opts));
 
+  // Enable tcmalloc memory GC periodically for tserver-1, and disabled for tserver-0 and tserver-2.
+  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(0),
+                              "gc_tcmalloc_memory_interval_seconds", "0"));
+  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(1),
+                              "gc_tcmalloc_memory_interval_seconds", "1"));
+  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(2),
+                              "gc_tcmalloc_memory_interval_seconds", "0"));
+
   // Write some data for scan later.
   {
     TestWorkload workload(cluster_.get());
@@ -84,19 +92,11 @@ TEST_F(MemoryGcITest, TestPeriodicGc) {
     workload.Setup();
     workload.Start();
     ASSERT_EVENTUALLY([&]() {
-      ASSERT_GE(workload.rows_inserted(), 10000);
+      ASSERT_GE(workload.rows_inserted(), 1000000);
     });
     workload.StopAndJoin();
   }
 
-  // Limit max overhead memory usage of tserver-1, and no limit for tserver-0 and tserver-2.
-  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(0),
-                              "tcmalloc_max_free_bytes_percentage", "100"));
-  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(1),
-                              "tcmalloc_max_free_bytes_percentage", "10"));
-  ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(2),
-                              "tcmalloc_max_free_bytes_percentage", "100"));
-
   // Start scan, then more memory will be allocated by tcmalloc.
   {
     TestWorkload workload(cluster_.get());
@@ -108,11 +108,11 @@ TEST_F(MemoryGcITest, TestPeriodicGc) {
       NO_FATALS(
         double ratio;
         GetOverheadRatio(cluster_->tablet_server(0), &ratio);
-        ASSERT_GE(ratio, 0.1);
+        ASSERT_GE(ratio, 0.1) << "tserver-0";
         GetOverheadRatio(cluster_->tablet_server(1), &ratio);
-        ASSERT_LE(ratio, 0.1);
+        ASSERT_LE(ratio, 0.1) << "tserver-1";
         GetOverheadRatio(cluster_->tablet_server(2), &ratio);
-        ASSERT_GE(ratio, 0.1);
+        ASSERT_GE(ratio, 0.1) << "tserver-2";
       );
     });
     workload.StopAndJoin();


[kudu] 02/08: [docs] a small update on RELEASING.adoc

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 266589c5b6eab1bd9020bc364fda211a2a046798
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Oct 10 10:36:31 2019 -0700

    [docs] a small update on RELEASING.adoc
    
    Minor clarification on the username to use for logging in at
    gerrit.cloudera.org to update gerrit mirroring configuration.  Also,
    fixed formatting of the numbered sub-list and added an item to verify
    that gerrit has been restarted successfully.
    
    Change-Id: I3046e24b63b1d70a0e5f4d5d28b5335a26255137
    Reviewed-on: http://gerrit.cloudera.org:8080/14411
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    (cherry picked from commit 6f49b2dc076603d83d4ce2abaab47eab960b47bd)
    Reviewed-on: http://gerrit.cloudera.org:8080/14448
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 RELEASING.adoc | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/RELEASING.adoc b/RELEASING.adoc
index 6f19b46..a09a409 100644
--- a/RELEASING.adoc
+++ b/RELEASING.adoc
@@ -67,17 +67,19 @@ in `master`.
   have to perform this step because SSH access is behind a firewall. The steps
   are as follows:
   1. Ensure your public SSH key is in `~gerrit/.ssh/authorized_keys` on gerrit.cloudera.org
-  2. From behind the firewall, `ssh gerrit.cloudera.org` to log in.
-  3. Change to the gerrit user, `sudo su gerrit`
-  4. Back up the existing replication configuration file by executing
+  2. From behind the firewall, `ssh gerrit@gerrit.cloudera.org` to log in.
+  3. Back up the existing replication configuration file by executing
      `cp ~/etc/replication.config ~/etc/replication.config.bak.`date '+%Y%m%d.%H%M%S'``
-  5. Edit `etc/replication.config` to add a line for the new branch, such as `branch-1.x.y`
-  6. Send email to the dev lists for Kudu and Impala (dev@kudu.apache.org and
+  4. Edit `etc/replication.config` to add a line for the new branch, such as `branch-1.x.y`
+  5. Send email to the dev lists for Kudu and Impala (dev@kudu.apache.org and
      dev@impala.apache.org) indicating that you are going to restart Gerrit
      (link:https://s.apache.org/2Wj7[example]). It is best to do the restart at
      some time of day when you don't expect many people to be using the system,
      since Gerrit can take a few minutes to restart.
-  7: Restart Gerrit, `~/bin/gerrit.sh restart`
+  6. Restart Gerrit: `~/bin/gerrit.sh restart`
+  7. Make sure Gerrit has been successfully restarted: after a few minutes,
+     try to open the link:https://gerrit.cloudera.org/#/admin/projects[project list]
+     page in your favorite browser.
 
 . As needed, patches can be cherry-picked to the new branch.
 


[kudu] 01/08: [tablet] remove used method in Mutation

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6347c66b0e3341aa09d5d3bde1a2d89fbedd922d
Author: lingbin <li...@gmail.com>
AuthorDate: Thu Oct 10 22:28:36 2019 +0800

    [tablet] remove used method in Mutation
    
    Change-Id: Ic3c7cded06ce7fead5f56371166e3908d16efa7a
    Reviewed-on: http://gerrit.cloudera.org:8080/14409
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    (cherry picked from commit b6e4672092292acf7316a700c71fad3bb54ffa77)
    Reviewed-on: http://gerrit.cloudera.org:8080/14447
---
 src/kudu/tablet/mutation.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/kudu/tablet/mutation.h b/src/kudu/tablet/mutation.h
index 9071665..5a28c13 100644
--- a/src/kudu/tablet/mutation.h
+++ b/src/kudu/tablet/mutation.h
@@ -94,17 +94,13 @@ class Mutation {
     *list = this;
   }
 
-  // O(n) algorithm to reverse the order of a linked list of
-  // mutations.
+  // O(n) algorithm to reverse the order of a linked list of mutations.
   static void ReverseMutationList(Mutation** list);
 
  private:
   friend class MSRow;
   friend class MemRowSet;
 
-  template<bool ATOMIC>
-  void DoAppendToList(Mutation **list);
-
   // The transaction ID which made this mutation. If this transaction is not
   // committed in the snapshot of the reader, this mutation should be ignored.
   Timestamp timestamp_;


[kudu] 07/08: [cfile] KUDU-2852 Push predicate evaluation for int type RLE decoder

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2dadfee143db3703a163e15fe512f497dbe3ebb2
Author: Bankim Bhavsar <ba...@cloudera.com>
AuthorDate: Thu Oct 3 14:43:22 2019 -0700

    [cfile] KUDU-2852 Push predicate evaluation for int type RLE decoder
    
    This change adds optimization that evaluates the predicate for each run
    instead of materializing each cell and then applying the predicate
    for integer datatype RLE decoder.
    
    Added a utility method in SelectionVectorView to clear bits
    from caller specified offset. This helps clear batch of rows
    from a caller maintained offset without advancing the internal
    row_offset in SelectionVectorView.
    
    Tests:
    
    To benchmark, adjusted the all_types-scan-correctness-test and
    tested with 1M rows and run-length of 10k repeating
    integer values on a release build.
    
    Following are results with different predicate values where 1st line
    is scan time duration with decoder level evaluation and 2nd line
    is scan time duration with decoder level evaluation turned off
    (i.e. --materializing_iterator_decoder_eval = false)
    
    Small subset: [5 10)
    3-5ms
    9-12ms
    
    Large subset ~50%: [2000 - 7000)
    3-5ms
    8-9ms
    
    Select All: [1, 10001)
    12-15ms
    18-22ms
    
    Select None: [10001 10003)
    3-5ms
    9-12ms
    
    Biggest improvement of around %60 is seen in cases where small subset or
    no rows are selected. As more rows are selected the improvement reduces to
    40-50%.
    
    Change-Id: I6e05775ec1301d3d0b0365a7704b8e962a20455e
    Reviewed-on: http://gerrit.cloudera.org:8080/14380
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
    (cherry picked from commit 384a535a0079ec634d57b55593962cae4cb6f19a)
    Reviewed-on: http://gerrit.cloudera.org:8080/14453
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/cfile/rle_block.h                         |  49 ++++
 src/kudu/common/rowblock.h                         |   7 +-
 src/kudu/tablet/all_types-scan-correctness-test.cc | 271 +++++++++++++++++----
 3 files changed, 274 insertions(+), 53 deletions(-)

diff --git a/src/kudu/cfile/rle_block.h b/src/kudu/cfile/rle_block.h
index fd260f8..7595494 100644
--- a/src/kudu/cfile/rle_block.h
+++ b/src/kudu/cfile/rle_block.h
@@ -423,6 +423,55 @@ class RleIntBlockDecoder final : public BlockDecoder {
     return Status::OK();
   }
 
+  Status CopyNextAndEval(size_t* n,
+                         ColumnMaterializationContext* ctx,
+                         SelectionVectorView* sel,
+                         ColumnDataView* dst) override {
+    DCHECK(parsed_);
+
+    DCHECK_LE(*n, dst->nrows());
+    DCHECK_EQ(dst->stride(), sizeof(CppType));
+
+    ctx->SetDecoderEvalSupported();
+    if (PREDICT_FALSE(*n == 0 || cur_idx_ >= num_elems_)) {
+      *n = 0;
+      return Status::OK();
+    }
+
+    const size_t to_fetch = std::min(*n, static_cast<size_t>(num_elems_ - cur_idx_));
+    size_t remaining = to_fetch;
+    uint8_t* data_ptr = dst->data();
+    size_t row_offset = 0;
+    while (remaining > 0) {
+      CppType val = 0;
+      const size_t num_read = rle_decoder_.GetNextRun(&val, remaining);
+      DCHECK(num_read > 0);
+      DCHECK_LE(num_read, remaining);
+      if (ctx->pred()->EvaluateCell<IntType>(static_cast<const void*>(&val))) {
+        // Copy data for matching predicate
+        for (size_t row_idx = row_offset; row_idx < row_offset + num_read;
+             ++row_idx, data_ptr += kCppTypeSize) {
+          // Skip copying if the row has already been cleared.
+          if (!sel->TestBit(row_idx)) {
+            continue;
+          }
+          *(reinterpret_cast<CppType *>(data_ptr)) = val;
+        }
+      } else {
+        // Mark that the rows will not be returned.
+        sel->ClearBits(num_read, row_offset);
+        data_ptr += num_read * kCppTypeSize;
+      }
+      remaining -= num_read;
+      row_offset += num_read;
+    }
+
+    cur_idx_ += to_fetch;
+    *n = to_fetch;
+
+    return Status::OK();
+  }
+
   virtual bool HasNext() const OVERRIDE {
     return cur_idx_ < num_elems_;
   }
diff --git a/src/kudu/common/rowblock.h b/src/kudu/common/rowblock.h
index d65ee4e..98e2283 100644
--- a/src/kudu/common/rowblock.h
+++ b/src/kudu/common/rowblock.h
@@ -214,9 +214,10 @@ class SelectionVectorView {
     DCHECK_LE(row_idx, sel_vec_->nrows() - row_offset_);
     return BitmapTest(sel_vec_->bitmap(), row_offset_ + row_idx);
   }
-  void ClearBits(size_t nrows) {
-    DCHECK_LE(nrows, sel_vec_->nrows() - row_offset_);
-    BitmapChangeBits(sel_vec_->mutable_bitmap(), row_offset_, nrows, false);
+  // Clear "nrows" bits from the supplied "offset" in the current view.
+  void ClearBits(size_t nrows, size_t offset = 0) {
+    DCHECK_LE(offset + nrows, sel_vec_->nrows() - row_offset_);
+    BitmapChangeBits(sel_vec_->mutable_bitmap(), row_offset_ + offset, nrows, false);
   }
  private:
   SelectionVector* sel_vec_;
diff --git a/src/kudu/tablet/all_types-scan-correctness-test.cc b/src/kudu/tablet/all_types-scan-correctness-test.cc
index 19f3232..2df112e 100644
--- a/src/kudu/tablet/all_types-scan-correctness-test.cc
+++ b/src/kudu/tablet/all_types-scan-correctness-test.cc
@@ -44,6 +44,7 @@
 #include "kudu/util/memory/arena.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/status.h"
+#include "kudu/util/stopwatch.h"
 #include "kudu/util/test_macros.h"
 
 using std::unique_ptr;
@@ -189,9 +190,10 @@ struct NumTypeRowOps : public RowOpsBase {
   size_t cur;
 };
 
-// Calculates the number of values in the range [lower, upper) given a repeated, completely
-// non-null pattern with the specified cardinality and the specified number of rows.
-int ExpectedCount(int nrows, int cardinality, int lower_val, int upper_val) {
+// Calculates the number of values in the range [lower, upper) given a sequential, completely
+// non-null pattern that is repeated with the specified cardinality and the specified number
+// of rows.
+int ExpectedCountSequential(int nrows, int cardinality, int lower_val, int upper_val) {
   if (lower_val >= upper_val || lower_val >= cardinality) {
     return 0;
   }
@@ -216,6 +218,50 @@ int ExpectedCount(int nrows, int cardinality, int lower_val, int upper_val) {
   return (nrows / cardinality) * (upper - lower) + last_chunk_count;
 }
 
+// Calculates number of values in the range [lower_val, upper_val) for a repeating pattern
+// like [00111 00222 00333]. The value lies between range [0, cardinality) and
+// repeats every "cardinality" chunks where each chunk is "cardinality" rows.
+// E.g. nrows: 34, cardinality: 5, null_upper: 2, lower_val: 1, upper_val: 4
+// [-, -, 0, 0, 0, -, -, 1, 1, 1, -, -, 2, 2, 2, -, -, 3, 3, 3, -, -, 4, 4, 4, <-- stride
+//  -, -, 0, 0, 0, -, -, 1, 1]
+int ExpectedCountRepeating(int nrows, int cardinality, int null_upper, int lower_val,
+                           int upper_val) {
+
+  if (lower_val >= upper_val || lower_val >= cardinality || null_upper >= cardinality) {
+    return 0;
+  }
+  int lower = std::max(0, lower_val);
+  int upper = std::min(cardinality, upper_val);
+
+  // Each stride comprises of cardinality chunks and each chunk comprises of cardinality rows.
+  // For above example there is 1 full stride comprising of 25 (5 * 5) rows.
+  int strides = nrows / (cardinality * cardinality);
+  // For above example there are 3 full matching chunks in a full stride.
+  int matching_chunks_per_stride = upper - lower;
+  // Matching rows in a chunk where chunk itself is within lower and upper.
+  // For above example if value in chunk lies between lower and upper
+  // then there are 3 rows in each such matching chunk.
+  int matching_rows_per_chunk = cardinality - null_upper;
+
+  // For above example there is 1 full chunk in last partial stride.
+  int chunks_in_last_stride = (nrows % (cardinality * cardinality)) / cardinality;
+  int matching_chunks_in_last_stride = std::max(0, std::min(upper, chunks_in_last_stride) - lower);
+
+  // For above example there are 4 remainder rows [-, -, 1, 1], 2 of which are within range.
+  int remainder_rows = (nrows % (cardinality * cardinality)) % cardinality;
+  int remainder_matching_rows = std::max(0, remainder_rows - null_upper);
+  if (remainder_matching_rows > 0) {
+    int val_in_remainder_rows = chunks_in_last_stride;
+    if (!(val_in_remainder_rows >= lower && val_in_remainder_rows < upper)) {
+      remainder_matching_rows = 0;
+    }
+  }
+
+  return strides * matching_chunks_per_stride * matching_rows_per_chunk +
+      matching_chunks_in_last_stride * matching_rows_per_chunk +
+      remainder_matching_rows;
+}
+
 std::string TraceMsg(const std::string& test_name,  int expected, int actual) {
   std::ostringstream ss;
   ss << test_name << " Scan Results: " << expected << " expected, " << actual << " returned.";
@@ -236,11 +282,12 @@ public:
     KuduTabletTest::SetUp();
   }
 
-  // Adds a pattern of repeated values with the first "null_upper" of every "cardinality" rows
-  // being set to null.
+  // Adds a pattern of sequential values in every "cardinality" rows with the
+  // first "null_upper" being set to null. This pattern of sequential values is then
+  // repeated after every "cardinality" rows.
   // E.g. nrows: 9, cardinality: 5, null_upper: 2
   // [-, -, 2, 3, 4, -, -, 2, 3]
-  void FillTestTablet(int nrows, int cardinality, int null_upper) {
+  void FillTestTabletWithSequentialPattern(int nrows, int cardinality, int null_upper) {
     base_nrows_ = nrows;
     base_cardinality_ = cardinality;
     base_null_upper_ = null_upper;
@@ -261,6 +308,37 @@ public:
     ASSERT_OK(tablet()->Flush());
   }
 
+  // Adds a pattern of repeating value for every "cardinality" rows with the
+  // first "null_upper" being set to null. The repeating value cycles from
+  // [0, cardinality) values after every "cardinality" rows. This helps test RLE.
+  // E.g. nrows: 34, cardinality: 5, null_upper: 2
+  // [-, -, 0, 0, 0, -, -, 1, 1, 1, -, -, 2, 2, 2, -, -, 3, 3, 3, -, -, 4, 4, 4, <-- stride
+  //  -, -, 0, 0, 0, -, -, 1, 1]
+  void FillTestTabletWithRepeatPattern(int nrows, int cardinality, int null_upper) {
+    base_nrows_ = nrows;
+    base_cardinality_ = cardinality;
+    base_null_upper_ = null_upper;
+    LocalTabletWriter writer(tablet().get(), &client_schema_);
+    KuduPartialRow row(&client_schema_);
+    int val = 0;
+    for (int i = 0; i < nrows;) {
+      CHECK_OK(row.SetInt32(0, i));
+      // Populate the bottom of the repeating pattern with NULLs.
+      // Note: Non-positive null_upper will yield a completely non-NULL column.
+      if (i % cardinality < null_upper) {
+        rowops_.GenerateRow(-1, false, &row);
+      } else {
+        rowops_.GenerateRow(val, false, &row);
+      }
+      ASSERT_OK_FAST(writer.Insert(row));
+      i++;
+      if (i % cardinality == 0) {
+        val = (val + 1) % cardinality;
+      }
+    }
+    ASSERT_OK(tablet()->Flush());
+  }
+
   // Adds the above pattern to the table with keys starting after the base rows.
   void FillAlteredTestTablet(int nrows) {
     added_nrows_ = nrows;
@@ -313,14 +391,83 @@ public:
     ASSERT_OK(SilentIterateToStringList(iter.get(), count));
   }
 
-  void RunTests() {
-    RunUnalteredTabletTests();
-    RunAlteredTabletTests();
+  void RunSequentialTests() {
+    RunUnalteredSequentialTabletTests();
+    RunAlteredSequentialTabletTests();
+  }
+
+  void RunRepeatingTests() {
+    RunUnalteredRepeatingTabletTests();
+  }
+
+  void RunUnalteredRepeatingTabletTests() {
+    int count = 0;
+    {
+      ScanSpec spec;
+      auto pred = rowops_.GenerateRangePredicate(schema_, kColA, kLower, kUpper);
+      spec.AddPredicate(pred);
+      LOG_TIMING(INFO, "Range") {
+        ScanWithSpec(schema_, spec, &count);
+      }
+      int expected_count = ExpectedCountRepeating(base_nrows_, base_cardinality_, base_null_upper_,
+                                                  kLower, kUpper);
+      SCOPED_TRACE(TraceMsg("Range", expected_count, count));
+      ASSERT_EQ(expected_count, count);
+    }
+
+    {
+      // MultiColumn Range scan
+      // This predicates two columns:
+      //   col_a: [0, upper_val)
+      //   col_b: [lower_val, cardinality)
+      // Since the two columns have identical data, the result will be:
+      //   AND:   [lower_val, upper_val)
+      ScanSpec spec;
+      ColumnPredicate pred_a = rowops_.GenerateRangePredicate(schema_, kColA, 0, kUpper);
+      spec.AddPredicate(pred_a);
+      ColumnPredicate pred_b = rowops_.GenerateRangePredicate(schema_, kColB, kLower,
+                                                              base_cardinality_);
+      spec.AddPredicate(pred_b);
+      LOG_TIMING(INFO, "MultiColumn Range") {
+        ScanWithSpec(schema_, spec, &count);
+      }
+      int expected_count = ExpectedCountRepeating(base_nrows_, base_cardinality_, base_null_upper_,
+                                                  kLower, kUpper);
+      SCOPED_TRACE(TraceMsg("MultiColumn Range", expected_count, count));
+      ASSERT_EQ(expected_count, count);
+    }
+
+    {
+      ScanSpec spec;
+      auto pred = ColumnPredicate::IsNotNull(schema_.column(kColB));
+      spec.AddPredicate(pred);
+      LOG_TIMING(INFO, "IsNotNull") {
+        ScanWithSpec(schema_, spec, &count);
+      }
+      int expected_count = ExpectedCountRepeating(base_nrows_, base_cardinality_, base_null_upper_,
+                                                  0, base_cardinality_);
+      SCOPED_TRACE(TraceMsg("IsNotNull", expected_count, count));
+      ASSERT_EQ(expected_count, count);
+    }
+
+    {
+      ScanSpec spec;
+      auto pred = ColumnPredicate::IsNull(schema_.column(kColB));
+      spec.AddPredicate(pred);
+      LOG_TIMING(INFO, "IsNull") {
+        ScanWithSpec(schema_, spec, &count);
+      }
+      int expected_count = base_nrows_ -
+          ExpectedCountRepeating(base_nrows_, base_cardinality_, base_null_upper_, 0,
+                                 base_cardinality_);
+      SCOPED_TRACE(TraceMsg("IsNull", expected_count, count));
+      ASSERT_EQ(expected_count, count);
+    }
   }
 
   // Runs queries on an un-altered table. Correctness is determined by comparing the number of rows
   // returned with the number of rows expected by each query.
-  void RunUnalteredTabletTests() {
+  void RunUnalteredSequentialTabletTests() {
     int lower_non_null = kLower;
     if (kLower < base_null_upper_) {
       lower_non_null = base_null_upper_;
@@ -332,7 +479,8 @@ public:
       auto pred = rowops_.GenerateRangePredicate(schema_, kColA, kLower, kUpper);
       spec.AddPredicate(pred);
       ScanWithSpec(schema_, spec, &count);
-      int expected_count = ExpectedCount(base_nrows_, base_cardinality_, lower_non_null, kUpper);
+      int expected_count =
+          ExpectedCountSequential(base_nrows_, base_cardinality_, lower_non_null, kUpper);
       SCOPED_TRACE(TraceMsg("Range", expected_count, count));
       ASSERT_EQ(expected_count, count);
     }
@@ -350,7 +498,8 @@ public:
                                                               base_cardinality_);
       spec.AddPredicate(pred_b);
       ScanWithSpec(schema_, spec, &count);
-      int expected_count = ExpectedCount(base_nrows_, base_cardinality_, lower_non_null, kUpper);
+      int expected_count =
+          ExpectedCountSequential(base_nrows_, base_cardinality_, lower_non_null, kUpper);
       SCOPED_TRACE(TraceMsg("MultiColumn Range", expected_count, count));
       ASSERT_EQ(expected_count, count);
     }
@@ -359,8 +508,8 @@ public:
       auto pred = ColumnPredicate::IsNotNull(schema_.column(kColB));
       spec.AddPredicate(pred);
       ScanWithSpec(schema_, spec, &count);
-      int expected_count = ExpectedCount(base_nrows_, base_cardinality_, base_null_upper_,
-          base_cardinality_);
+      int expected_count = ExpectedCountSequential(base_nrows_, base_cardinality_, base_null_upper_,
+                                                   base_cardinality_);
       SCOPED_TRACE(TraceMsg("IsNotNull", expected_count, count));
       ASSERT_EQ(expected_count, count);
     }
@@ -369,7 +518,8 @@ public:
       auto pred = ColumnPredicate::IsNull(schema_.column(kColB));
       spec.AddPredicate(pred);
       ScanWithSpec(schema_, spec, &count);
-      int expected_count = ExpectedCount(base_nrows_, base_cardinality_, 0, base_null_upper_);
+      int expected_count =
+          ExpectedCountSequential(base_nrows_, base_cardinality_, 0, base_null_upper_);
       SCOPED_TRACE(TraceMsg("IsNull", expected_count, count));
       ASSERT_EQ(expected_count, count);
     }
@@ -377,7 +527,7 @@ public:
 
   // Runs tests with an altered table. Queries are run with different read-defaults and are deemed
   // correct if they return the correct number of results.
-  void RunAlteredTabletTests() {
+  void RunAlteredSequentialTabletTests() {
     int lower_non_null = kLower;
     // Determine the lowest non-null value in the data range. Used in getting expected counts.
     if (kLower < base_null_upper_) {
@@ -403,10 +553,10 @@ public:
       spec.AddPredicate(pred_b);
       spec.AddPredicate(pred_c);
       ScanWithSpec(altered_schema_, spec, &count);
-      int base_expected_count = ExpectedCount(base_nrows_,
-                                              base_cardinality_,
-                                              lower_non_null,
-                                              kUpper);
+      int base_expected_count = ExpectedCountSequential(base_nrows_,
+                                                        base_cardinality_,
+                                                        lower_non_null,
+                                                        kUpper);
       // Since the new column has the same data as the base columns, IsNull with a Range predicate
       // should yield no rows from the added rows.
       int altered_expected_count = 0;
@@ -425,10 +575,10 @@ public:
       // Since the table has a null read-default on the added column, the IsNotNull predicate
       // should filter out all rows in the base data.
       int base_expected_count = 0;
-      int altered_expected_count = ExpectedCount(added_nrows_,
-                                                 base_cardinality_,
-                                                 lower_non_null,
-                                                 kUpper);
+      int altered_expected_count = ExpectedCountSequential(added_nrows_,
+                                                           base_cardinality_,
+                                                           lower_non_null,
+                                                           kUpper);
       int expected_count = base_expected_count + altered_expected_count;
       SCOPED_TRACE(TraceMsg("Null default, Range+IsNotNull", expected_count, count));
       ASSERT_EQ(expected_count, count);
@@ -445,10 +595,10 @@ public:
       // Since the added column has a null read-default, the base rows will be completely filtered.
       int base_expected_count = 0;
       // The added data will be predicated with [lower, upper).
-      int altered_expected_count = ExpectedCount(added_nrows_,
-                                                 base_cardinality_,
-                                                 lower_non_null,
-                                                 kUpper);
+      int altered_expected_count = ExpectedCountSequential(added_nrows_,
+                                                           base_cardinality_,
+                                                           lower_non_null,
+                                                           kUpper);
       int expected_count = base_expected_count + altered_expected_count;
       SCOPED_TRACE(TraceMsg("Null default, Range", expected_count, count));
       ASSERT_EQ(expected_count, count);
@@ -490,12 +640,14 @@ public:
       spec.AddPredicate(pred_b);
       spec.AddPredicate(pred_c);
       ScanWithSpec(altered_schema_, spec, &count);
-      int base_expected_count = ExpectedCount(base_nrows_, base_cardinality_, lower_non_null,
-                                              kUpper);
+      int base_expected_count =
+          ExpectedCountSequential(base_nrows_, base_cardinality_, lower_non_null,
+                                  kUpper);
       // Since the new column has the same data as the base columns, IsNotNull with a Range
       // predicate should yield the same rows as the Range query alone on the altered data.
-      int altered_expected_count = ExpectedCount(added_nrows_, base_cardinality_, lower_non_null,
-                                                 kUpper);
+      int altered_expected_count =
+          ExpectedCountSequential(added_nrows_, base_cardinality_, lower_non_null,
+                                  kUpper);
       int expected_count = base_expected_count + altered_expected_count;
       SCOPED_TRACE(TraceMsg("Non-null default, Range+IsNotNull", expected_count, count));
       ASSERT_EQ(expected_count, count);
@@ -516,11 +668,12 @@ public:
       }
       // Because the read_default is in range, the predicate on "val_c" will be satisfied
       // by base data, and all rows that satisfy "pred_b" will be returned.
-      int base_expected_count = ExpectedCount(base_nrows_, base_cardinality_, lower, kUpper);
-      int altered_expected_count = ExpectedCount(added_nrows_,
-                                                 base_cardinality_,
-                                                 lower_non_null,
-                                                 kUpper);
+      int base_expected_count =
+          ExpectedCountSequential(base_nrows_, base_cardinality_, lower, kUpper);
+      int altered_expected_count = ExpectedCountSequential(added_nrows_,
+                                                           base_cardinality_,
+                                                           lower_non_null,
+                                                           kUpper);
       int expected_count = base_expected_count + altered_expected_count;
       SCOPED_TRACE(TraceMsg("Non-null default, Range with Default", expected_count, count));
       ASSERT_EQ(expected_count, count);
@@ -542,10 +695,10 @@ public:
       // Because the read_default is out of range, the "pred_c" will not be satisfied by base data,
       // so all base rows will be filtered.
       int base_expected_count = 0;
-      int altered_expected_count = ExpectedCount(added_nrows_,
-                                                 base_cardinality_,
-                                                 default_plus_one,
-                                                 kUpper);
+      int altered_expected_count = ExpectedCountSequential(added_nrows_,
+                                                           base_cardinality_,
+                                                           default_plus_one,
+                                                           kUpper);
       int expected_count = base_expected_count + altered_expected_count;
       SCOPED_TRACE(TraceMsg("Non-null default, Range without Default", expected_count, count));
       ASSERT_EQ(expected_count, count);
@@ -599,22 +752,40 @@ typedef ::testing::Types<NumTypeRowOps<KeyTypeWrapper<INT8, BIT_SHUFFLE>>,
 
 TYPED_TEST_CASE(AllTypesScanCorrectnessTest, KeyTypes);
 
-TYPED_TEST(AllTypesScanCorrectnessTest, AllNonNull) {
+TYPED_TEST(AllTypesScanCorrectnessTest, AllNonNullSequential) {
   int null_upper = 0;
-  this->FillTestTablet(kNumBaseRows, kCardinality, null_upper);
-  this->RunTests();
+  this->FillTestTabletWithSequentialPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunSequentialTests();
 }
 
-TYPED_TEST(AllTypesScanCorrectnessTest, SomeNull) {
+TYPED_TEST(AllTypesScanCorrectnessTest, SomeNullSequential) {
   int null_upper = kUpper/2;
-  this->FillTestTablet(kNumBaseRows, kCardinality, null_upper);
-  this->RunTests();
+  this->FillTestTabletWithSequentialPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunSequentialTests();
+}
+
+TYPED_TEST(AllTypesScanCorrectnessTest, AllNullSequential) {
+  int null_upper = kCardinality;
+  this->FillTestTabletWithSequentialPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunSequentialTests();
 }
 
-TYPED_TEST(AllTypesScanCorrectnessTest, AllNull) {
+TYPED_TEST(AllTypesScanCorrectnessTest, AllNonNullRepeating) {
+  int null_upper = 0;
+  this->FillTestTabletWithRepeatPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunRepeatingTests();
+}
+
+TYPED_TEST(AllTypesScanCorrectnessTest, AllNullRepeating) {
   int null_upper = kCardinality;
-  this->FillTestTablet(kNumBaseRows, kCardinality, null_upper);
-  this->RunTests();
+  this->FillTestTabletWithRepeatPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunRepeatingTests();
+}
+
+TYPED_TEST(AllTypesScanCorrectnessTest, SomeNullRepeating) {
+  int null_upper = kUpper / 2;
+  this->FillTestTabletWithRepeatPattern(kNumBaseRows, kCardinality, null_upper);
+  this->RunRepeatingTests();
 }
 
 }  // namespace tablet


[kudu] 04/08: [build] Fix a bug for memory_gc-itest

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8c4f72b6c83be929b54c558f876f5961a15c3737
Author: oclarms <oc...@gmail.com>
AuthorDate: Fri Oct 11 16:52:54 2019 +0800

    [build] Fix a bug for memory_gc-itest
    
    In CMakeList.txt, we should use KUDU_TCMALLOC_AVAILABLE to determine
    whether tcmalloc is enabled instead of TCMALLOC_ENABLED.
    
    Change-Id: I2982af9c7070ffd0f53791d4960c3bccfee4a8cc
    Reviewed-on: http://gerrit.cloudera.org:8080/14415
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    (cherry picked from commit 06621af23fc29366f33e4950e035502457f1a3b6)
    Reviewed-on: http://gerrit.cloudera.org:8080/14449
    Reviewed-by: Yingchun Lai <40...@qq.com>
---
 src/kudu/integration-tests/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index cd103ab..08eb111 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -94,7 +94,7 @@ ADD_KUDU_TEST_DEPENDENCIES(master_migration-itest
 ADD_KUDU_TEST(master_replication-itest)
 ADD_KUDU_TEST(master_sentry-itest RUN_SERIAL true NUM_SHARDS 8 PROCESSORS 4)
 ADD_KUDU_TEST(master-stress-test RUN_SERIAL true NUM_SHARDS 3)
-if (TCMALLOC_ENABLED)
+if(${KUDU_TCMALLOC_AVAILABLE})
   ADD_KUDU_TEST(memory_gc-itest)
 endif()
 ADD_KUDU_TEST(multidir_cluster-itest)


[kudu] 06/08: net_util: tag DNS resolution as a blocking operation

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 781cc0bc6c9318bf58d0664eafde407ba5fc78e0
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Sun Oct 13 21:41:39 2019 -0700

    net_util: tag DNS resolution as a blocking operation
    
    ThreadRestrictions tags are basically only used to prevent reactor threads
    from doing IO or invoking blocking operations. For some reason we didn't
    annotate DNS resolution as blocking. After doing so, I had to make a few
    exceptions in existing code. They're not great, but at least they're not in
    tserver reactors.
    
    Change-Id: I90e280855bc8e6797f887da26dfa67cfaf945dcf
    Reviewed-on: http://gerrit.cloudera.org:8080/14424
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Kudu Jenkins
    (cherry picked from commit 4c4abfe875bf949613f3f0fef5941ed60039ee62)
    Reviewed-on: http://gerrit.cloudera.org:8080/14451
    Tested-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/client/client-internal.cc | 4 +++-
 src/kudu/master/catalog_manager.cc | 8 ++++++++
 src/kudu/master/catalog_manager.h  | 1 -
 src/kudu/util/net/net_util.cc      | 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 3e1d8e1..befa03c 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -648,7 +648,9 @@ void KuduClient::Data::ConnectToClusterAsync(KuduClient* client,
     HostPort hp;
     Status s = hp.ParseString(master_server_addr, master::Master::kDefaultPort);
     if (s.ok()) {
-      // TODO(todd): Do address resolution asynchronously as well.
+      // TODO(todd): Until address resolution is done asynchronously, we need
+      // to allow waiting as some callers may be reactor threads.
+      ThreadRestrictions::ScopedAllowWait allow_wait;
       s = dns_resolver_->ResolveAddresses(hp, &addrs);
     }
     if (!s.ok()) {
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index f3d5bc1..6108981 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -88,6 +88,7 @@
 #include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/bind.h"
 #include "kudu/gutil/bind_helpers.h"
+#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
@@ -142,6 +143,7 @@
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/stopwatch.h"
 #include "kudu/util/thread.h"
+#include "kudu/util/thread_restrictions.h"
 #include "kudu/util/threadpool.h"
 #include "kudu/util/trace.h"
 
@@ -3335,6 +3337,12 @@ Status RetryingTSRpcTask::ResetTSProxy() {
   // so the task need not take ownership of the returned pointer.
   target_ts_desc_ = ts_desc.get();
 
+  // We may be called by a reactor thread, and creating proxies may trigger DNS
+  // resolution.
+  //
+  // TODO(adar): make the DNS resolution asynchronous.
+  ThreadRestrictions::ScopedAllowWait allow_wait;
+
   shared_ptr<tserver::TabletServerAdminServiceProxy> ts_proxy;
   RETURN_NOT_OK(target_ts_desc_->GetTSAdminProxy(master_->messenger(), &ts_proxy));
   ts_proxy_.swap(ts_proxy);
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index f4ce890..2dd529f 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -37,7 +37,6 @@
 #include <sparsehash/dense_hash_map>
 
 #include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
index c9f1129..74653dd 100644
--- a/src/kudu/util/net/net_util.cc
+++ b/src/kudu/util/net/net_util.cc
@@ -54,6 +54,7 @@
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/stopwatch.h"
 #include "kudu/util/subprocess.h"
+#include "kudu/util/thread_restrictions.h"
 #include "kudu/util/trace.h"
 
 // Mac OS 10.9 does not appear to define HOST_NAME_MAX in unistd.h
@@ -91,6 +92,7 @@ Status GetAddrInfo(const string& hostname,
                    const addrinfo& hints,
                    const string& op_description,
                    AddrInfo* info) {
+  ThreadRestrictions::AssertWaitAllowed();
   addrinfo* res = nullptr;
   const int rc = getaddrinfo(hostname.c_str(), nullptr, &hints, &res);
   const int err = errno; // preserving the errno from the getaddrinfo() call


[kudu] 03/08: util: gscoped_ptr -> unique_ptr

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1589893e4a7b17359784680e365ac1b6c1d5b197
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Fri Oct 11 16:58:24 2019 -0700

    util: gscoped_ptr<ThreadPool> -> unique_ptr<ThreadPool>
    
    This replaces all usages of gscoped_ptr<ThreadPool> with
    unique_ptr<ThreadPool>.
    
    I also made some trivial gscoped_ptr -> unique_ptr conversions here and
    there, and replaced a couple `ifndef`s with `pragma once`s.
    
    There are no functional changes in this patch.
    
    Change-Id: Ia69a079cf9bf834f4a626b6771442b84c7a37983
    Reviewed-on: http://gerrit.cloudera.org:8080/14419
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Kudu Jenkins
    (cherry picked from commit bd7416aec2f865fe4c3d187801cc775b549cc84d)
    Reviewed-on: http://gerrit.cloudera.org:8080/14450
---
 src/kudu/codegen/compilation_manager.h              |  8 +++-----
 src/kudu/consensus/consensus-test-util.h            |  6 +++---
 src/kudu/consensus/consensus_peers-test.cc          |  2 +-
 src/kudu/consensus/consensus_queue-test.cc          |  4 +++-
 src/kudu/consensus/leader_election-test.cc          |  3 ++-
 src/kudu/consensus/log.cc                           |  2 +-
 src/kudu/consensus/log.h                            |  9 +++------
 src/kudu/consensus/raft_consensus_quorum-test.cc    |  2 +-
 src/kudu/fs/data_dirs.cc                            |  3 +--
 src/kudu/kserver/kserver.h                          |  7 +++----
 src/kudu/master/catalog_manager.h                   |  6 +++---
 src/kudu/master/master.h                            | 13 +++++--------
 src/kudu/rpc/messenger.h                            |  8 +++-----
 src/kudu/tablet/tablet_replica-test.cc              |  6 +++---
 src/kudu/thrift/client.h                            |  3 +--
 src/kudu/tools/ksck.cc                              |  1 -
 src/kudu/tools/ksck.h                               | 10 +++-------
 src/kudu/tools/ksck_checksum.h                      |  3 +--
 src/kudu/tools/table_scanner.h                      |  3 +--
 src/kudu/tserver/tablet_copy_source_session-test.cc |  6 +++---
 src/kudu/tserver/ts_tablet_manager.h                |  8 ++++----
 src/kudu/util/countdown_latch-test.cc               |  5 +++--
 src/kudu/util/curl_util-test.cc                     |  5 +++--
 src/kudu/util/maintenance_manager.h                 |  3 +--
 src/kudu/util/net/dns_resolver.h                    |  3 +--
 src/kudu/util/threadpool-test.cc                    |  5 ++---
 src/kudu/util/threadpool.cc                         |  5 ++---
 src/kudu/util/threadpool.h                          |  9 +++------
 src/kudu/util/trace.h                               |  7 +++----
 29 files changed, 66 insertions(+), 89 deletions(-)

diff --git a/src/kudu/codegen/compilation_manager.h b/src/kudu/codegen/compilation_manager.h
index 77c829a..4cca427 100644
--- a/src/kudu/codegen/compilation_manager.h
+++ b/src/kudu/codegen/compilation_manager.h
@@ -14,11 +14,10 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-
-#ifndef KUDU_CODEGEN_COMPILATION_MANAGER_H
-#define KUDU_CODEGEN_COMPILATION_MANAGER_H
+#pragma once
 
 #include <cstdint>
+#include <memory>
 
 #include "kudu/codegen/code_generator.h"
 #include "kudu/codegen/code_cache.h"
@@ -95,7 +94,7 @@ class CompilationManager {
 
   CodeGenerator generator_;
   CodeCache cache_;
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
 
   AtomicInt<int64_t> hit_counter_;
   AtomicInt<int64_t> query_counter_;
@@ -108,4 +107,3 @@ class CompilationManager {
 } // namespace codegen
 } // namespace kudu
 
-#endif
diff --git a/src/kudu/consensus/consensus-test-util.h b/src/kudu/consensus/consensus-test-util.h
index a0a0e42..cc799c1 100644
--- a/src/kudu/consensus/consensus-test-util.h
+++ b/src/kudu/consensus/consensus-test-util.h
@@ -401,7 +401,7 @@ class NoOpTestPeerProxyFactory : public PeerProxyFactory {
     return messenger_;
   }
  private:
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
   std::shared_ptr<rpc::Messenger> messenger_;
 };
 
@@ -630,7 +630,7 @@ class LocalTestPeerProxyFactory : public PeerProxyFactory {
   }
 
  private:
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
   std::shared_ptr<rpc::Messenger> messenger_;
   TestPeerMapManager* const peers_;
     // NOTE: There is no need to delete this on the dctor because proxies are externally managed
@@ -734,7 +734,7 @@ class TestTransactionFactory : public ConsensusRoundHandler {
   }
 
  private:
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
   RaftConsensus* consensus_;
   log::Log* log_;
 };
diff --git a/src/kudu/consensus/consensus_peers-test.cc b/src/kudu/consensus/consensus_peers-test.cc
index 73246f5..08797c6 100644
--- a/src/kudu/consensus/consensus_peers-test.cc
+++ b/src/kudu/consensus/consensus_peers-test.cc
@@ -165,7 +165,7 @@ class ConsensusPeersTest : public KuduTest {
   scoped_refptr<MetricEntity> metric_entity_;
   gscoped_ptr<FsManager> fs_manager_;
   scoped_refptr<Log> log_;
-  gscoped_ptr<ThreadPool> raft_pool_;
+  unique_ptr<ThreadPool> raft_pool_;
   gscoped_ptr<PeerMessageQueue> message_queue_;
   const Schema schema_;
   LogOptions options_;
diff --git a/src/kudu/consensus/consensus_queue-test.cc b/src/kudu/consensus/consensus_queue-test.cc
index 807681b..0f8d866 100644
--- a/src/kudu/consensus/consensus_queue-test.cc
+++ b/src/kudu/consensus/consensus_queue-test.cc
@@ -17,6 +17,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <memory>
 #include <ostream>
 #include <string>
 #include <vector>
@@ -62,6 +63,7 @@ DECLARE_int32(consensus_max_batch_size_bytes);
 DECLARE_int32(follower_unavailable_considered_failed_sec);
 
 using kudu::consensus::HealthReportPB;
+using std::unique_ptr;
 using std::vector;
 
 namespace kudu {
@@ -228,7 +230,7 @@ class ConsensusQueueTest : public KuduTest {
   MetricRegistry metric_registry_;
   scoped_refptr<MetricEntity> metric_entity_;
   scoped_refptr<log::Log> log_;
-  gscoped_ptr<ThreadPool> raft_pool_;
+  unique_ptr<ThreadPool> raft_pool_;
   gscoped_ptr<PeerMessageQueue> queue_;
   scoped_refptr<log::LogAnchorRegistry> registry_;
   scoped_refptr<clock::Clock> clock_;
diff --git a/src/kudu/consensus/leader_election-test.cc b/src/kudu/consensus/leader_election-test.cc
index 245fb47..94facbc 100644
--- a/src/kudu/consensus/leader_election-test.cc
+++ b/src/kudu/consensus/leader_election-test.cc
@@ -59,6 +59,7 @@ namespace consensus {
 
 using std::shared_ptr;
 using std::string;
+using std::unique_ptr;
 using std::unordered_map;
 using std::vector;
 using strings::Substitute;
@@ -147,7 +148,7 @@ class LeaderElectionTest : public KuduTest {
   RaftConfigPB config_;
   ProxyMap proxies_;
   gscoped_ptr<PeerProxyFactory> proxy_factory_;
-  gscoped_ptr<ThreadPool> pool_;
+  unique_ptr<ThreadPool> pool_;
 
   CountDownLatch latch_;
   gscoped_ptr<ElectionResult> result_;
diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc
index 43b11a1..7a95afa 100644
--- a/src/kudu/consensus/log.cc
+++ b/src/kudu/consensus/log.cc
@@ -258,7 +258,7 @@ class Log::AppendThread {
 
   // Pool with a single thread, which handles shutting down the thread
   // when idle.
-  gscoped_ptr<ThreadPool> append_pool_;
+  unique_ptr<ThreadPool> append_pool_;
 };
 
 
diff --git a/src/kudu/consensus/log.h b/src/kudu/consensus/log.h
index 5abbf3a..34ffe15 100644
--- a/src/kudu/consensus/log.h
+++ b/src/kudu/consensus/log.h
@@ -14,9 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-
-#ifndef KUDU_CONSENSUS_LOG_H_
-#define KUDU_CONSENSUS_LOG_H_
+#pragma once
 
 #include <atomic>
 #include <cstddef>
@@ -350,7 +348,7 @@ class Log : public RefCountedThreadSafe<Log> {
   uint32_t schema_version_;
 
   // The currently active segment being written.
-  gscoped_ptr<WritableLogSegment> active_segment_;
+  std::unique_ptr<WritableLogSegment> active_segment_;
 
   // The current (active) segment sequence number.
   uint64_t active_segment_sequence_number_;
@@ -389,7 +387,7 @@ class Log : public RefCountedThreadSafe<Log> {
   // Thread writing to the log
   gscoped_ptr<AppendThread> append_thread_;
 
-  gscoped_ptr<ThreadPool> allocation_pool_;
+  std::unique_ptr<ThreadPool> allocation_pool_;
 
   // If true, sync on all appends.
   bool force_sync_all_;
@@ -565,4 +563,3 @@ class Log::LogFaultHooks {
 
 }  // namespace log
 }  // namespace kudu
-#endif /* KUDU_CONSENSUS_LOG_H_ */
diff --git a/src/kudu/consensus/raft_consensus_quorum-test.cc b/src/kudu/consensus/raft_consensus_quorum-test.cc
index 75edcab..1b66e43 100644
--- a/src/kudu/consensus/raft_consensus_quorum-test.cc
+++ b/src/kudu/consensus/raft_consensus_quorum-test.cc
@@ -579,7 +579,7 @@ class RaftConsensusQuorumTest : public KuduTest {
   vector<shared_ptr<MemTracker>> parent_mem_trackers_;
   vector<FsManager*> fs_managers_;
   vector<scoped_refptr<Log> > logs_;
-  gscoped_ptr<ThreadPool> raft_pool_;
+  unique_ptr<ThreadPool> raft_pool_;
   vector<scoped_refptr<ConsensusMetadataManager>> cmeta_managers_;
   gscoped_ptr<TestPeerMapManager> peers_;
   vector<TestTransactionFactory*> txn_factories_;
diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc
index 7263c09..c5205ab 100644
--- a/src/kudu/fs/data_dirs.cc
+++ b/src/kudu/fs/data_dirs.cc
@@ -40,7 +40,6 @@
 #include "kudu/fs/block_manager_util.h"
 #include "kudu/fs/fs.pb.h"
 #include "kudu/gutil/bind.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/integral_types.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
@@ -710,7 +709,7 @@ Status DataDirManager::Open() {
     const string data_dir = instance->dir();
 
     // Create a per-dir thread pool.
-    gscoped_ptr<ThreadPool> pool;
+    unique_ptr<ThreadPool> pool;
     RETURN_NOT_OK(ThreadPoolBuilder(Substitute("data dir $0", i))
                   .set_max_threads(1)
                   .set_trace_metric_prefix("data dirs")
diff --git a/src/kudu/kserver/kserver.h b/src/kudu/kserver/kserver.h
index c9fdec1..09903a9 100644
--- a/src/kudu/kserver/kserver.h
+++ b/src/kudu/kserver/kserver.h
@@ -19,7 +19,6 @@
 
 #include <string>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/server/server_base.h"
 #include "kudu/util/threadpool.h"
@@ -63,13 +62,13 @@ class KuduServer : public server::ServerBase {
  private:
 
   // Thread pool for preparing transactions, shared between all tablets.
-  gscoped_ptr<ThreadPool> tablet_prepare_pool_;
+  std::unique_ptr<ThreadPool> tablet_prepare_pool_;
 
   // Thread pool for applying transactions, shared between all tablets.
-  gscoped_ptr<ThreadPool> tablet_apply_pool_;
+  std::unique_ptr<ThreadPool> tablet_apply_pool_;
 
   // Thread pool for Raft-related operations, shared between all tablets.
-  gscoped_ptr<ThreadPool> raft_pool_;
+  std::unique_ptr<ThreadPool> raft_pool_;
 
   DISALLOW_COPY_AND_ASSIGN(KuduServer);
 };
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index 03e423a..f4ce890 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -1085,12 +1085,12 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   // Random number generator used for selecting replica locations.
   ThreadSafeRandom rng_;
 
-  gscoped_ptr<SysCatalogTable> sys_catalog_;
+  std::unique_ptr<SysCatalogTable> sys_catalog_;
 
   // Background thread, used to execute the catalog manager tasks
   // like the assignment and cleaner
   friend class CatalogManagerBgTasks;
-  gscoped_ptr<CatalogManagerBgTasks> background_tasks_;
+  std::unique_ptr<CatalogManagerBgTasks> background_tasks_;
 
   std::unique_ptr<hms::HmsCatalog> hms_catalog_;
   std::unique_ptr<HmsNotificationLogListenerTask> hms_notification_log_listener_;
@@ -1111,7 +1111,7 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   State state_;
 
   // Singleton pool that serializes invocations of ElectedAsLeaderCb().
-  gscoped_ptr<ThreadPool> leader_election_pool_;
+  std::unique_ptr<ThreadPool> leader_election_pool_;
 
   // This field is updated when a node becomes leader master,
   // waits for all outstanding uncommitted metadata (table and tablet metadata)
diff --git a/src/kudu/master/master.h b/src/kudu/master/master.h
index 0b3c66b..b66a31b 100644
--- a/src/kudu/master/master.h
+++ b/src/kudu/master/master.h
@@ -14,8 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_MASTER_MASTER_H
-#define KUDU_MASTER_MASTER_H
+#pragma once
 
 #include <atomic>
 #include <cstdint>
@@ -23,7 +22,6 @@
 #include <vector>
 
 #include "kudu/common/wire_protocol.pb.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/kserver/kserver.h"
@@ -133,11 +131,11 @@ class Master : public kserver::KuduServer {
 
   std::unique_ptr<MasterCertAuthority> cert_authority_;
   std::unique_ptr<security::TokenSigner> token_signer_;
-  gscoped_ptr<CatalogManager> catalog_manager_;
-  gscoped_ptr<MasterPathHandlers> path_handlers_;
+  std::unique_ptr<CatalogManager> catalog_manager_;
+  std::unique_ptr<MasterPathHandlers> path_handlers_;
 
   // For initializing the catalog manager.
-  gscoped_ptr<ThreadPool> init_pool_;
+  std::unique_ptr<ThreadPool> init_pool_;
 
   // The status of the master initialization. This is set
   // by the async initialization task.
@@ -155,11 +153,10 @@ class Master : public kserver::KuduServer {
   // A simplistic cache to track already assigned locations.
   std::unique_ptr<LocationCache> location_cache_;
 
-  gscoped_ptr<TSManager> ts_manager_;
+  std::unique_ptr<TSManager> ts_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(Master);
 };
 
 } // namespace master
 } // namespace kudu
-#endif
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index 56b087c..5dfa104 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -14,8 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_RPC_MESSENGER_H
-#define KUDU_RPC_MESSENGER_H
+#pragma once
 
 #include <cstdint>
 #include <memory>
@@ -379,8 +378,8 @@ class Messenger {
 
   // Separate client and server negotiation pools to avoid possibility of distributed
   // deadlock. See KUDU-2041.
-  gscoped_ptr<ThreadPool> client_negotiation_pool_;
-  gscoped_ptr<ThreadPool> server_negotiation_pool_;
+  std::unique_ptr<ThreadPool> client_negotiation_pool_;
+  std::unique_ptr<ThreadPool> server_negotiation_pool_;
 
   std::unique_ptr<security::TlsContext> tls_context_;
 
@@ -458,4 +457,3 @@ class Messenger {
 } // namespace rpc
 } // namespace kudu
 
-#endif
diff --git a/src/kudu/tablet/tablet_replica-test.cc b/src/kudu/tablet/tablet_replica-test.cc
index 376fb89..e41239e 100644
--- a/src/kudu/tablet/tablet_replica-test.cc
+++ b/src/kudu/tablet/tablet_replica-test.cc
@@ -388,9 +388,9 @@ class TabletReplicaTest : public KuduTabletTest {
   MetricRegistry metric_registry_;
   scoped_refptr<MetricEntity> metric_entity_;
   shared_ptr<Messenger> messenger_;
-  gscoped_ptr<ThreadPool> prepare_pool_;
-  gscoped_ptr<ThreadPool> apply_pool_;
-  gscoped_ptr<ThreadPool> raft_pool_;
+  unique_ptr<ThreadPool> prepare_pool_;
+  unique_ptr<ThreadPool> apply_pool_;
+  unique_ptr<ThreadPool> raft_pool_;
   unique_ptr<DnsResolver> dns_resolver_;
 
   scoped_refptr<ConsensusMetadataManager> cmeta_manager_;
diff --git a/src/kudu/thrift/client.h b/src/kudu/thrift/client.h
index ff948cb..2e684e7 100644
--- a/src/kudu/thrift/client.h
+++ b/src/kudu/thrift/client.h
@@ -28,7 +28,6 @@
 
 #include <glog/logging.h>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -123,7 +122,7 @@ class HaClient {
   Status Reconnect();
 
   // Background thread which executes calls to the Thrift service.
-  gscoped_ptr<ThreadPool> threadpool_;
+  std::unique_ptr<ThreadPool> threadpool_;
 
   // Client options.
   std::vector<HostPort> addresses_;
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index c66d9bd..6c39b7f 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -34,7 +34,6 @@
 #include <glog/logging.h>
 
 #include "kudu/consensus/quorum_util.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/join.h"
diff --git a/src/kudu/tools/ksck.h b/src/kudu/tools/ksck.h
index 4412b6e..71aa2b7 100644
--- a/src/kudu/tools/ksck.h
+++ b/src/kudu/tools/ksck.h
@@ -16,9 +16,7 @@
 // under the License.
 //
 // Ksck, a tool to run a Kudu System Check.
-
-#ifndef KUDU_TOOLS_KSCK_H
-#define KUDU_TOOLS_KSCK_H
+#pragma once
 
 #include <atomic>
 #include <cstdint>
@@ -36,7 +34,6 @@
 
 #include "kudu/common/schema.h"
 #include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/rebalance/cluster_status.h" // IWYU pragma: keep
@@ -482,7 +479,7 @@ class KsckCluster {
   MasterList masters_;
   TSMap tablet_servers_;
   std::vector<std::shared_ptr<KsckTable>> tables_;
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
 
   std::vector<std::string> table_filters_;
   std::vector<std::string> tablet_id_filters_;
@@ -614,7 +611,7 @@ class Ksck {
       int table_num_replicas);
 
   const std::shared_ptr<KsckCluster> cluster_;
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
 
   bool check_replica_count_ = true;
   std::vector<std::string> table_filters_;
@@ -633,4 +630,3 @@ class Ksck {
 } // namespace tools
 } // namespace kudu
 
-#endif // KUDU_TOOLS_KSCK_H
diff --git a/src/kudu/tools/ksck_checksum.h b/src/kudu/tools/ksck_checksum.h
index a09dba7..47b0947 100644
--- a/src/kudu/tools/ksck_checksum.h
+++ b/src/kudu/tools/ksck_checksum.h
@@ -28,7 +28,6 @@
 #include <vector>
 
 #include "kudu/common/schema.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/tools/ksck_results.h"
 #include "kudu/util/countdown_latch.h"
@@ -259,7 +258,7 @@ class KsckChecksumManager : public std::enable_shared_from_this<KsckChecksumMana
 
   // A threadpool for running tasks that find additional tablets that can
   // be checksummed based on available slots on tablet servers.
-  gscoped_ptr<ThreadPool> find_tablets_to_checksum_pool_;
+  std::unique_ptr<ThreadPool> find_tablets_to_checksum_pool_;
 
   std::atomic<int64_t> rows_summed_;
   std::atomic<int64_t> disk_bytes_summed_;
diff --git a/src/kudu/tools/table_scanner.h b/src/kudu/tools/table_scanner.h
index 17c75d2..b784835 100644
--- a/src/kudu/tools/table_scanner.h
+++ b/src/kudu/tools/table_scanner.h
@@ -29,7 +29,6 @@
 #include "kudu/client/client.h"
 #include "kudu/client/scan_batch.h"
 #include "kudu/client/shared_ptr.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/util/atomic.h"
 #include "kudu/util/mutex.h"
 #include "kudu/util/status.h"
@@ -97,7 +96,7 @@ class TableScanner {
   std::string table_name_;
   boost::optional<client::sp::shared_ptr<kudu::client::KuduClient>> dst_client_;
   boost::optional<std::string> dst_table_name_;
-  gscoped_ptr<ThreadPool> thread_pool_;
+  std::unique_ptr<ThreadPool> thread_pool_;
 
   // Protects output to 'out_' so that rows don't get interleaved.
   Mutex output_lock_;
diff --git a/src/kudu/tserver/tablet_copy_source_session-test.cc b/src/kudu/tserver/tablet_copy_source_session-test.cc
index fa18b0a..5bc912b 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -258,9 +258,9 @@ class TabletCopyTest : public KuduTabletTest {
 
   MetricRegistry metric_registry_;
   scoped_refptr<LogAnchorRegistry> log_anchor_registry_;
-  gscoped_ptr<ThreadPool> prepare_pool_;
-  gscoped_ptr<ThreadPool> apply_pool_;
-  gscoped_ptr<ThreadPool> raft_pool_;
+  unique_ptr<ThreadPool> prepare_pool_;
+  unique_ptr<ThreadPool> apply_pool_;
+  unique_ptr<ThreadPool> raft_pool_;
   unique_ptr<DnsResolver> dns_resolver_;
   scoped_refptr<TabletReplica> tablet_replica_;
   scoped_refptr<TabletCopySourceSession> session_;
diff --git a/src/kudu/tserver/ts_tablet_manager.h b/src/kudu/tserver/ts_tablet_manager.h
index 880fc86..863c1b7 100644
--- a/src/kudu/tserver/ts_tablet_manager.h
+++ b/src/kudu/tserver/ts_tablet_manager.h
@@ -20,6 +20,7 @@
 #include <cstdint>
 #include <functional>
 #include <map>
+#include <memory>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ -28,7 +29,6 @@
 #include <gtest/gtest_prod.h>
 
 #include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/tablet/metadata.pb.h"
@@ -393,13 +393,13 @@ class TSTabletManager : public tserver::TabletReplicaLookupIf {
   TSTabletManagerStatePB state_;
 
   // Thread pool used to run tablet copy operations.
-  gscoped_ptr<ThreadPool> tablet_copy_pool_;
+  std::unique_ptr<ThreadPool> tablet_copy_pool_;
 
   // Thread pool used to open the tablets async, whether bootstrap is required or not.
-  gscoped_ptr<ThreadPool> open_tablet_pool_;
+  std::unique_ptr<ThreadPool> open_tablet_pool_;
 
   // Thread pool used to delete tablets asynchronously.
-  gscoped_ptr<ThreadPool> delete_tablet_pool_;
+  std::unique_ptr<ThreadPool> delete_tablet_pool_;
 
   FunctionGaugeDetacher metric_detacher_;
 
diff --git a/src/kudu/util/countdown_latch-test.cc b/src/kudu/util/countdown_latch-test.cc
index adb2623..32a673f 100644
--- a/src/kudu/util/countdown_latch-test.cc
+++ b/src/kudu/util/countdown_latch-test.cc
@@ -15,10 +15,11 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include <memory>
+
 #include <boost/bind.hpp> // IWYU pragma: keep
 #include <gtest/gtest.h>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/util/countdown_latch.h"
 #include "kudu/util/monotime.h"
@@ -40,7 +41,7 @@ static void DecrementLatch(CountDownLatch* latch, int amount) {
 // as 1 by one.
 TEST(TestCountDownLatch, TestLatch) {
 
-  gscoped_ptr<ThreadPool> pool;
+  std::unique_ptr<ThreadPool> pool;
   ASSERT_OK(ThreadPoolBuilder("cdl-test").set_max_threads(1).Build(&pool));
 
   CountDownLatch latch(1000);
diff --git a/src/kudu/util/curl_util-test.cc b/src/kudu/util/curl_util-test.cc
index f43b6c7..7f750e7 100644
--- a/src/kudu/util/curl_util-test.cc
+++ b/src/kudu/util/curl_util-test.cc
@@ -17,9 +17,10 @@
 
 #include "kudu/util/curl_util.h"
 
+#include <memory>
+
 #include <gtest/gtest.h>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/util/debug/sanitizer_scopes.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/monotime.h"
@@ -41,7 +42,7 @@ TEST(CurlUtilTest, TestTimeout) {
 
 TEST(CurlUtilTest, NonSharedObjectsBetweenThreads) {
   const int kThreadCount = 8;
-  gscoped_ptr<ThreadPool> pool;
+  std::unique_ptr<ThreadPool> pool;
   ThreadPoolBuilder("curl-util-test")
       .set_min_threads(kThreadCount)
       .set_max_threads(kThreadCount)
diff --git a/src/kudu/util/maintenance_manager.h b/src/kudu/util/maintenance_manager.h
index 95a7987..631d0d6 100644
--- a/src/kudu/util/maintenance_manager.h
+++ b/src/kudu/util/maintenance_manager.h
@@ -30,7 +30,6 @@
 #include <glog/logging.h>
 #include <gtest/gtest_prod.h>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/util/atomic.h"
@@ -343,7 +342,7 @@ class MaintenanceManager : public std::enable_shared_from_this<MaintenanceManage
   OpMapTy ops_; // Registered operations.
   Mutex lock_;
   scoped_refptr<kudu::Thread> monitor_thread_;
-  gscoped_ptr<ThreadPool> thread_pool_;
+  std::unique_ptr<ThreadPool> thread_pool_;
   ConditionVariable cond_;
   bool shutdown_;
   int32_t polling_interval_ms_;
diff --git a/src/kudu/util/net/dns_resolver.h b/src/kudu/util/net/dns_resolver.h
index 8526385..b07384a 100644
--- a/src/kudu/util/net/dns_resolver.h
+++ b/src/kudu/util/net/dns_resolver.h
@@ -23,7 +23,6 @@
 #include <string>
 #include <vector>
 
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/sockaddr.h"
@@ -97,7 +96,7 @@ class DnsResolver {
   bool GetCachedAddresses(const HostPort& hostport,
                           std::vector<Sockaddr>* addresses);
 
-  gscoped_ptr<ThreadPool> pool_;
+  std::unique_ptr<ThreadPool> pool_;
   std::unique_ptr<HostRecordCache> cache_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsResolver);
diff --git a/src/kudu/util/threadpool-test.cc b/src/kudu/util/threadpool-test.cc
index 23fc45c..f440cda 100644
--- a/src/kudu/util/threadpool-test.cc
+++ b/src/kudu/util/threadpool-test.cc
@@ -38,7 +38,6 @@
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/bind.h"
 #include "kudu/gutil/bind_helpers.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -92,7 +91,7 @@ class ThreadPoolTest : public KuduTest {
   }
 
  protected:
-  gscoped_ptr<ThreadPool> pool_;
+  unique_ptr<ThreadPool> pool_;
 };
 
 TEST_F(ThreadPoolTest, TestNoTaskOpenClose) {
@@ -346,7 +345,7 @@ TEST_F(ThreadPoolTest, TestZeroQueueSize) {
 // other tasks on the same pool.
 TEST_F(ThreadPoolTest, TestSlowThreadStart) {
   // Start a pool of threads from which we'll submit tasks.
-  gscoped_ptr<ThreadPool> submitter_pool;
+  unique_ptr<ThreadPool> submitter_pool;
   ASSERT_OK(ThreadPoolBuilder("submitter")
             .set_min_threads(5)
             .set_max_threads(5)
diff --git a/src/kudu/util/threadpool.cc b/src/kudu/util/threadpool.cc
index 23dda3d..e983c7c 100644
--- a/src/kudu/util/threadpool.cc
+++ b/src/kudu/util/threadpool.cc
@@ -123,10 +123,9 @@ ThreadPoolBuilder& ThreadPoolBuilder::set_metrics(ThreadPoolMetrics metrics) {
   return *this;
 }
 
-Status ThreadPoolBuilder::Build(gscoped_ptr<ThreadPool>* pool) const {
+Status ThreadPoolBuilder::Build(unique_ptr<ThreadPool>* pool) const {
   pool->reset(new ThreadPool(*this));
-  RETURN_NOT_OK((*pool)->Init());
-  return Status::OK();
+  return (*pool)->Init();
 }
 
 ////////////////////////////////////////////////////////
diff --git a/src/kudu/util/threadpool.h b/src/kudu/util/threadpool.h
index 1557486..789b132 100644
--- a/src/kudu/util/threadpool.h
+++ b/src/kudu/util/threadpool.h
@@ -14,8 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_UTIL_THREAD_POOL_H
-#define KUDU_UTIL_THREAD_POOL_H
+#pragma once
 
 #include <deque>
 #include <iosfwd>
@@ -28,7 +27,6 @@
 #include <gtest/gtest_prod.h>
 
 #include "kudu/gutil/callback.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
@@ -128,7 +126,7 @@ class ThreadPoolBuilder {
   ThreadPoolBuilder& set_metrics(ThreadPoolMetrics metrics);
 
   // Instantiate a new ThreadPool with the existing builder arguments.
-  Status Build(gscoped_ptr<ThreadPool>* pool) const;
+  Status Build(std::unique_ptr<ThreadPool>* pool) const;
 
  private:
   friend class ThreadPool;
@@ -168,7 +166,7 @@ class ThreadPoolBuilder {
 //    static void Func(int n) { ... }
 //    class Task : public Runnable { ... }
 //
-//    gscoped_ptr<ThreadPool> thread_pool;
+//    unique_ptr<ThreadPool> thread_pool;
 //    CHECK_OK(
 //        ThreadPoolBuilder("my_pool")
 //            .set_min_threads(0)
@@ -502,4 +500,3 @@ class ThreadPoolToken {
 };
 
 } // namespace kudu
-#endif
diff --git a/src/kudu/util/trace.h b/src/kudu/util/trace.h
index fb79150..61e3b13 100644
--- a/src/kudu/util/trace.h
+++ b/src/kudu/util/trace.h
@@ -14,10 +14,10 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-#ifndef KUDU_UTIL_TRACE_H
-#define KUDU_UTIL_TRACE_H
+#pragma once
 
 #include <iosfwd>
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -224,7 +224,7 @@ class Trace : public RefCountedThreadSafe<Trace> {
 
   void MetricsToJSON(JsonWriter* jw) const;
 
-  gscoped_ptr<ThreadSafeArena> arena_;
+  std::unique_ptr<ThreadSafeArena> arena_;
 
   // Lock protecting the entries linked list.
   mutable simple_spinlock lock_;
@@ -289,4 +289,3 @@ class ScopedTraceLatencyCounter {
 };
 
 } // namespace kudu
-#endif /* KUDU_UTIL_TRACE_H */