Mailing-List: contact commits-help@kudu.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kudu.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: mpercy@apache.org
To: commits@kudu.apache.org
Date: Tue, 30 Aug 2016 06:25:34 -0000
Message-Id: <6488ce8883554d82970ebf51f7e8d962@git.apache.org>
Subject: [1/3] kudu git commit: raft_consensus-itest: inserter thread should
 FATAL instead of FAIL
archived-at: Tue, 30 Aug 2016 06:25:36 -0000

Repository: kudu
Updated Branches:
  refs/heads/master ed216bcdf -> 0d18a0b02


raft_consensus-itest: inserter thread should FATAL instead of FAIL

This test has a thread which inserts rows and is supposed to fail the
test case if it sees any row errors. Failing the test using FAIL() from
a non-main thread is not thread-safe in gtest. Furthermore, FAIL() acts
as a 'return' and thus the latch used to communicate the thread
completion never gets fired. So, when this assertion failed, the test
would hang forever.

This was a regression caused by d0cff255f84e75b70c0c39ccd34a35f348e3c722
which changed the code from a CHECK(...) to a FAIL().

In order to correct this behavior, this patch switches to using the
utility method FlushSessionOrDie() which has an identical
implementation.

Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b
Reviewed-on: http://gerrit.cloudera.org:8080/4140
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Reviewed-by: Mike Percy <mpercy@apache.org>


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

Branch: refs/heads/master
Commit: f78d1c840a703d0f36c2bcf299b87d44f2a9fb0e
Parents: ed216bc
Author: Todd Lipcon <todd@apache.org>
Authored: Fri Aug 26 17:02:54 2016 -0700
Committer: Mike Percy <mpercy@apache.org>
Committed: Tue Aug 30 06:24:53 2016 +0000

----------------------------------------------------------------------
 .../integration-tests/raft_consensus-itest.cc   | 22 +-------------------
 1 file changed, 1 insertion(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/f78d1c84/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 8d6553d..9fc74ef 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -226,29 +226,9 @@ class RaftConsensusITest : public TabletServerIntegrationTestBase {
         CHECK_OK(session->Apply(insert.release()));
       }
 
-      // We don't handle write idempotency yet. (i.e making sure that when a leader fails
-      // writes to it that were eventually committed by the new leader but un-ackd to the
-      // client are not retried), so some errors are expected.
-      // It's OK as long as the errors are Status::AlreadyPresent();
+      FlushSessionOrDie(session);
 
       int inserted = last_row_in_batch - first_row_in_batch;
-
-      Status s = session->Flush();
-      if (PREDICT_FALSE(!s.ok())) {
-        std::vector<client::KuduError*> errors;
-        ElementDeleter d(&errors);
-        bool overflow;
-        session->GetPendingErrors(&errors, &overflow);
-        CHECK(!overflow);
-        if (!errors.empty()) {
-          for (const client::KuduError* e : errors) {
-            LOG(ERROR) << "Unexpected error: " << e->status().ToString();
-          }
-          FAIL() << "Found errors while inserting.";
-        }
-        inserted -= errors.size();
-      }
-
       for (CountDownLatch* latch : latches) {
         latch->CountDown(inserted);
       }