You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2019/08/13 14:54:58 UTC
[mesos] branch master updated (970026f -> 6fb9b4a)
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 970026f Fixed a flaky test `RescindOffersEnforcingLimits`.
new d5e67cf Dispatched invocations of resource provider mock default actions.
new 6fb9b4a Guarded access to possibly destructed resource provider driver.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/tests/api_tests.cpp | 11 ++++-
src/tests/mesos.hpp | 65 ++++++++++++++++++++-------
src/tests/resource_provider_manager_tests.cpp | 30 ++++++++-----
src/tests/slave_tests.cpp | 26 ++++++++---
4 files changed, 97 insertions(+), 35 deletions(-)
[mesos] 01/02: Dispatched invocations of resource provider mock
default actions.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d5e67cf9471cc0e197a2e9f2119d63a4e3f63d99
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Mon Aug 12 15:52:05 2019 +0200
Dispatched invocations of resource provider mock default actions.
When invoking member functions of the mock resource provider
`TestResourceProviderProcess` from default mock actions we need to
ensure that the mock object remains alive for the duration of the
invocation.
This patch replaces direct invocations which might run on any thread
(e.g., also while the mock object is being destructed on another thread)
which safe `dispatch`es of the default methods.
Review: https://reviews.apache.org/r/71272/
---
src/tests/api_tests.cpp | 11 ++++++--
src/tests/mesos.hpp | 39 ++++++++++++++++++++-------
src/tests/resource_provider_manager_tests.cpp | 30 +++++++++++++--------
src/tests/slave_tests.cpp | 26 +++++++++++++-----
4 files changed, 76 insertions(+), 30 deletions(-)
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index c209967..36fb7b0 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -9192,11 +9192,18 @@ TEST_P(AgentAPITest, MarkResourceProviderGone)
Future<mesos::v1::resource_provider::Event::Subscribed> subscribed;
+ auto resourceProviderProcess = resourceProvider.process->self();
EXPECT_CALL(*resourceProvider.process, subscribed(_))
.WillOnce(DoAll(
Invoke(
- resourceProvider.process.get(),
- &v1::TestResourceProviderProcess::subscribedDefault),
+ [resourceProviderProcess](
+ const typename mesos::v1::resource_provider::Event::Subscribed&
+ subscribed) {
+ dispatch(
+ resourceProviderProcess,
+ &v1::TestResourceProviderProcess::subscribedDefault,
+ subscribed);
+ }),
FutureArg<0>(&subscribed)))
.WillRepeatedly(Return());
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 73b6e42..1b00881 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -3039,32 +3039,50 @@ public:
TestResourceProviderProcess(
const ResourceProviderInfo& _info,
const Option<Resources>& _resources = None())
- : info(_info),
+ : process::ProcessBase(process::ID::generate("test-resource-provider")),
+ info(_info),
resources(_resources)
{
- ON_CALL(*this, connected())
- .WillByDefault(
- Invoke(this, &TestResourceProviderProcess::connectedDefault));
+ auto self = this->self();
+
+ ON_CALL(*this, connected()).WillByDefault(Invoke([self]() {
+ dispatch(self, &TestResourceProviderProcess::connectedDefault);
+ }));
EXPECT_CALL(*this, connected()).WillRepeatedly(DoDefault());
ON_CALL(*this, subscribed(_))
.WillByDefault(
- Invoke(this, &TestResourceProviderProcess::subscribedDefault));
+ Invoke([self](const typename Event::Subscribed& subscribed) {
+ dispatch(
+ self,
+ &TestResourceProviderProcess::subscribedDefault,
+ subscribed);
+ }));
EXPECT_CALL(*this, subscribed(_)).WillRepeatedly(DoDefault());
ON_CALL(*this, applyOperation(_))
.WillByDefault(
- Invoke(this, &TestResourceProviderProcess::operationDefault));
+ Invoke([self](const typename Event::ApplyOperation& operation) {
+ dispatch(
+ self,
+ &TestResourceProviderProcess::operationDefault,
+ operation);
+ }));
EXPECT_CALL(*this, applyOperation(_)).WillRepeatedly(DoDefault());
ON_CALL(*this, publishResources(_))
.WillByDefault(
- Invoke(this, &TestResourceProviderProcess::publishDefault));
+ Invoke([self](const typename Event::PublishResources& publish) {
+ dispatch(
+ self,
+ &TestResourceProviderProcess::publishDefault,
+ publish);
+ }));
EXPECT_CALL(*this, publishResources(_)).WillRepeatedly(DoDefault());
- ON_CALL(*this, teardown())
- .WillByDefault(
- Invoke(this, &TestResourceProviderProcess::teardownDefault));
+ ON_CALL(*this, teardown()).WillByDefault(Invoke([self]() {
+ dispatch(self, &TestResourceProviderProcess::teardownDefault);
+ }));
EXPECT_CALL(*this, teardown()).WillRepeatedly(DoDefault());
}
@@ -3087,6 +3105,7 @@ public:
{
while (!events.empty()) {
Event event = events.front();
+
events.pop();
switch (event.type()) {
diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp
index 2792000..bcf6a03 100644
--- a/src/tests/resource_provider_manager_tests.cpp
+++ b/src/tests/resource_provider_manager_tests.cpp
@@ -1192,13 +1192,15 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(
// We terminate the resource provider once we have confirmed that it
// got disconnected. This avoids it to in turn resubscribe racing
// with the newly created resource provider.
+ auto resourceProviderProcess = resourceProvider->process->self();
Future<Nothing> disconnected;
EXPECT_CALL(*resourceProvider->process, disconnected())
.WillOnce(DoAll(
- Invoke(
- resourceProvider->process.get(),
- &v1::TestResourceProviderProcess::stop),
- FutureSatisfy(&disconnected)))
+ Invoke([resourceProviderProcess]() {
+ dispatch(
+ resourceProviderProcess, &v1::TestResourceProviderProcess::stop);
+ }),
+ FutureSatisfy(&disconnected)))
.WillRepeatedly(Return()); // Ignore spurious calls concurrent with `stop`.
// The agent failover.
@@ -1270,12 +1272,15 @@ TEST_P(ResourceProviderManagerHttpApiTest, ResubscribeUnknownID)
// We explicitly terminate the resource provider after the expected
// disconnect to prevent it from resubscribing indefinitely.
+ auto resourceProviderProcess = resourceProvider.process->self();
Future<Nothing> disconnected;
EXPECT_CALL(*resourceProvider.process, disconnected())
.WillOnce(DoAll(
- Invoke(
- resourceProvider.process.get(), &v1::TestResourceProviderProcess::stop),
- FutureSatisfy(&disconnected)));
+ Invoke([resourceProviderProcess]() {
+ dispatch(
+ resourceProviderProcess, &v1::TestResourceProviderProcess::stop);
+ }),
+ FutureSatisfy(&disconnected)));
// Start and register a resource provider.
Owned<EndpointDetector> endpointDetector(
@@ -1423,13 +1428,16 @@ TEST_F(ResourceProviderManagerHttpApiTest, ResourceProviderSubscribeDisconnect)
// We terminate the first resource provider once we have confirmed
// that it got disconnected. This avoids it to in turn resubscribe
// racing with the other resource provider.
+ auto resourceProviderProcess1 = resourceProvider1.process->self();
Future<Nothing> disconnected1;
EXPECT_CALL(*resourceProvider1.process, disconnected())
.WillOnce(DoAll(
- Invoke(
- resourceProvider1.process.get(),
- &v1::TestResourceProviderProcess::stop),
- FutureSatisfy(&disconnected1)))
+ Invoke([resourceProviderProcess1]() {
+ dispatch(
+ resourceProviderProcess1,
+ &v1::TestResourceProviderProcess::stop);
+ }),
+ FutureSatisfy(&disconnected1)))
.WillRepeatedly(Return()); // Ignore spurious calls concurrent with `stop`.
Future<Event::Subscribed> subscribed2;
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 7c6e1d9..1a92602 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -109,6 +109,8 @@ using mesos::internal::protobuf::createLabel;
using mesos::master::detector::MasterDetector;
using mesos::master::detector::StandaloneMasterDetector;
+using mesos::v1::resource_provider::Event;
+
using mesos::slave::ContainerConfig;
using mesos::slave::ContainerTermination;
@@ -10682,15 +10684,25 @@ TEST_F(SlaveTest, ResourceProviderPublishAll)
// Two PUBLISH_RESOURCES events will be received: one for launching the
// executor, and the other for launching the task.
+ auto resourceProviderProcess = resourceProvider.process->self();
EXPECT_CALL(*resourceProvider.process, publishResources(_))
- .WillOnce(Invoke(
- resourceProvider.process.get(),
- &v1::TestResourceProviderProcess::publishDefault))
+ .WillOnce(
+ Invoke([resourceProviderProcess](
+ const typename Event::PublishResources& publish) {
+ dispatch(
+ resourceProviderProcess,
+ &v1::TestResourceProviderProcess::publishDefault,
+ publish);
+ }))
.WillOnce(DoAll(
- FutureArg<0>(&publish),
- Invoke(
- resourceProvider.process.get(),
- &v1::TestResourceProviderProcess::publishDefault)));
+ Invoke([resourceProviderProcess](
+ const typename Event::PublishResources& publish) {
+ dispatch(
+ resourceProviderProcess,
+ &v1::TestResourceProviderProcess::publishDefault,
+ publish);
+ }),
+ FutureArg<0>(&publish)));
Future<TaskStatus> taskStarting;
Future<TaskStatus> taskRunning;
[mesos] 02/02: Guarded access to possibly destructed resource
provider driver.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 6fb9b4a5c93378305bd54c8b1f5b12a79377318e
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Tue Aug 13 11:05:35 2019 +0200
Guarded access to possibly destructed resource provider driver.
We provide a method to tear down a resource provider driver which
internally resets the resource provider HTTP driver pointer. We need to
prevent access to the pointed to value once the driver has been torn
down.
This patch switches all internal uses of the the driver to `send` data
to use a public interface method instead. We also make sure that that
public method does not access an invalidated pointed to value.
Review: https://reviews.apache.org/r/71277
---
src/tests/mesos.hpp | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 1b00881..25359a2 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -3136,7 +3136,11 @@ public:
process::Future<Nothing> send(const Call& call)
{
- return driver->send(call);
+ if (driver != nullptr) {
+ return driver->send(call);
+ } else {
+ return process::Failure("Cannot send call since driver is torn down");
+ }
}
void start(
@@ -3198,7 +3202,10 @@ public:
call.set_type(Call::SUBSCRIBE);
call.mutable_subscribe()->mutable_resource_provider_info()->CopyFrom(info);
- driver->send(call);
+ send(call)
+ .onFailed([](const std::string& failure) {
+ LOG(INFO) << "Failed to send call: " << failure;
+ });
}
void subscribedDefault(const typename Event::Subscribed& subscribed)
@@ -3224,7 +3231,10 @@ public:
update->mutable_resource_version_uuid()->set_value(
id::UUID::random().toBytes());
- driver->send(call);
+ send(call)
+ .onFailed([](const std::string& failure) {
+ LOG(INFO) << "Failed to send call: " << failure;
+ });
}
}
@@ -3299,7 +3309,10 @@ public:
update->mutable_latest_status()->CopyFrom(update->status());
- driver->send(call);
+ send(call)
+ .onFailed([](const std::string& failure) {
+ LOG(INFO) << "Failed to send call: " << failure;
+ });
}
void publishDefault(const typename Event::PublishResources& publish)
@@ -3315,7 +3328,10 @@ public:
update->mutable_uuid()->CopyFrom(publish.uuid());
update->set_status(Call::UpdatePublishResourcesStatus::OK);
- driver->send(call);
+ send(call)
+ .onFailed([](const std::string& failure) {
+ LOG(INFO) << "Failed to send call: " << failure;
+ });
}
void teardownDefault() {}