[2/5] git commit: ZooKeeperMasterContenderProcess::contend now checks if the future for the previous contend() is still pending and if so it directly returns that future without recontending.

[vi...@apache.org]

ZooKeeperMasterContenderProcess::contend now checks if the future for
the previous contend() is still pending and if so it directly returns
that future without recontending.

From: Jiang Yan Xu <ya...@jxu.me>
Review: https://reviews.apache.org/r/16589


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

Branch: refs/heads/master
Commit: b2516db5595dbe2bbb8b0d32972c670143191d08
Parents: e03700a
Author: Vinod Kone <vi...@twitter.com>
Authored: Wed Jan 15 12:51:17 2014 -0800
Committer: Vinod Kone <vi...@twitter.com>
Committed: Wed Jan 15 12:54:57 2014 -0800

----------------------------------------------------------------------
 src/master/contender.cpp                      |  9 +++-
 src/master/contender.hpp                      |  8 ++--
 src/tests/master_contender_detector_tests.cpp | 52 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b2516db5/src/master/contender.cpp
----------------------------------------------------------------------
diff --git a/src/master/contender.cpp b/src/master/contender.cpp
index 7bd25b5..89e368b 100644
--- a/src/master/contender.cpp
+++ b/src/master/contender.cpp
@@ -65,6 +65,7 @@ private:
 
   // The master this contender contends on behalf of.
   Option<PID<Master> > master;
+  Option<Future<Future<Nothing> > > candidacy;
 };
 
 
@@ -203,13 +204,19 @@ Future<Future<Nothing> > ZooKeeperMasterContenderProcess::contend()
     return Failure("Initialize the contender first");
   }
 
+  // Should not recontend if the last election is still ongoing.
+  if (candidacy.isSome() && candidacy.get().isPending()) {
+    return candidacy.get();
+  }
+
   if (contender != NULL) {
     LOG(INFO) << "Withdrawing the previous membership before recontending";
     delete contender;
   }
 
   contender = new LeaderContender(group.get(), master.get());
-  return contender->contend();
+  candidacy = contender->contend();
+  return candidacy.get();
 }
 
 } // namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/b2516db5/src/master/contender.hpp
----------------------------------------------------------------------
diff --git a/src/master/contender.hpp b/src/master/contender.hpp
index 0156369..2a7e7c4 100644
--- a/src/master/contender.hpp
+++ b/src/master/contender.hpp
@@ -74,9 +74,11 @@ public:
   // The inner Future returns Nothing when the contender is out of
   // the contest (i.e. its membership is lost).
   //
-  // This method can be used to contend again. Each call to this
-  // method causes the previous candidacy to be withdrawn before
-  // re-contending.
+  // This method can be used to contend again after candidacy is
+  // obtained (the outer future satisfied), otherwise the future for
+  // the pending election is returned.
+  // Recontending after candidacy is obtained causes the previous
+  // candidacy to be withdrawn.
   virtual process::Future<process::Future<Nothing> > contend() = 0;
 };
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b2516db5/src/tests/master_contender_detector_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_contender_detector_tests.cpp b/src/tests/master_contender_detector_tests.cpp
index 951d720..9cd576f 100644
--- a/src/tests/master_contender_detector_tests.cpp
+++ b/src/tests/master_contender_detector_tests.cpp
@@ -213,6 +213,58 @@ TEST_F(ZooKeeperMasterContenderDetectorTest, MasterContender)
 }
 
 
+// Verifies that contender does not recontend if the current election
+// is still pending.
+TEST_F(ZooKeeperMasterContenderDetectorTest, ContenderPendingElection)
+{
+  Try<zookeeper::URL> url = zookeeper::URL::parse(
+      "zk://" + server->connectString() + "/mesos");
+
+  ASSERT_SOME(url);
+
+  ZooKeeperMasterContender contender(url.get());
+
+  PID<Master> master;
+  master.ip = 10000000;
+  master.port = 10000;
+
+  contender.initialize(master);
+
+  // Drop Group::join so that 'contended' will stay pending.
+  Future<Nothing> join = DROP_DISPATCH(_, &GroupProcess::join);
+
+  Future<Future<Nothing> > contended = contender.contend();
+  AWAIT_READY(join);
+
+  Clock::pause();
+
+  // Make sure GroupProcess::join is dispatched (and dropped).
+  Clock::settle();
+
+  EXPECT_TRUE(contended.isPending());
+
+  process::filter(NULL);
+
+  process::TestsFilter* filter =
+    process::FilterTestEventListener::instance()->install();
+  pthread_mutex_lock(&filter->mutex);
+
+  // Expect GroupProcess::join not getting called because
+  // ZooKeeperMasterContender directly returns.
+  EXPECT_CALL(filter->mock, filter(testing::A<const process::DispatchEvent&>()))
+    .With(DispatchMatcher(_, &GroupProcess::join))
+    .Times(0);
+  pthread_mutex_lock(&filter->mutex);
+
+  // Recontend and settle so that if ZooKeeperMasterContender is not
+  // directly returning, GroupProcess::join is dispatched.
+  contender.contend();
+  Clock::settle();
+
+  Clock::resume();
+}
+
+
 // Two contenders, the first wins. Kill the first, then the second
 // is elected.
 TEST_F(ZooKeeperMasterContenderDetectorTest, MasterContenders)