You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/07/13 21:41:07 UTC
[02/17] mesos git commit: Fixed a race condition in
`UriDiskProfileAdaptorTests`.
Fixed a race condition in `UriDiskProfileAdaptorTests`.
There was a race between `Clock::advance()` in the `FetchFromHTTP` test
and `delay()` in `UriDiskProfileAdaptorProcess::_poll`. This patch
avoids the race by enforcing an order between the dispatch of the
`__poll` function (previously `_poll`) and the clock manipulation
in the test.
Review: https://reviews.apache.org/r/65640
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/afd1647b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/afd1647b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/afd1647b
Branch: refs/heads/master
Commit: afd1647b96abdc9352864f0a8e2f4c24a2f0c7d7
Parents: 0449b5f
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Tue Feb 13 14:08:00 2018 -0800
Committer: Chun-Hung Hsiao <ch...@mesosphere.io>
Committed: Fri Jul 13 14:30:06 2018 -0700
----------------------------------------------------------------------
src/Makefile.am | 8 +++++-
.../storage/uri_disk_profile_adaptor.cpp | 30 +++++++++++---------
.../storage/uri_disk_profile_adaptor.hpp | 7 +++--
src/tests/disk_profile_adaptor_tests.cpp | 17 ++++++-----
4 files changed, 38 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/afd1647b/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index db42e71..06b6a34 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2661,7 +2661,12 @@ mesos_tests_CPPFLAGS += -I../$(GMOCK)/include
mesos_tests_CPPFLAGS += -DTESTLIBEXECDIR=\"$(testlibexecdir)\"
mesos_tests_CPPFLAGS += -DSBINDIR=\"$(sbindir)\"
-mesos_tests_LDADD = ../3rdparty/libgmock.la libmesos.la $(LIB_GRPC) $(LDADD)
+mesos_tests_LDADD = \
+ ../3rdparty/libgmock.la \
+ libmesos.la \
+ liburi_disk_profile_adaptor.la \
+ $(LIB_GRPC) \
+ $(LDADD)
if !OS_FREEBSD
mesos_tests_LDADD += -ldl # FreeBSD includes dynamic lib utils in libc.
@@ -2670,6 +2675,7 @@ endif
mesos_tests_DEPENDENCIES = \
../3rdparty/libgmock.la \
libmesos.la \
+ liburi_disk_profile_adaptor.la \
$(MESOS_TEST_MODULES)
if OS_LINUX
http://git-wip-us.apache.org/repos/asf/mesos/blob/afd1647b/src/resource_provider/storage/uri_disk_profile_adaptor.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/storage/uri_disk_profile_adaptor.cpp b/src/resource_provider/storage/uri_disk_profile_adaptor.cpp
index 614590e..11876f4 100644
--- a/src/resource_provider/storage/uri_disk_profile_adaptor.cpp
+++ b/src/resource_provider/storage/uri_disk_profile_adaptor.cpp
@@ -197,24 +197,28 @@ void UriDiskProfileAdaptorProcess::poll()
CHECK_SOME(url);
http::get(url.get())
- .onAny(defer(self(), [=](const Future<http::Response>& future) {
- if (future.isReady()) {
- // NOTE: We don't check the HTTP status code because we don't know
- // what potential codes are considered successful.
- _poll(future->body);
- } else if (future.isFailed()) {
- _poll(Error(future.failure()));
- } else {
- _poll(Error("Future discarded or abandoned"));
- }
- }));
+ .onAny(defer(self(), &Self::_poll, lambda::_1));
} else {
- _poll(os::read(flags.uri.string()));
+ __poll(os::read(flags.uri.string()));
}
}
-void UriDiskProfileAdaptorProcess::_poll(const Try<string>& fetched)
+void UriDiskProfileAdaptorProcess::_poll(const Future<http::Response>& future)
+{
+ if (future.isReady()) {
+ // NOTE: We don't check the HTTP status code because we don't know
+ // what potential codes are considered successful.
+ __poll(future->body);
+ } else if (future.isFailed()) {
+ __poll(Error(future.failure()));
+ } else {
+ __poll(Error("Future discarded or abandoned"));
+ }
+}
+
+
+void UriDiskProfileAdaptorProcess::__poll(const Try<string>& fetched)
{
if (fetched.isSome()) {
Try<DiskProfileMapping> parsed = parseDiskProfileMapping(fetched.get());
http://git-wip-us.apache.org/repos/asf/mesos/blob/afd1647b/src/resource_provider/storage/uri_disk_profile_adaptor.hpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/storage/uri_disk_profile_adaptor.hpp b/src/resource_provider/storage/uri_disk_profile_adaptor.hpp
index 0484933..a2f8842 100644
--- a/src/resource_provider/storage/uri_disk_profile_adaptor.hpp
+++ b/src/resource_provider/storage/uri_disk_profile_adaptor.hpp
@@ -216,13 +216,15 @@ public:
const hashset<std::string>& knownProfiles,
const ResourceProviderInfo& resourceProviderInfo);
-private:
// Helpers for fetching the `--uri`.
// If `--poll_interval` is set, this method will dispatch to itself with
// a delay once the fetch is complete.
+ // Made public for testing purpose.
void poll();
- void _poll(const Try<std::string>& fetched);
+ void _poll(const process::Future<process::http::Response>& future);
+ void __poll(const Try<std::string>& fetched);
+private:
// Helper that is called upon successfully polling and parsing the `--uri`.
// This method will check the following conditions before updating the state
// of the module:
@@ -230,7 +232,6 @@ private:
// * All properties of known profiles must match those in the updated set.
void notify(const resource_provider::DiskProfileMapping& parsed);
-private:
UriDiskProfileAdaptor::Flags flags;
// The last fetched profile mapping.
http://git-wip-us.apache.org/repos/asf/mesos/blob/afd1647b/src/tests/disk_profile_adaptor_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/disk_profile_adaptor_tests.cpp b/src/tests/disk_profile_adaptor_tests.cpp
index 4485f16..7aa31fc 100644
--- a/src/tests/disk_profile_adaptor_tests.cpp
+++ b/src/tests/disk_profile_adaptor_tests.cpp
@@ -40,8 +40,8 @@
#include "module/manager.hpp"
-#include "resource_provider/storage/uri_disk_profile_adaptor.hpp"
#include "resource_provider/storage/disk_profile_utils.hpp"
+#include "resource_provider/storage/uri_disk_profile_adaptor.hpp"
#include "tests/flags.hpp"
#include "tests/mesos.hpp"
@@ -400,9 +400,7 @@ TEST_F(UriDiskProfileAdaptorTest, ParseInvalids)
// This creates a UriDiskProfileAdaptor module configured to read from a
// file and tests the basic `watch` -> `translate` workflow which
// callers of the module are expected to follow.
-//
-// Enable this test once MESOS-8567 is resolved.
-TEST_F(UriDiskProfileAdaptorTest, DISABLED_FetchFromFile)
+TEST_F(UriDiskProfileAdaptorTest, FetchFromFile)
{
Clock::pause();
@@ -456,6 +454,7 @@ TEST_F(UriDiskProfileAdaptorTest, DISABLED_FetchFromFile)
params);
ASSERT_SOME(_module);
+
Owned<DiskProfileAdaptor> module(_module.get());
// Start watching for updates.
@@ -646,6 +645,7 @@ TEST_F(UriDiskProfileAdaptorTest, FetchFromHTTP)
params);
ASSERT_SOME(_module);
+
Owned<DiskProfileAdaptor> module(_module.get());
// Wait for the first HTTP poll to complete.
@@ -659,13 +659,16 @@ TEST_F(UriDiskProfileAdaptorTest, FetchFromHTTP)
// Start watching for an update to the list of profiles.
future = module->watch({"profile"}, resourceProviderInfo);
+ Future<Nothing> contentsPolled =
+ FUTURE_DISPATCH(_, &storage::UriDiskProfileAdaptorProcess::_poll);
+
// Trigger the second HTTP poll.
Clock::advance(pollInterval);
AWAIT_ASSERT_READY(secondCall);
- // Dispatch a call to the module, which ensures that the polling has actually
- // completed (not just the HTTP call).
- AWAIT_ASSERT_READY(module->translate("profile", resourceProviderInfo));
+ // Ensure that the polling has actually completed.
+ AWAIT_ASSERT_READY(contentsPolled);
+ Clock::settle();
// We don't expect the module to notify watcher(s) because the server's
// response is considered invalid (the module does not allow profiles