You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/07/19 17:59:50 UTC

mesos git commit: Removed the `os::sleep` from `Clock::settle`.

Repository: mesos
Updated Branches:
  refs/heads/master a4892956c -> 5db0458cb


Removed the `os::sleep` from `Clock::settle`.

`Clock::settle` is used to wait until all pending libprocess events
have been handled. Test cases should typically use it only when there is
no other way to achieve the proper synchronization between two events.

Previously, `Clock::settle` also contained an `os::sleep` call to
workaround broken test cases that assumed `settle` provided stronger
guarantees than described above (e.g., some test cases assumed that
doing `http::get` followed by a `Clock::settle` ensured that the remote
side of the HTTP connection will have seen the request). Currently,
there are relatively few such test cases, so it is better to fix those
test cases (or add a `sleep` call to them) and remove the `sleep` from
`Clock::settle`.

Review: https://reviews.apache.org/r/50187/


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

Branch: refs/heads/master
Commit: 5db0458cb0b51907a25f18e4939f49f38b29b926
Parents: a489295
Author: Neil Conway <ne...@gmail.com>
Authored: Tue Jul 19 10:59:14 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Tue Jul 19 10:59:14 2016 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp             | 14 --------------
 3rdparty/libprocess/src/tests/metrics_tests.cpp |  4 ++++
 2 files changed, 4 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5db0458c/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 9661386..c20a72b 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -3129,20 +3129,6 @@ void ProcessManager::settle()
 {
   bool done = true;
   do {
-    // While refactoring in order to isolate libev behind abstractions
-    // it became evident that this os::sleep is vital for tests to
-    // pass. In particular, there are certain tests that assume too
-    // much before they attempt to do a settle. One such example is
-    // tests doing http::get followed by Clock::settle, where they
-    // expect the http::get will have properly enqueued a process on
-    // the run queue but http::get is just sending bytes on a
-    // socket. Without sleeping at the beginning of this function we
-    // can get unlucky and appear settled when in actuality the
-    // kernel just hasn't copied the bytes to a socket or we haven't
-    // yet read the bytes and enqueued an event on a process (and the
-    // process on the run queue).
-    os::sleep(Milliseconds(10));
-
     done = true; // Assume to start that we are settled.
 
     synchronized (runq_mutex) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/5db0458c/3rdparty/libprocess/src/tests/metrics_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/metrics_tests.cpp b/3rdparty/libprocess/src/tests/metrics_tests.cpp
index 5a82f4f..d4d99de 100644
--- a/3rdparty/libprocess/src/tests/metrics_tests.cpp
+++ b/3rdparty/libprocess/src/tests/metrics_tests.cpp
@@ -314,6 +314,10 @@ TEST_F(MetricsTest, SnapshotTimeout)
   Future<Response> response = http::get(upid, "snapshot", "timeout=2secs");
 
   // Make sure the request is pending before the timeout is exceeded.
+  //
+  // TODO(neilc): Replace the `sleep` here with a less flaky
+  // synchronization method.
+  os::sleep(Milliseconds(10));
   Clock::settle();
 
   ASSERT_TRUE(response.isPending());