kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: raft_consensus-itest: fix failing TestMemoryRemainsConstantDespiteTwoDeadFollowers
Date Fri, 30 Jun 2017 04:46:54 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 332ebcd06 -> 3548895a2

raft_consensus-itest: fix failing TestMemoryRemainsConstantDespiteTwoDeadFollowers

This test started failing recently with a seemingly unrelated change to
the minicluster (e6758739a90adeb2e4d0c6cf76185cf90cc7d2b0). It's still a
mystery as to the relationship here, after a couple hours of debugging.

But, looking at the failures with some extra tracing added, I determined
that the issue was something like the following:

- the test sets up a scenario in which writes are expected to time out
  by killing 2/3 replicas
- due to recent client changes by Alexey, the client now considers a
  Timeout error on writes to mean that the server is dead, and marks it
  as no longer a leader in the cache
- this causes the next write on other threads to go to the master to
  lookup the "new location" assuming that a leader election probably has
- in fact, _all_ of the threads quickly go to the master to perform the
  same lookup, exhausing the "master lookup permits" counter. Any excess
  threads beyond that count end up backing off sleeping
- the master lookups in TSAN builds can sometimes take tens of
  milliseconds, especially in the GCE test environment which uses low
  core-count workers. Thus, some of the lookups to the masters
  themselves time out.
- those that do succeed, of course, return the same location information
  as we had before
- any writers which happen to wake up from back-offs to get new location
  information then try to do a write, but it's quick likely that before
  they get a chance to do so, another thread has already experienced
  another timeout and marked the replica as bad.

Essentially, the test can get into a kind of spiral where it's flooding
the master with lookups, each of which is taking longer than 50ms, and
thus cause more timeouts, which cause more lookups, etc.

As identified elsewhere, the metacache probably needs a pretty major
re-working to fix these sorts of problems, but this scenario is also
fairly contrived. So, this patch just bumps the timeout to 150ms instead
of 50ms, and changes the payload of the writes to be much larger, so the
desired backpressure kicks in faster.

Before this change, TSAN test runs on gce were failing nearly 100%. With
the change I passed 100/100.

Change-Id: Ifa7d3d7655c2ecf376e894b8a1412e2fe3df0753
Reviewed-on: http://gerrit.cloudera.org:8080/7337
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>

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

Branch: refs/heads/master
Commit: 3548895a27ee65982acd4bd1d6ca1bf33bf39288
Parents: 332ebcd
Author: Todd Lipcon <todd@apache.org>
Authored: Thu Jun 29 18:11:23 2017 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Fri Jun 30 04:46:06 2017 +0000

 src/kudu/integration-tests/raft_consensus-itest.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 51680a5..204f44e 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -2484,7 +2484,8 @@ TEST_F(RaftConsensusITest, TestMemoryRemainsConstantDespiteTwoDeadFollowers)
   TestWorkload workload(cluster_.get());
-  workload.set_write_timeout_millis(50);
+  workload.set_write_timeout_millis(150);
+  workload.set_payload_bytes(30000);

View raw message