kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject incubator-kudu git commit: Fix some TSAN failures seen on GCE
Date Sat, 16 Jan 2016 02:44:37 GMT
Repository: incubator-kudu
Updated Branches:
  refs/heads/master 79130fc0a -> 56f97d557


Fix some TSAN failures seen on GCE

This fixes a few TSAN issues that I've seen while testing on GCE:

- Add suppression for epoll_ctl

libev is missing TSAN annotations and thus its synchronization doesn't
get registered as "happens-before" relationships. This can cause spurious
TSAN errors relating to epoll file descriptors. Before this suppression,
I saw 8/100 runs of DeleteTableTest TSAN fail with such a race.

- Make churny election test a little less churny on TSAN

The TestChurnyElections test sets timeouts to 1ms. On TSAN on GCE instances
this is often failing because it doesn't make any progress at all. This
patch sets the election timeout to 5ms instead of 1ms and also disables
fsync, which is a bit slower on GCE due to storage being remote.

- Increase timeout waiting for WALs in some integration tests

This fixes some flakiness in delete_table-test TSAN builds where writes
were going slowly enough that we didn't reach the desired number of WALs
within 30 seconds. After doubling to 60 seconds, I didn't see failures.

- Bump test workload write timeout to 50ms in memory limit test

With a 10ms timeout, the client was often unable to even connect to the
server to start any transactions in TSAN builds. Bumping to 50ms makes
the test much more reliable.

- Add TSAN annotations around iterator stats

We assume that iterator stats can be fuzzy and don't want to add barriers
on the write side. This patch adds an IGNORE annotation around reading
the stats.

Change-Id: Idb7447b9b88c37a45be3840d5b24edafc237ad51
Reviewed-on: http://gerrit.cloudera.org:8080/1793
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Internal Jenkins


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

Branch: refs/heads/master
Commit: 56f97d5577c2d7f7c6b9715aff250a0ca8d4fb1e
Parents: 79130fc
Author: Todd Lipcon <todd@apache.org>
Authored: Thu Jan 14 17:21:55 2016 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Sat Jan 16 02:44:19 2016 +0000

----------------------------------------------------------------------
 build-support/tsan-suppressions.txt                         | 1 +
 .../integration-tests/external_mini_cluster_fs_inspector.h  | 2 +-
 src/kudu/integration-tests/raft_consensus-itest.cc          | 9 ++++++++-
 src/kudu/tablet/cfile_set.cc                                | 3 +++
 4 files changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/56f97d55/build-support/tsan-suppressions.txt
----------------------------------------------------------------------
diff --git a/build-support/tsan-suppressions.txt b/build-support/tsan-suppressions.txt
index a833cd9..a39c9a8 100644
--- a/build-support/tsan-suppressions.txt
+++ b/build-support/tsan-suppressions.txt
@@ -47,6 +47,7 @@ race:_ULx86_64_init
 # for examples.
 race:evpipe_write
 race:evpipe_init
+race:epoll_ctl
 
 # concurrent btree uses optimistic concurrency, needs to be annotated a bunch
 # more before it would pass. Relatively confident that it is correct based on

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/56f97d55/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
index 1ccfc5f..2bf3023 100644
--- a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
+++ b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
@@ -73,7 +73,7 @@ class ExternalMiniClusterFsInspector {
   Status WaitForMinFilesInTabletWalDirOnTS(int index,
                                            const std::string& tablet_id,
                                            int count,
-                                           const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+                                           const MonoDelta& timeout = MonoDelta::FromSeconds(60));
   Status WaitForReplicaCount(int expected, const MonoDelta& timeout = MonoDelta::FromSeconds(30));
   Status WaitForTabletDataStateOnTS(int index,
                                     const std::string& tablet_id,

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/56f97d55/src/kudu/integration-tests/raft_consensus-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 8496ddb..b7ed3b3 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -913,9 +913,16 @@ TEST_F(RaftConsensusITest, DISABLED_TestChurnyElections_WithNotificationLatency)
 void RaftConsensusITest::DoTestChurnyElections(bool with_latency) {
   vector<string> ts_flags, master_flags;
 
+#ifdef THREAD_SANITIZER
+  // On TSAN builds, we need to be a little bit less churny in order to make
+  // any progress at all.
+  ts_flags.push_back("--raft_heartbeat_interval_ms=5");
+#else
   ts_flags.push_back("--raft_heartbeat_interval_ms=1");
+#endif
   ts_flags.push_back("--leader_failure_monitor_check_mean_ms=1");
   ts_flags.push_back("--leader_failure_monitor_check_stddev_ms=1");
+  ts_flags.push_back("--never_fsync");
   if (with_latency) {
     ts_flags.push_back("--consensus_inject_latency_ms_in_notifications=50");
   }
@@ -2221,7 +2228,7 @@ TEST_F(RaftConsensusITest, TestMemoryRemainsConstantDespiteTwoDeadFollowers)
{
   TestWorkload workload(cluster_.get());
   workload.set_table_name(kTableId);
   workload.set_timeout_allowed(true);
-  workload.set_write_timeout_millis(10);
+  workload.set_write_timeout_millis(50);
   workload.Setup();
   workload.Start();
 

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/56f97d55/src/kudu/tablet/cfile_set.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/cfile_set.cc b/src/kudu/tablet/cfile_set.cc
index 121dd70..91dffc0 100644
--- a/src/kudu/tablet/cfile_set.cc
+++ b/src/kudu/tablet/cfile_set.cc
@@ -24,6 +24,7 @@
 #include "kudu/cfile/cfile_writer.h"
 #include "kudu/common/scan_spec.h"
 #include "kudu/gutil/algorithm.h"
+#include "kudu/gutil/dynamic_annotations.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -476,7 +477,9 @@ void CFileSet::Iterator::GetIteratorStats(vector<IteratorStats>*
stats) const {
   stats->clear();
   stats->reserve(col_iters_.size());
   for (const ColumnIterator* iter : col_iters_) {
+    ANNOTATE_IGNORE_READS_BEGIN();
     stats->push_back(iter->io_statistics());
+    ANNOTATE_IGNORE_READS_END();
   }
 }
 


Mime
View raw message