You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/05/29 23:44:27 UTC
[2/5] kudu git commit: KUDU-2444
TabletServerTest.TestTombstonedTabletOnWebUI is flaky
KUDU-2444 TabletServerTest.TestTombstonedTabletOnWebUI is flaky
It's possible for a tablet to be RUNNING with working consensus
but still have its "creating" transition live in the tablet
manager. This makes deleting a tablet without retry immediately
after it is created flaky. To fix this for tests using an
in-process tablet server, I added a method to the TSTabletManager
that waits for there to be no tablet transitions in progress.
This method is intended for use only in tests, and is used by the
tablet server test fixture to ensure its single tablet has transitioned
out of the creating state before the actual test runs.
In addition to the original report from tablet_server-test.cc,
I also saw a test in tablet_copy_client_session-itest.cc fail
because of the same issue. I changed it to retry the delete, as
this tests uses an external minicluster.
I ran the affected tests 1000 times with 8 stress threads in
debug mode in dist-test. All passed.
Change-Id: Ib8dc7acc4c39f9d9b067a52df4bd6732f545a8de
Reviewed-on: http://gerrit.cloudera.org:8080/10443
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/98ef3c03
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/98ef3c03
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/98ef3c03
Branch: refs/heads/master
Commit: 98ef3c03b2c07cbbde69c039a0d04ad208358fc0
Parents: e04fd4e
Author: Will Berkeley <wd...@apache.org>
Authored: Thu May 17 13:00:48 2018 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Fri May 25 21:36:31 2018 +0000
----------------------------------------------------------------------
.../tablet_copy_client_session-itest.cc | 9 +++++++--
src/kudu/tserver/tablet_server-test-base.cc | 11 ++++++++---
src/kudu/tserver/tablet_server-test.cc | 11 +++++++----
src/kudu/tserver/ts_tablet_manager.cc | 15 +++++++++++++++
src/kudu/tserver/ts_tablet_manager.h | 5 +++++
5 files changed, 42 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/98ef3c03/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_copy_client_session-itest.cc b/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
index 78a0e77..bacf419 100644
--- a/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
+++ b/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
@@ -269,8 +269,13 @@ TEST_F(TabletCopyClientSessionITest, TestCopyFromCrashedSource) {
// Attempt the copy again. This time it should succeed.
ASSERT_OK(WaitUntilTabletRunning(ts0, tablet_id, kDefaultTimeout));
- ASSERT_OK(StartTabletCopy(ts1, tablet_id, ts0->uuid(), src_addr,
- std::numeric_limits<int64_t>::max(), kDefaultTimeout));
+ // KUDU-2444: Even though the copy failed and the tablet replica is
+ // tombstoned, the tablet manager may not have transitioned the replica out
+ // of the 'copying' phase yet, so we retry here.
+ ASSERT_EVENTUALLY([&]{
+ ASSERT_OK(StartTabletCopy(ts1, tablet_id, ts0->uuid(), src_addr,
+ std::numeric_limits<int64_t>::max(), kDefaultTimeout));
+ });
ASSERT_OK(WaitUntilTabletRunning(ts1, tablet_id, kDefaultTimeout));
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/98ef3c03/src/kudu/tserver/tablet_server-test-base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_server-test-base.cc b/src/kudu/tserver/tablet_server-test-base.cc
index 1168601..8aaa0dd 100644
--- a/src/kudu/tserver/tablet_server-test-base.cc
+++ b/src/kudu/tserver/tablet_server-test-base.cc
@@ -132,10 +132,15 @@ void TabletServerTestBase::StartTabletServer(int num_data_dirs) {
Status TabletServerTestBase::WaitForTabletRunning(const char *tablet_id) {
scoped_refptr<tablet::TabletReplica> tablet_replica;
- RETURN_NOT_OK(mini_server_->server()->tablet_manager()->GetTabletReplica(tablet_id,
- &tablet_replica));
+ const auto* tablet_manager = mini_server_->server()->tablet_manager();
+ RETURN_NOT_OK(tablet_manager->GetTabletReplica(tablet_id, &tablet_replica));
RETURN_NOT_OK(tablet_replica->WaitUntilConsensusRunning(MonoDelta::FromSeconds(10)));
- return tablet_replica->consensus()->WaitUntilLeaderForTests(MonoDelta::FromSeconds(10));
+ RETURN_NOT_OK(
+ tablet_replica->consensus()->WaitUntilLeaderForTests(MonoDelta::FromSeconds(10)));
+
+ // KUDU-2444: Even though the tablet replica is fully running, the tablet
+ // manager may regard it as still transitioning to the running state.
+ return tablet_manager->WaitForNoTransitionsForTests(MonoDelta::FromSeconds(10));
}
void TabletServerTestBase::UpdateTestRowRemote(int64_t row_idx,
http://git-wip-us.apache.org/repos/asf/kudu/blob/98ef3c03/src/kudu/tserver/tablet_server-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_server-test.cc b/src/kudu/tserver/tablet_server-test.cc
index 436c80f..cc9de59 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-#include "kudu/tserver/tablet_server-test-base.h"
+#include "kudu/tserver/tablet_server.h"
#include <stdlib.h>
#include <unistd.h>
@@ -80,7 +80,7 @@
#include "kudu/tserver/heartbeater.h"
#include "kudu/tserver/mini_tablet_server.h"
#include "kudu/tserver/scanners.h"
-#include "kudu/tserver/tablet_server.h"
+#include "kudu/tserver/tablet_server-test-base.h"
#include "kudu/tserver/tablet_server_options.h"
#include "kudu/tserver/tablet_server_test_util.h"
#include "kudu/tserver/ts_tablet_manager.h"
@@ -536,8 +536,11 @@ TEST_F(TabletServerTest, TestFailedTabletsOnWebUI) {
TEST_F(TabletServerTest, TestTombstonedTabletOnWebUI) {
TSTabletManager* tablet_manager = mini_server_->server()->tablet_manager();
TabletServerErrorPB::Code error_code;
- ASSERT_OK(tablet_manager->DeleteTablet(kTabletId,
- tablet::TABLET_DATA_TOMBSTONED, boost::none, &error_code));
+ ASSERT_OK(
+ tablet_manager->DeleteTablet(kTabletId,
+ tablet::TABLET_DATA_TOMBSTONED,
+ boost::none,
+ &error_code));
// Restart the server. We drop the tablet_replica_ reference since it becomes
// invalid when the server shuts down.
http://git-wip-us.apache.org/repos/asf/kudu/blob/98ef3c03/src/kudu/tserver/ts_tablet_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index 59fb0d6..d55b0bb 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -1493,6 +1493,21 @@ int TSTabletManager::RefreshTabletStateCacheAndReturnCount(tablet::TabletStatePB
return FindWithDefault(tablet_state_counts_, st, 0);
}
+Status TSTabletManager::WaitForNoTransitionsForTests(const MonoDelta& timeout) const {
+ const MonoTime start = MonoTime::Now();
+ while (MonoTime::Now() - start < timeout) {
+ {
+ shared_lock<RWMutex> lock(lock_);
+ if (transition_in_progress_.empty()) {
+ return Status::OK();
+ }
+ }
+ SleepFor(MonoDelta::FromMilliseconds(50));
+ }
+ return Status::TimedOut("transitions still in progress after waiting $0",
+ timeout.ToString());
+}
+
TransitionInProgressDeleter::TransitionInProgressDeleter(
TransitionInProgressMap* map, RWMutex* lock, string entry)
: in_progress_(map), lock_(lock), entry_(std::move(entry)) {}
http://git-wip-us.apache.org/repos/asf/kudu/blob/98ef3c03/src/kudu/tserver/ts_tablet_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.h b/src/kudu/tserver/ts_tablet_manager.h
index 742b5a4..30e1b9c 100644
--- a/src/kudu/tserver/ts_tablet_manager.h
+++ b/src/kudu/tserver/ts_tablet_manager.h
@@ -218,6 +218,11 @@ class TSTabletManager : public tserver::TabletReplicaLookupIf {
// and return the count for tablet state 'st'.
int RefreshTabletStateCacheAndReturnCount(tablet::TabletStatePB st);
+ // Wait a period up to 'timeout' for there to be no tablet state transitions
+ // registered with the tablet manager.
+ // This method is for use in tests only. See KUDU-2444.
+ Status WaitForNoTransitionsForTests(const MonoDelta& timeout) const;
+
private:
FRIEND_TEST(TsTabletManagerTest, TestPersistBlocks);