You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/12/09 00:19:46 UTC

[1/3] kudu git commit: log-rolling-itest: wait for log roller on server startup

Repository: kudu
Updated Branches:
  refs/heads/master 18a8a90f7 -> 4bd36c46c


log-rolling-itest: wait for log roller on server startup

The log roller runs early in the startup sequence, but we were still
hitting cases where the test outran the roller.

Change-Id: Iefbf678163c0f4ba07eedfd762177c5ace5e35e6
Reviewed-on: http://gerrit.cloudera.org:8080/5431
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: 0f6562723a4d69f45a78f0ed2859abbc5f447d06
Parents: 18a8a90
Author: Dan Burkert <da...@apache.org>
Authored: Thu Dec 8 15:07:32 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Dec 9 00:03:10 2016 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/log-rolling-itest.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/0f656272/src/kudu/integration-tests/log-rolling-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log-rolling-itest.cc b/src/kudu/integration-tests/log-rolling-itest.cc
index bd21150..899ca28 100644
--- a/src/kudu/integration-tests/log-rolling-itest.cc
+++ b/src/kudu/integration-tests/log-rolling-itest.cc
@@ -26,6 +26,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/external_mini_cluster.h"
 #include "kudu/util/env.h"
+#include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
 using std::string;
@@ -61,7 +62,9 @@ TEST(LogRollingITest, TestLogCleanupOnStartup) {
   ASSERT_OK(cluster.master()->WaitForCatalogManager());
 
   for (int i = 1; i <= 10; i++) {
-    ASSERT_EQ(std::min(3, i), CountInfoLogs(*cluster.master()->log_dir()));
+    AssertEventually([&] () {
+        ASSERT_EQ(std::min(3, i), CountInfoLogs(*cluster.master()->log_dir()));
+    });
     cluster.master()->Shutdown();
     ASSERT_OK(cluster.master()->Restart());
   }


[3/3] kudu git commit: Remove the clock from MvccManager

Posted by to...@apache.org.
Remove the clock from MvccManager

Now that safe time has it's own managing entity, there is no
need for MvccManager to have a reference to the clock since
it's no longer supposed to update it or wait on it.

Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Reviewed-on: http://gerrit.cloudera.org:8080/5326
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: 4bd36c46cdd1ff9c337baadbb05edd2a5f87b94e
Parents: a69e390
Author: David Alves <dr...@apache.org>
Authored: Fri Dec 2 04:02:11 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Dec 9 00:13:29 2016 +0000

----------------------------------------------------------------------
 src/kudu/tablet/compaction-test.cc             |  1 -
 src/kudu/tablet/deltamemstore-test.cc          |  3 +-
 src/kudu/tablet/diskrowset-test-base.h         |  3 +-
 src/kudu/tablet/major_delta_compaction-test.cc |  4 +-
 src/kudu/tablet/memrowset-test.cc              |  3 +-
 src/kudu/tablet/mvcc-test.cc                   | 61 +++++++++++++++------
 src/kudu/tablet/mvcc.cc                        | 12 ++--
 src/kudu/tablet/mvcc.h                         |  5 +-
 src/kudu/tablet/tablet.cc                      |  1 -
 9 files changed, 56 insertions(+), 37 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/compaction-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/compaction-test.cc b/src/kudu/tablet/compaction-test.cc
index 8f64b49..dca1d80 100644
--- a/src/kudu/tablet/compaction-test.cc
+++ b/src/kudu/tablet/compaction-test.cc
@@ -72,7 +72,6 @@ class TestCompaction : public KuduRowSetTest {
       row_builder_(schema_),
       arena_(32*1024, 128*1024),
       clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)),
-      mvcc_(clock_),
       log_anchor_registry_(new log::LogAnchorRegistry()) {
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/deltamemstore-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/deltamemstore-test.cc b/src/kudu/tablet/deltamemstore-test.cc
index 1ecfe5c..60712f7 100644
--- a/src/kudu/tablet/deltamemstore-test.cc
+++ b/src/kudu/tablet/deltamemstore-test.cc
@@ -49,8 +49,7 @@ class TestDeltaMemStore : public KuduTest {
   TestDeltaMemStore()
     : op_id_(consensus::MaximumOpId()),
       schema_(CreateSchema()),
-      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)),
-      mvcc_(clock_) {
+      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)) {
     CHECK_OK(DeltaMemStore::Create(0, 0,
                                    new log::LogAnchorRegistry(),
                                    MemTracker::GetRootTracker(), &dms_));

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/diskrowset-test-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/diskrowset-test-base.h b/src/kudu/tablet/diskrowset-test-base.h
index dae3bdb..6d2b7eb 100644
--- a/src/kudu/tablet/diskrowset-test-base.h
+++ b/src/kudu/tablet/diskrowset-test-base.h
@@ -61,8 +61,7 @@ class TestRowSet : public KuduRowSetTest {
     : KuduRowSetTest(CreateTestSchema()),
       n_rows_(FLAGS_roundtrip_num_rows),
       op_id_(consensus::MaximumOpId()),
-      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)),
-      mvcc_(clock_) {
+      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)) {
     CHECK_GT(n_rows_, 0);
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/major_delta_compaction-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/major_delta_compaction-test.cc b/src/kudu/tablet/major_delta_compaction-test.cc
index a1f5a84..e5e3b15 100644
--- a/src/kudu/tablet/major_delta_compaction-test.cc
+++ b/src/kudu/tablet/major_delta_compaction-test.cc
@@ -48,9 +48,7 @@ class TestMajorDeltaCompaction : public KuduRowSetTest {
                               ColumnSchema("val1", INT32),
                               ColumnSchema("val2", STRING),
                               ColumnSchema("val3", INT32),
-                              ColumnSchema("val4", STRING) }, 1)),
-      mvcc_(scoped_refptr<server::Clock>(
-          server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp))) {
+                              ColumnSchema("val4", STRING) }, 1)) {
   }
 
   struct ExpectedRow {

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/memrowset-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/memrowset-test.cc b/src/kudu/tablet/memrowset-test.cc
index 4aad650..a5fd09c 100644
--- a/src/kudu/tablet/memrowset-test.cc
+++ b/src/kudu/tablet/memrowset-test.cc
@@ -49,8 +49,7 @@ class TestMemRowSet : public ::testing::Test {
       log_anchor_registry_(new LogAnchorRegistry()),
       schema_(CreateSchema()),
       key_schema_(schema_.CreateKeyProjection()),
-      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)),
-      mvcc_(clock_) {
+      clock_(server::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)) {
     FLAGS_enable_data_block_fsync = false; // Keep unit tests fast.
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/mvcc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/mvcc-test.cc b/src/kudu/tablet/mvcc-test.cc
index 543d806..21ccc4a 100644
--- a/src/kudu/tablet/mvcc-test.cc
+++ b/src/kudu/tablet/mvcc-test.cc
@@ -62,7 +62,7 @@ class MvccTest : public KuduTest {
 };
 
 TEST_F(MvccTest, TestMvccBasic) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   MvccSnapshot snap;
 
   // Initial state should not have any committed transactions.
@@ -99,7 +99,7 @@ TEST_F(MvccTest, TestMvccBasic) {
 }
 
 TEST_F(MvccTest, TestMvccMultipleInFlight) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   MvccSnapshot snap;
 
   Timestamp t1 = clock_->Now();
@@ -173,7 +173,7 @@ TEST_F(MvccTest, TestMvccMultipleInFlight) {
 TEST_F(MvccTest, TestOutOfOrderTxns) {
   scoped_refptr<Clock> hybrid_clock(new HybridClock());
   ASSERT_OK(hybrid_clock->Init());
-  MvccManager mgr(hybrid_clock);
+  MvccManager mgr;
 
   // Start a normal non-commit-wait txn.
   Timestamp normal_txn = hybrid_clock->Now();
@@ -215,7 +215,7 @@ TEST_F(MvccTest, TestOutOfOrderTxns) {
 // Tests starting transaction at a point-in-time in the past and committing them while
 // adjusting safe time.
 TEST_F(MvccTest, TestSafeTimeWithOutOfOrderTxns) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   // Set the clock to some time in the "future".
   ASSERT_OK(clock_->Update(Timestamp(100)));
@@ -251,7 +251,7 @@ TEST_F(MvccTest, TestSafeTimeWithOutOfOrderTxns) {
 }
 
 TEST_F(MvccTest, TestScopedTransaction) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   MvccSnapshot snap;
 
   {
@@ -369,7 +369,7 @@ TEST_F(MvccTest, TestMayHaveUncommittedTransactionsBefore) {
 }
 
 TEST_F(MvccTest, TestAreAllTransactionsCommitted) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   // start several transactions and take snapshots along the way
   Timestamp tx1 = clock_->Now();
@@ -409,7 +409,7 @@ TEST_F(MvccTest, TestAreAllTransactionsCommitted) {
 }
 
 TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapWithNoInflights) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   Timestamp to_wait_for = clock_->Now();
   mgr.AdjustSafeTime(clock_->Now());
   thread waiting_thread = thread(&MvccTest::WaitForSnapshotAtTSThread, this, &mgr, to_wait_for);
@@ -420,8 +420,7 @@ TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapWithNoInflights) {
 }
 
 TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapBeforeSafeTimeWithInFlights) {
-
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   Timestamp tx1 = clock_->Now();
   mgr.StartTransaction(tx1);
@@ -448,7 +447,7 @@ TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapBeforeSafeTimeWithInFlights) {
 }
 
 TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapAfterSafeTimeWithInFlights) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   Timestamp tx1 = clock_->Now();
   mgr.StartTransaction(tx1);
   Timestamp tx2 = clock_->Now();
@@ -478,8 +477,7 @@ TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapAfterSafeTimeWithInFlights) {
 }
 
 TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapAtTimestampWithInFlights) {
-
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   // Transactions with timestamp 1 through 3
   Timestamp tx1 = clock_->Now();
@@ -516,11 +514,42 @@ TEST_F(MvccTest, TestWaitForCleanSnapshot_SnapAtTimestampWithInFlights) {
   ASSERT_TRUE(HasResultSnapshot());
 }
 
+TEST_F(MvccTest, TestWaitForApplyingTransactionsToCommit) {
+  MvccManager mgr;
+
+  Timestamp tx1 = clock_->Now();
+  mgr.StartTransaction(tx1);
+  Timestamp tx2 = clock_->Now();
+  mgr.StartTransaction(tx2);
+  mgr.AdjustSafeTime(tx2);
+
+  // Wait should return immediately, since we have no transactions "applying"
+  // yet.
+  mgr.WaitForApplyingTransactionsToCommit();
+
+  mgr.StartApplyingTransaction(tx1);
+
+  thread waiting_thread = thread(&MvccManager::WaitForApplyingTransactionsToCommit, &mgr);
+  while (mgr.GetNumWaitersForTests() == 0) {
+    SleepFor(MonoDelta::FromMilliseconds(5));
+  }
+  ASSERT_EQ(mgr.GetNumWaitersForTests(), 1);
+
+  // Aborting the other transaction shouldn't affect our waiter.
+  mgr.AbortTransaction(tx2);
+  ASSERT_EQ(mgr.GetNumWaitersForTests(), 1);
+
+  // Committing our transaction should wake the waiter.
+  mgr.CommitTransaction(tx1);
+  ASSERT_EQ(mgr.GetNumWaitersForTests(), 0);
+  waiting_thread.join();
+}
+
 // Test that if we abort a transaction we don't advance the safe time and don't
 // add the transaction to the committed set.
 TEST_F(MvccTest, TestTxnAbort) {
 
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   // Transactions with timestamps 1 through 3
   Timestamp tx1 = clock_->Now();
@@ -555,7 +584,7 @@ TEST_F(MvccTest, TestTxnAbort) {
 // coalesce to the latest timestamp.
 TEST_F(MvccTest, TestAutomaticCleanTimeMoveToSafeTimeOnCommit) {
 
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   clock_->Update(Timestamp(20));
 
   mgr.StartTransaction(Timestamp(10));
@@ -578,7 +607,7 @@ TEST_F(MvccTest, TestAutomaticCleanTimeMoveToSafeTimeOnCommit) {
 //
 // Any other transition should fire a CHECK failure.
 TEST_F(MvccTest, TestIllegalStateTransitionsCrash) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
   MvccSnapshot snap;
 
   EXPECT_DEATH({
@@ -637,7 +666,7 @@ TEST_F(MvccTest, TestIllegalStateTransitionsCrash) {
 }
 
 TEST_F(MvccTest, TestWaitUntilCleanDeadline) {
-  MvccManager mgr(clock_.get());
+  MvccManager mgr;
 
   // Transactions with timestamp 1
   Timestamp tx1 = clock_->Now();

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/mvcc.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/mvcc.cc b/src/kudu/tablet/mvcc.cc
index bfa8a79..e0c1e32 100644
--- a/src/kudu/tablet/mvcc.cc
+++ b/src/kudu/tablet/mvcc.cc
@@ -36,10 +36,9 @@ namespace kudu { namespace tablet {
 
 using strings::Substitute;
 
-MvccManager::MvccManager(const scoped_refptr<server::Clock>& clock)
+MvccManager::MvccManager()
   : safe_time_(Timestamp::kMin),
-    earliest_in_flight_(Timestamp::kMax),
-    clock_(clock) {
+    earliest_in_flight_(Timestamp::kMax) {
   cur_snap_.all_committed_before_ = Timestamp::kInitialTimestamp;
   cur_snap_.none_committed_at_or_after_ = Timestamp::kInitialTimestamp;
 }
@@ -52,7 +51,8 @@ void MvccManager::StartTransaction(Timestamp timestamp) {
   CHECK(InitTransactionUnlocked(timestamp)) << "There is already a transaction with timestamp: "
                                             << timestamp.value() << " in flight or this timestamp "
                                             << "is before than or equal to \"safe\" time."
-                                            << "Current Snapshot: " << cur_snap_.ToString();
+                                            << "Current Snapshot: " << cur_snap_.ToString()
+                                            << " Current safe time: " << safe_time_;
 }
 
 void MvccManager::StartApplyingTransaction(Timestamp timestamp) {
@@ -130,10 +130,6 @@ MvccManager::TxnState MvccManager::RemoveInFlightAndGetStateUnlocked(Timestamp t
 
 void MvccManager::CommitTransactionUnlocked(Timestamp timestamp,
                                             bool* was_earliest_in_flight) {
-  DCHECK(clock_->IsAfter(timestamp))
-    << "Trying to commit a transaction with a future timestamp: "
-    << timestamp.ToString() << ". Current time: " << clock_->Stringify(clock_->Now());
-
   *was_earliest_in_flight = earliest_in_flight_ == timestamp;
 
   // Remove from our in-flight list.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/mvcc.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/mvcc.h b/src/kudu/tablet/mvcc.h
index bd106a8..948a4ce 100644
--- a/src/kudu/tablet/mvcc.h
+++ b/src/kudu/tablet/mvcc.h
@@ -178,7 +178,7 @@ class MvccSnapshot {
 // this class like "clean" and "safe" time.
 class MvccManager {
  public:
-  explicit MvccManager(const scoped_refptr<server::Clock>& clock);
+  MvccManager();
 
   // Begins a new transaction, which is assigned the provided timestamp.
   //
@@ -282,6 +282,8 @@ class MvccManager {
 
   bool InitTransactionUnlocked(const Timestamp& timestamp);
 
+  // TODO(dralves) ponder merging these since the new ALL_COMMITTED path no longer
+  // waits for the clean timestamp.
   enum WaitFor {
     ALL_COMMITTED,
     NONE_APPLYING
@@ -355,7 +357,6 @@ class MvccManager {
   // over timestamps_in_flight_ on every commit.
   Timestamp earliest_in_flight_;
 
-  scoped_refptr<server::Clock> clock_;
   mutable std::vector<WaitingState*> waiters_;
 
   DISALLOW_COPY_AND_ASSIGN(MvccManager);

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bd36c46/src/kudu/tablet/tablet.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet.cc b/src/kudu/tablet/tablet.cc
index aa5d39f..f45d075 100644
--- a/src/kudu/tablet/tablet.cc
+++ b/src/kudu/tablet/tablet.cc
@@ -165,7 +165,6 @@ Tablet::Tablet(const scoped_refptr<TabletMetadata>& metadata,
     mem_trackers_(tablet_id(), parent_mem_tracker),
     next_mrs_id_(0),
     clock_(clock),
-    mvcc_(clock),
     rowsets_flush_sem_(1),
     state_(kInitialized) {
       CHECK(schema()->has_column_ids());


[2/3] kudu git commit: KUDU-1753 [delete_table-test] deleted-while-in-scan test

Posted by to...@apache.org.
KUDU-1753 [delete_table-test] deleted-while-in-scan test

Added an integration test to ensure a tablet server keeps all the
necessary data around until a scan operation ends even if tablet
is being deleted concurrently.

The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.

This is in the context of the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Reviewed-on: http://gerrit.cloudera.org:8080/5345
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>


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

Branch: refs/heads/master
Commit: a69e390a6370dd7118c918fe366b27038971e991
Parents: 0f65627
Author: Alexey Serbin <as...@cloudera.com>
Authored: Fri Dec 2 19:08:44 2016 -0800
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Fri Dec 9 00:03:47 2016 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/delete_table-test.cc | 164 +++++++++++++++++--
 .../external_mini_cluster-itest-base.cc         |  49 +++---
 .../external_mini_cluster-itest-base.h          |   1 +
 3 files changed, 182 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a69e390a/src/kudu/integration-tests/delete_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/delete_table-test.cc b/src/kudu/integration-tests/delete_table-test.cc
index 9b8589f..34f3556 100644
--- a/src/kudu/integration-tests/delete_table-test.cc
+++ b/src/kudu/integration-tests/delete_table-test.cc
@@ -15,14 +15,16 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <boost/optional.hpp>
-#include <glog/stl_logging.h>
-#include <gtest/gtest.h>
 #include <memory>
 #include <string>
 #include <unordered_map>
 
+#include <boost/optional.hpp>
+#include <glog/stl_logging.h>
+#include <gtest/gtest.h>
+
 #include "kudu/client/client-test-util.h"
+#include "kudu/client/shared_ptr.h"
 #include "kudu/common/wire_protocol-test-util.h"
 #include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/split.h"
@@ -41,9 +43,11 @@
 #include "kudu/util/subprocess.h"
 
 using kudu::client::KuduClient;
-using kudu::client::KuduClientBuilder;
+using kudu::client::KuduScanner;
+using kudu::client::KuduScanBatch;
 using kudu::client::KuduSchema;
 using kudu::client::KuduSchemaFromSchema;
+using kudu::client::KuduTable;
 using kudu::client::KuduTableCreator;
 using kudu::consensus::CONSENSUS_CONFIG_COMMITTED;
 using kudu::consensus::ConsensusMetadataPB;
@@ -83,6 +87,11 @@ class DeleteTableTest : public ExternalMiniClusterITestBase {
     SUPERBLOCK_EXPECTED = 1
   };
 
+  enum ErrorDumpStackSelector {
+    ON_ERROR_DO_NOT_DUMP_STACKS = 0,
+    ON_ERROR_DUMP_STACKS = 1,
+  };
+
   // Get the UUID of the leader of the specified tablet, as seen by the TS with
   // the given 'ts_uuid'.
   string GetLeaderUUID(const string& ts_uuid, const string& tablet_id);
@@ -114,9 +123,10 @@ class DeleteTableTest : public ExternalMiniClusterITestBase {
   void WaitForAllTSToCrash();
   void WaitUntilTabletRunning(int index, const std::string& tablet_id);
 
-  // Delete the given table. If the operation times out, dumps the master stacks
-  // to help debug master-side deadlocks.
-  void DeleteTable(const string& table_name);
+  // Delete the given table. If the operation times out, optionally dump
+  // the master stacks to help debug master-side deadlocks.
+  void DeleteTable(const string& table_name,
+                   ErrorDumpStackSelector selector = ON_ERROR_DUMP_STACKS);
 };
 
 string DeleteTableTest::GetLeaderUUID(const string& ts_uuid, const string& tablet_id) {
@@ -208,11 +218,12 @@ void DeleteTableTest::WaitUntilTabletRunning(int index, const std::string& table
                                           tablet_id, MonoDelta::FromSeconds(60)));
 }
 
-void DeleteTableTest::DeleteTable(const string& table_name) {
+void DeleteTableTest::DeleteTable(const string& table_name,
+                                  ErrorDumpStackSelector selector) {
   Status s = client_->DeleteTable(table_name);
-  if (s.IsTimedOut()) {
+  if (s.IsTimedOut() && (ON_ERROR_DUMP_STACKS == selector)) {
     WARN_NOT_OK(PstackWatcher::DumpPidStacks(cluster_->master()->pid()),
-                        "Couldn't dump stacks");
+                "Couldn't dump stacks");
   }
   ASSERT_OK(s);
 }
@@ -1263,4 +1274,137 @@ const char* tombstoned_faults[] = {"fault_crash_after_blocks_deleted",
 INSTANTIATE_TEST_CASE_P(FaultFlags, DeleteTableTombstonedParamTest,
                         ::testing::ValuesIn(tombstoned_faults));
 
+// Make sure the tablet server keeps the necessary data to serve scan request in
+// progress if tablet is marked for deletion.
+TEST_F(DeleteTableTest, DISABLED_TestDeleteTableWhileScanInProgress) {
+  const KuduScanner::ReadMode read_modes[] = {
+    KuduScanner::READ_LATEST,
+    KuduScanner::READ_AT_SNAPSHOT,
+  };
+  const auto read_mode_to_string = [](KuduScanner::ReadMode mode) {
+    switch (mode) {
+      case KuduScanner::READ_LATEST:
+        return "READ_LATEST";
+      case KuduScanner::READ_AT_SNAPSHOT:
+        return "READ_AT_SNAPSHOT";
+      default:
+        return "UNKNOWN";
+    }
+  };
+
+  const KuduClient::ReplicaSelection replica_selectors[] = {
+    KuduClient::LEADER_ONLY,
+    KuduClient::CLOSEST_REPLICA,
+    KuduClient::FIRST_REPLICA,
+  };
+  const auto replica_sel_to_string = [](KuduClient::ReplicaSelection sel) {
+    switch (sel) {
+      case KuduClient::LEADER_ONLY:
+        return "LEADER_ONLY";
+      case KuduClient::CLOSEST_REPLICA:
+        return "CLOSEST_REPLICA";
+      case KuduClient::FIRST_REPLICA:
+        return "FIRST_REPLICA";
+      default:
+        return "UNKNOWN";
+    }
+  };
+
+  const std::vector<std::string> extra_ts_flags = {
+    // Set the flush threshold low so that we have a mix of flushed and
+    // unflushed operations in the WAL, when we bootstrap.
+    "--flush_threshold_mb=1",
+
+    // Set the compaction budget to be low so that we get multiple passes
+    // of compaction instead of selecting all of the rowsets in a single
+    // compaction of the whole tablet.
+    "--tablet_compaction_budget_mb=1",
+
+    // Set the major delta compaction ratio low enough that we trigger
+    // a lot of them.
+    "--tablet_delta_store_major_compact_min_ratio=0.001",
+  };
+
+  // Approximate number of rows to insert. This is not exact number due to the
+  // way how the test controls the progress of the test workload.
+  const size_t rows_to_insert = AllowSlowTests() ? 100000 : 10000;
+  for (const auto sel : replica_selectors) {
+    for (const auto mode : read_modes) {
+      SCOPED_TRACE(Substitute("mode $0; replica $1",
+                              read_mode_to_string(mode),
+                              replica_sel_to_string(sel)));
+      NO_FATALS(StartCluster(extra_ts_flags));
+
+      TestWorkload w(cluster_.get());
+      w.set_write_pattern(TestWorkload::INSERT_RANDOM_ROWS);
+      w.Setup();
+
+      // Start the workload, and wait to see some rows actually inserted.
+      w.Start();
+      while (w.rows_inserted() < rows_to_insert) {
+        SleepFor(MonoDelta::FromMilliseconds(50));
+      }
+      w.StopAndJoin();
+      const int64_t ref_row_count = w.rows_inserted();
+
+      using kudu::client::sp::shared_ptr;
+      shared_ptr<KuduTable> table;
+      ASSERT_OK(client_->OpenTable(w.table_name(), &table));
+      KuduScanner scanner(table.get());
+      ASSERT_OK(scanner.SetReadMode(mode));
+      ASSERT_OK(scanner.SetSelection(sel));
+      // Setup batch size to be small enough to guarantee the scan
+      // will not fetch all the data at once.
+      ASSERT_OK(scanner.SetBatchSizeBytes(1));
+      ASSERT_OK(scanner.Open());
+      ASSERT_TRUE(scanner.HasMoreRows());
+      KuduScanBatch batch;
+      ASSERT_OK(scanner.NextBatch(&batch));
+      size_t row_count = batch.NumRows();
+
+      // Once the first batch of data has been fetched and there is some more
+      // to fetch, delete the table.
+      NO_FATALS(DeleteTable(w.table_name(), ON_ERROR_DO_NOT_DUMP_STACKS));
+
+      // Wait while the table is no longer advertised on the cluster.
+      // This ensures the table deletion request has been processed by tablet
+      // servers.
+      vector<string> tablets;
+      do {
+        SleepFor(MonoDelta::FromMilliseconds(250));
+        tablets = inspect_->ListTablets();
+      } while (!tablets.empty());
+
+      // Make sure the scanner can continue and fetch the rest of rows.
+      ASSERT_TRUE(scanner.HasMoreRows());
+      while (scanner.HasMoreRows()) {
+        KuduScanBatch batch;
+        const Status s = scanner.NextBatch(&batch);
+        ASSERT_TRUE(s.ok()) << s.ToString();
+        row_count += batch.NumRows();
+      }
+
+      // Verify the total row count. The exact count must be there in case of
+      // READ_AT_SNAPSHOT mode regardless of replica selection or if reading
+      // from a leader tablet in any scan mode. In the case of the READ_LATEST
+      // mode the data might be fetched from a lagging replica and the scan
+      // row count might be less than the inserted row count.
+      if (mode == KuduScanner::READ_AT_SNAPSHOT ||
+          sel == KuduClient::LEADER_ONLY) {
+        EXPECT_EQ(ref_row_count, row_count);
+      }
+
+      // Close the scanner to make sure it does not hold any references on the
+      // data about to be deleted by the hosting tablet server.
+      scanner.Close();
+
+      // Make sure the table has been deleted.
+      EXPECT_OK(inspect_->WaitForNoData());
+      NO_FATALS(cluster_->AssertNoCrashes());
+      // Tear down the cluster -- prepare for the next iteration of the loop.
+      NO_FATALS(StopCluster());
+    }
+  }
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/a69e390a/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-itest-base.cc b/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
index 134d590..657d4df 100644
--- a/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
+++ b/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
@@ -33,28 +33,7 @@
 namespace kudu {
 
 void ExternalMiniClusterITestBase::TearDown() {
-  if (!cluster_) {
-    return;
-  }
-
-  if (HasFatalFailure()) {
-    LOG(INFO) << "Found fatal failure";
-    for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-      if (!cluster_->tablet_server(i)->IsProcessAlive()) {
-        LOG(INFO) << "Tablet server " << i
-                  << " is not running. Cannot dump its stacks.";
-        continue;
-      }
-      LOG(INFO) << "Attempting to dump stacks of TS " << i
-                << " with UUID " << cluster_->tablet_server(i)->uuid()
-                << " and pid " << cluster_->tablet_server(i)->pid();
-      WARN_NOT_OK(PstackWatcher::DumpPidStacks(cluster_->tablet_server(i)->pid()),
-                  "Couldn't dump stacks");
-    }
-  }
-  cluster_->Shutdown();
-  STLDeleteValues(&ts_map_);
-
+  StopCluster();
   KuduTest::TearDown();
 }
 
@@ -77,4 +56,30 @@ void ExternalMiniClusterITestBase::StartCluster(
   ASSERT_OK(cluster_->CreateClient(nullptr, &client_));
 }
 
+void ExternalMiniClusterITestBase::StopCluster() {
+  if (!cluster_) {
+    return;
+  }
+
+  if (HasFatalFailure()) {
+    LOG(INFO) << "Found fatal failure";
+    for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+      if (!cluster_->tablet_server(i)->IsProcessAlive()) {
+        LOG(INFO) << "Tablet server " << i << " is not running. Cannot dump its stacks.";
+        continue;
+      }
+      LOG(INFO) << "Attempting to dump stacks of TS " << i
+                << " with UUID " << cluster_->tablet_server(i)->uuid()
+                << " and pid " << cluster_->tablet_server(i)->pid();
+      WARN_NOT_OK(PstackWatcher::DumpPidStacks(cluster_->tablet_server(i)->pid()),
+                  "Couldn't dump stacks");
+    }
+  }
+  cluster_->Shutdown();
+  client_.reset();
+  inspect_.reset();
+  cluster_.reset();
+  STLDeleteValues(&ts_map_);
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/a69e390a/src/kudu/integration-tests/external_mini_cluster-itest-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-itest-base.h b/src/kudu/integration-tests/external_mini_cluster-itest-base.h
index c76c931..c6885f6 100644
--- a/src/kudu/integration-tests/external_mini_cluster-itest-base.h
+++ b/src/kudu/integration-tests/external_mini_cluster-itest-base.h
@@ -47,6 +47,7 @@ class ExternalMiniClusterITestBase : public KuduTest {
   void StartCluster(const std::vector<std::string>& extra_ts_flags = {},
                     const std::vector<std::string>& extra_master_flags = {},
                     int num_tablet_servers = 3);
+  void StopCluster();
 
   std::unique_ptr<ExternalMiniCluster> cluster_;
   std::unique_ptr<itest::ExternalMiniClusterFsInspector> inspect_;