Mailing-List: contact mesos-commits-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mesos-dev@incubator.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1418146 - in /incubator/mesos/trunk/src:
 detector/detector.cpp
 tests/zookeeper_tests.cpp
Date: Fri, 07 Dec 2012 00:45:42 -0000
To: mesos-commits@incubator.apache.org
From: benh@apache.org
Message-Id: <20121207004542.98BD423888E4@eris.apache.org>

Author: benh
Date: Fri Dec  7 00:45:41 2012
New Revision: 1418146

URL: http://svn.apache.org/viewvc?rev=1418146&view=rev
Log:
Added more ZooKeeper master detector tests.

From: Vinod Kone <vinodkone@gmail.com>
Review: https://reviews.apache.org/r/7842

Modified:
    incubator/mesos/trunk/src/detector/detector.cpp
    incubator/mesos/trunk/src/tests/zookeeper_tests.cpp

Modified: incubator/mesos/trunk/src/detector/detector.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/detector/detector.cpp?rev=1418146&r1=1418145&r2=1418146&view=diff
==============================================================================
--- incubator/mesos/trunk/src/detector/detector.cpp (original)
+++ incubator/mesos/trunk/src/detector/detector.cpp Fri Dec  7 00:45:41 2012
@@ -281,7 +281,7 @@ int64_t ZooKeeperMasterDetectorProcess::
 void ZooKeeperMasterDetectorProcess::connected(bool reconnect)
 {
   if (!reconnect) {
-    LOG(INFO) << "Master detector connected to ZooKeeper ...";
+    LOG(INFO) << "Master detector (" << pid << ") connected to ZooKeeper ...";
 
     if (url.authentication.isSome()) {
       const std::string& scheme = url.authentication.get().scheme;
@@ -334,7 +334,7 @@ void ZooKeeperMasterDetectorProcess::con
     // Now determine who the master is (it may be us).
     detectMaster();
   } else {
-    LOG(INFO) << "Master detector reconnected ...";
+    LOG(INFO) << "Master detector (" << pid << ")  reconnected ...";
 
     // Cancel and cleanup the reconnect timer (if necessary).
     if (timer.isSome()) {
@@ -372,7 +372,7 @@ void ZooKeeperMasterDetectorProcess::con
 
 void ZooKeeperMasterDetectorProcess::reconnecting()
 {
-  LOG(INFO) << "Master detector lost connection to ZooKeeper, "
+  LOG(INFO) << "Master detector (" << pid << ")  lost connection to ZooKeeper, "
             << "attempting to reconnect ...";
 
   // ZooKeeper won't tell us of a session expiration until we
@@ -391,7 +391,7 @@ void ZooKeeperMasterDetectorProcess::rec
 
 void ZooKeeperMasterDetectorProcess::expired()
 {
-  LOG(WARNING) << "Master detector ZooKeeper session expired!";
+  LOG(WARNING) << "Master detector (" << pid << ")  ZooKeeper session expired!";
 
   // Cancel and cleanup the reconnect timer (if necessary).
   if (timer.isSome()) {
@@ -473,9 +473,9 @@ void ZooKeeperMasterDetectorProcess::det
     if (zk->retryable(code)) {
       // NOTE: We don't expect a ZNONODE here because 'url.path' is always
       // created in the connected() call. Despite that, we don't do a
-      // CHECK (code != ZNONODE) just to be safe incase the zk client library
+      // CHECK (code != ZNONODE) just to be safe in case the zk client library
       // does return the code unexpectedly.
-      LOG(ERROR) << "Master detector failed to get masters: "
+      LOG(ERROR) << "Master detector (" << pid << ")  failed to get masters: "
                  << zk->message(code);
       return; // Try again when we reconnect.
     } else {
@@ -483,7 +483,7 @@ void ZooKeeperMasterDetectorProcess::det
                  << zk->message(code);
     }
   } else {
-    LOG(INFO) << "Master detector found " << results.size()
+    LOG(INFO) << "Master detector (" << pid << ")  found " << results.size()
               << " registered masters";
   }
 
@@ -504,7 +504,7 @@ void ZooKeeperMasterDetectorProcess::det
 
   // No master present (lost or possibly hasn't come up yet).
   if (masterSeq.empty()) {
-    LOG(INFO) << "Master detector of " << pid << " couldn't find any masters";
+    LOG(INFO) << "Master detector (" << pid << ") couldn't find any masters";
     process::post(pid, NoMasterDetectedMessage());
   } else if (masterSeq != currentMasterSeq) {
     // Okay, let's fetch the master pid from ZooKeeper.
@@ -529,7 +529,8 @@ void ZooKeeperMasterDetectorProcess::det
       }
     } else {
       // Now let's parse what we fetched from ZooKeeper.
-      LOG(INFO) << "Master detector got new master pid: " << result;
+      LOG(INFO) << "Master detector (" << pid << ")  got new master pid: "
+                << result;
 
       UPID masterPid = result;
 

Modified: incubator/mesos/trunk/src/tests/zookeeper_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/zookeeper_tests.cpp?rev=1418146&r1=1418145&r2=1418146&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/zookeeper_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/zookeeper_tests.cpp Fri Dec  7 00:45:41 2012
@@ -301,7 +301,9 @@ TEST_F(ZooKeeperTest, MasterDetectorShut
 }
 
 
-TEST_F(ZooKeeperTest, MasterDetectorExpireZKSession)
+// Tests whether a leading master correctly detects a new master when its
+// ZooKeeper session is expired.
+TEST_F(ZooKeeperTest, MasterDetectorExpireMasterZKSession)
 {
   // Simulate a leading master.
   MockMasterDetectorListenerProcess leader;
@@ -341,7 +343,10 @@ TEST_F(ZooKeeperTest, MasterDetectorExpi
 
   // Follower's detector.
   ZooKeeperMasterDetector followerDetector(
-      url.get(), follower.self(), true, true);
+      url.get(),
+      follower.self(),
+      true,
+      true);
 
   WAIT_UNTIL(newMasterDetectedCall3);
 
@@ -362,6 +367,167 @@ TEST_F(ZooKeeperTest, MasterDetectorExpi
 }
 
 
+// Tests whether a slave correctly DOES NOT disconnect from the master
+// when its ZooKeeper session is expired, but the master still stays the leader
+// when the slave re-connects with the ZooKeeper.
+TEST_F(ZooKeeperTest, MasterDetectorExpireSlaveZKSession)
+{
+  // Simulate a leading master.
+  MockMasterDetectorListenerProcess master;
+
+  trigger newMasterDetectedCall1;
+  EXPECT_CALL(master, newMasterDetected(_))
+    .WillOnce(Trigger(&newMasterDetectedCall1));
+
+  EXPECT_CALL(master, noMasterDetected())
+    .Times(0);
+
+  process::spawn(master);
+
+  std::string znode = "zk://" + server->connectString() + "/mesos";
+
+  Try<zookeeper::URL> url = zookeeper::URL::parse(znode);
+  ASSERT_SOME(url);
+
+  // Leading master's detector.
+  ZooKeeperMasterDetector masterDetector(url.get(), master.self(), true, true);
+
+  WAIT_UNTIL(newMasterDetectedCall1);
+
+  // Simulate a slave.
+  MockMasterDetectorListenerProcess slave;
+
+  trigger newMasterDetectedCall2, newMasterDetectedCall3;
+  EXPECT_CALL(slave, newMasterDetected(_))
+    .Times(1)
+    .WillOnce(Trigger(&newMasterDetectedCall2));
+
+  EXPECT_CALL(slave, noMasterDetected())
+    .Times(0);
+
+  process::spawn(slave);
+
+  // Slave's master detector.
+  ZooKeeperMasterDetector slaveDetector(url.get(), slave.self(), false, true);
+
+  WAIT_UNTIL(newMasterDetectedCall2);
+
+  // Now expire the slave's zk session.
+  process::Future<int64_t> session = slaveDetector.session();
+  ASSERT_FUTURE_WILL_SUCCEED(session);
+
+  server->expireSession(session.get());
+
+  // Wait for enough time to ensure no NewMasterDetected message is sent.
+  sleep(4); // ZooKeeper needs extra time for session expiration.
+
+  process::terminate(slave);
+  process::wait(slave);
+
+  process::terminate(master);
+  process::wait(master);
+}
+
+
+// Tests whether a slave correctly detects the new master
+// when its ZooKeeper session is expired and a new master is elected before the
+// slave reconnects with ZooKeeper.
+TEST_F(ZooKeeperTest, MasterDetectorExpireSlaveZKSessionNewMaster)
+{
+  // Simulate a leading master.
+  MockMasterDetectorListenerProcess master1;
+
+  trigger newMasterDetectedCall1;
+  EXPECT_CALL(master1, newMasterDetected(_))
+    .WillOnce(Trigger(&newMasterDetectedCall1));
+
+  EXPECT_CALL(master1, noMasterDetected())
+    .Times(0);
+
+  process::spawn(master1);
+
+  std::string znode = "zk://" + server->connectString() + "/mesos";
+
+  Try<zookeeper::URL> url = zookeeper::URL::parse(znode);
+  ASSERT_SOME(url);
+
+  // Leading master's detector.
+  ZooKeeperMasterDetector masterDetector1(
+      url.get(),
+      master1.self(),
+      true,
+      true);
+
+  WAIT_UNTIL(newMasterDetectedCall1);
+
+  // Simulate a non-leading master.
+  MockMasterDetectorListenerProcess master2;
+
+  trigger newMasterDetectedCall2;
+  EXPECT_CALL(master2, newMasterDetected(_))
+    .WillOnce(Trigger(&newMasterDetectedCall2))
+    .WillRepeatedly(Return());
+
+  EXPECT_CALL(master2, noMasterDetected())
+    .Times(0);
+
+  process::spawn(master2);
+
+  // Non-leading master's detector.
+  ZooKeeperMasterDetector masterDetector2(
+      url.get(),
+      master2.self(),
+      true,
+      true);
+
+  WAIT_UNTIL(newMasterDetectedCall2);
+
+  // Simulate a slave.
+  MockMasterDetectorListenerProcess slave;
+
+  trigger newMasterDetectedCall3, newMasterDetectedCall4;
+  EXPECT_CALL(slave, newMasterDetected(_))
+    .Times(2)
+    .WillOnce(Trigger(&newMasterDetectedCall3))
+    .WillOnce(Trigger(&newMasterDetectedCall4));
+
+  EXPECT_CALL(slave, noMasterDetected())
+    .Times(AtMost(1));
+
+  process::spawn(slave);
+
+  // Slave's master detector.
+  ZooKeeperMasterDetector slaveDetector(url.get(), slave.self(), false, true);
+
+  WAIT_UNTIL(newMasterDetectedCall3);
+
+  // Now expire the slave's and leading master's zk sessions.
+  // NOTE: Here we assume that slave stays disconnected from the ZK when the
+  // leading master loses its session.
+  process::Future<int64_t> slaveSession = slaveDetector.session();
+  ASSERT_FUTURE_WILL_SUCCEED(slaveSession);
+
+  server->expireSession(slaveSession.get());
+
+  process::Future<int64_t> masterSession = masterDetector1.session();
+  ASSERT_FUTURE_WILL_SUCCEED(masterSession);
+
+  server->expireSession(masterSession.get());
+
+  // Wait for session expiration and ensure we receive a newMasterDetected call.
+  WAIT_FOR(newMasterDetectedCall4, Seconds(5.0)); // ZooKeeper needs extra time.
+
+  process::terminate(slave);
+  process::wait(slave);
+
+  process::terminate(master2);
+  process::wait(master2);
+
+  process::terminate(master1);
+  process::wait(master1);
+}
+
+
 TEST_F(ZooKeeperTest, Group)
 {
   zookeeper::Group group(server->connectString(), NO_TIMEOUT, "/test/");