You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2019/01/10 18:35:58 UTC
[mesos] branch master updated (3fece81 -> 9862262)
This is an automated email from the ASF dual-hosted git repository.
grag pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 3fece81 Added MESOS-9492 to 1.5.3 CHANGELOG.
new 148b748 Narrowed interface of `ReadOnlyHandler` members.
new 4b63b07 Added deduplication for read-only master requests.
new b4d8e7f Added new metric for cache hits.
new f961ed1 Exposed private data members for testing.
new 3955b35 Added unit tests for Master HTTP endpoints.
new d4dd325 Sent operation feedback when agent is marked as gone.
new 277d239 Remove outstanding operations when removing agents.
new 9862262 Fixed flaky check in cluster::Slave destructor.
The 8 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/Makefile.am | 3 +-
src/master/http.cpp | 89 +++++--
src/master/master.cpp | 71 +++++
src/master/master.hpp | 30 ++-
src/master/metrics.cpp | 4 +
src/master/metrics.hpp | 5 +
src/master/readonly_handler.cpp | 38 +--
src/tests/api_tests.cpp | 167 ++++++++++++
src/tests/cluster.cpp | 15 +-
src/tests/cluster.hpp | 6 +-
src/tests/master_load_tests.cpp | 559 ++++++++++++++++++++++++++++++++++++++++
src/tests/master_tests.cpp | 2 +
12 files changed, 930 insertions(+), 59 deletions(-)
create mode 100644 src/tests/master_load_tests.cpp
[mesos] 02/08: Added deduplication for read-only master requests.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 4b63b07ed4f7527236334bf3cb2741c7e5c28029
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:29:44 2019 -0800
Added deduplication for read-only master requests.
This change will skip the computation of an HTTP response
for requests with the same principal, endpoint and request
headers where both requests are authenticated within the
same batching windows.
Review: https://reviews.apache.org/r/68795/
---
src/master/http.cpp | 60 ++++++++++++++++++++++++++++++++++++++-------------
src/master/master.hpp | 6 ++++++
2 files changed, 51 insertions(+), 15 deletions(-)
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e352ec8..758946c 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -1259,9 +1259,10 @@ Future<Response> Master::Http::frameworks(
{VIEW_FRAMEWORK, VIEW_TASK, VIEW_EXECUTOR})
.then(defer(
master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::frameworks,
+ principal,
request.url.query,
approvers);
}));
@@ -2061,9 +2062,10 @@ Future<Response> Master::Http::slaves(
return ObjectApprovers::create(master->authorizer, principal, {VIEW_ROLE})
.then(defer(
master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::slaves,
+ principal,
request.url.query,
approvers);
}));
@@ -2359,19 +2361,16 @@ Future<Response> Master::Http::state(
return redirect(request);
}
- // TODO(alexr): De-duplicate response processing when the principal is
- // identical, e.g., if "bob" asks for state three times in one batch,
- // ideally we only compute the response for "bob" once since they're all
- // identical within a principal.
return ObjectApprovers::create(
master->authorizer,
principal,
{VIEW_ROLE, VIEW_FRAMEWORK, VIEW_TASK, VIEW_EXECUTOR, VIEW_FLAGS})
.then(defer(
master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::state,
+ principal,
request.url.query,
approvers);
}));
@@ -2380,16 +2379,44 @@ Future<Response> Master::Http::state(
Future<Response> Master::Http::deferBatchedRequest(
ReadOnlyRequestHandler handler,
+ const Option<Principal>& principal,
const hashmap<std::string, std::string>& queryParameters,
const Owned<ObjectApprovers>& approvers) const
{
bool scheduleBatch = batchedRequests.empty();
- // Add an element to the batched state requests.
- Promise<Response> promise;
- Future<Response> future = promise.future();
- batchedRequests.push_back(
- BatchedRequest{handler, queryParameters, approvers, std::move(promise)});
+ auto it = std::find_if(batchedRequests.begin(), batchedRequests.end(),
+ [handler, &principal, &queryParameters](
+ const BatchedRequest& batchedRequest) {
+ // NOTE: This is not a general-purpose request comparison, but
+ // specific to the batched requests which are always members of
+ // `ReadOnlyHandler`, since we rely on the response only depending
+ // on query parameters and the current master state.
+ return handler == batchedRequest.handler &&
+ principal == batchedRequest.principal &&
+ queryParameters == batchedRequest.queryParameters;
+ });
+
+ Future<Response> future;
+ if (it != batchedRequests.end()) {
+ // Return the existing future if we have a matching request.
+ // NOTE: This is effectively adding a layer of authorization permissions
+ // caching since we only checked the equality of principals, not the
+ // equality of the approvers themselves.
+ // On heavily-loaded masters, this could lead to a delay of several seconds
+ // before permission changes for a principal take effect.
+ future = it->promise.future();
+ } else {
+ // Add an element to the batched state requests.
+ Promise<Response> promise;
+ future = promise.future();
+ batchedRequests.push_back(BatchedRequest{
+ handler,
+ queryParameters,
+ principal,
+ approvers,
+ std::move(promise)});
+ }
// Schedule processing of batched requests if not yet scheduled.
if (scheduleBatch) {
@@ -2544,9 +2571,10 @@ Future<Response> Master::Http::stateSummary(
{VIEW_ROLE, VIEW_FRAMEWORK})
.then(defer(
master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::stateSummary,
+ principal,
request.url.query,
approvers);
}));
@@ -2596,9 +2624,10 @@ Future<Response> Master::Http::roles(
return ObjectApprovers::create(master->authorizer, principal, {VIEW_ROLE})
.then(defer(master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::roles,
+ principal,
request.url.query,
approvers);
}));
@@ -2972,9 +3001,10 @@ Future<Response> Master::Http::tasks(
{VIEW_FRAMEWORK, VIEW_TASK})
.then(defer(
master->self(),
- [this, request](const Owned<ObjectApprovers>& approvers) {
+ [this, request, principal](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::tasks,
+ principal,
request.url.query,
approvers);
}));
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 6230d1d..d5d33ff 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1809,6 +1809,7 @@ private:
process::Future<process::http::Response> deferBatchedRequest(
ReadOnlyRequestHandler handler,
+ const Option<process::http::authentication::Principal>& principal,
const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
@@ -1818,7 +1819,12 @@ private:
{
ReadOnlyRequestHandler handler;
hashmap<std::string, std::string> queryParameters;
+ Option<process::http::authentication::Principal> principal;
process::Owned<ObjectApprovers> approvers;
+
+ // NOTE: The returned response should be either of type
+ // `BODY` or `PATH`, since `PIPE`-type responses would
+ // break the deduplication mechanism.
process::Promise<process::http::Response> promise;
};
[mesos] 07/08: Remove outstanding operations when removing agents.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 277d239813807dc089b47a386124705a01867781
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:31:19 2019 -0800
Remove outstanding operations when removing agents.
Usually, offer operations are removed when the framework acknowledges
a terminal operation status update.
However, currently only operations on registered agents can be
acknowledged.
This commit explicitly deletes all outstanding operations from an agent
when it is removed.
Review: https://reviews.apache.org/r/69597/
---
src/master/master.cpp | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index a3de10d..454f06a 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -10985,6 +10985,32 @@ void Master::__removeSlave(
removeInverseOffer(inverseOffer, true); // Rescind!
}
+ // Usually, operations are removed when the framework acknowledges
+ // a terminal operation status update. However, currently only operations
+ // on registered agents can be acknowledged. Since we're about to remove
+ // this agent from the list of registered agents, clean out all outstanding
+ // operations to prevent leaks.
+ //
+ // NOTE: If the agent comes back, there will be a brief window between
+ // the `ReregisterSlaveMessage` and the first `UpdateSlaveMessage` where
+ // where the master will not be able to give correct answers to operation
+ // reconciliation requests. However, since the same thing happens during
+ // master failover, the scheduler must be able to handle this scenario
+ // anyway so we allow it to happen here.
+ foreachvalue (Operation* operation, utils::copy(slave->operations)) {
+ removeOperation(operation);
+ }
+
+ foreachvalue (
+ const Slave::ResourceProvider& provider,
+ slave->resourceProviders) {
+ foreachvalue (
+ Operation* operation,
+ utils::copy(provider.operations)) {
+ removeOperation(operation);
+ }
+ }
+
// Mark the slave as being removed.
slaves.registered.remove(slave);
slaves.removed.put(slave->id, Nothing());
[mesos] 08/08: Fixed flaky check in cluster::Slave destructor.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 9862262956d4a8f5e32c1267c2e0b36f41f7fc73
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:32:30 2019 -0800
Fixed flaky check in cluster::Slave destructor.
The destructor of `cluster::Slave` contained an assertion
that was not safe to assume in the presence of the
composing containerizer. This commit adds an additional
`Clock::settle()` to fix the issue.
Review: https://reviews.apache.org/r/69436/
---
src/tests/cluster.cpp | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/tests/cluster.cpp b/src/tests/cluster.cpp
index 4ad7c0b..6148984 100644
--- a/src/tests/cluster.cpp
+++ b/src/tests/cluster.cpp
@@ -688,8 +688,19 @@ Slave::~Slave()
}
}
- if (paused) {
- process::Clock::pause();
+ // When using the composing containerizer, the assertion checking
+ // `containers->empty()` below is racy, since the containers are
+ // not yet removed from that containerizer's internal data structures
+ // when the future becomes ready. Instead, an internal function to clean
+ // up these internal data structures is dispatched when the future
+ // becomes ready.
+ // To work around this, we wait for the clock to settle here. This can
+ // be removed once MESOS-9413 is resolved.
+ process::Clock::pause();
+ process::Clock::settle();
+
+ if (!paused) {
+ process::Clock::resume();
}
containers = containerizer->containers();
[mesos] 06/08: Sent operation feedback when agent is marked as gone.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d4dd325d06444fdc33dd8c581a6edf2235c36cbd
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:31:12 2019 -0800
Sent operation feedback when agent is marked as gone.
When an agent is marked as gone through the operator API,
notify all frameworks with outstanding offer operations on
that agent that these operations have been transitioned
to `OPERATION_GONE_BY_OPERATOR`.
Review: https://reviews.apache.org/r/69575/
---
src/master/master.cpp | 45 +++++++++++++
src/tests/api_tests.cpp | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 212 insertions(+)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 49b6e5c..a3de10d 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -8942,6 +8942,51 @@ void Master::markGone(Slave* slave, const TimeInfo& goneTime)
message.set_message("Agent has been marked gone");
send(slave->pid, message);
+ // Collect all offer operations on this agent and notify frameworks
+ // that they have been transitioned to `OPERATION_GONE_BY_OPERATOR`.
+ hashmap<UUID, const Operation*> operations;
+ operations.insert(slave->operations.begin(), slave->operations.end());
+ foreachvalue (
+ const Slave::ResourceProvider& provider,
+ slave->resourceProviders) {
+ operations.insert(provider.operations.begin(), provider.operations.end());
+ }
+
+ foreachvalue (const Operation* operation, operations) {
+ // Frameworks signal that they want to receive feedback
+ // on the operation status by setting the `id` field.
+ if (!operation->info().has_id()) {
+ continue;
+ }
+
+ // Offer operations made through the operator API might not have
+ // an associated framework id.
+ if (!operation->has_framework_id()) {
+ continue;
+ }
+
+ // NOTE: Frameworks that are not currently registered will not receive
+ // a notification here and need to rely on the reconciliation process.
+ Option<Framework*> framework =
+ frameworks.registered.get(operation->framework_id());
+
+ if (!framework.isSome() || !framework.get()->http.isSome()) {
+ continue;
+ }
+
+ // We don't need to ensure that the framework reliably gets this message
+ // since it will eventually receive the same status through reconciliation.
+ scheduler::Event update;
+ update.set_type(scheduler::Event::UPDATE_OPERATION_STATUS);
+ *update.mutable_update_operation_status()->mutable_status() =
+ protobuf::createOperationStatus(
+ OperationState::OPERATION_GONE_BY_OPERATOR,
+ operation->info().id(),
+ "Agent has been marked gone");
+
+ framework.get()->send(update);
+ }
+
__removeSlave(slave, "Agent has been marked gone", None());
}
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index b6064cd..c597243 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -51,6 +51,8 @@
#include "master/detector/standalone.hpp"
+#include "messages/messages.hpp"
+
#include "slave/slave.hpp"
#include "slave/containerizer/fetcher.hpp"
@@ -66,6 +68,7 @@
namespace http = process::http;
+
using google::protobuf::RepeatedPtrField;
using mesos::master::detector::MasterDetector;
@@ -73,6 +76,10 @@ using mesos::master::detector::StandaloneMasterDetector;
using mesos::slave::ContainerTermination;
+using mesos::v1::scheduler::Call;
+using mesos::v1::scheduler::Event;
+using mesos::v1::scheduler::Mesos;
+
using mesos::internal::devolve;
using mesos::internal::evolve;
@@ -4942,6 +4949,166 @@ TEST_P(MasterAPITest, TaskUpdatesUponAgentGone)
}
+// This test verifies that the master correctly sends
+// `OPERATION_GONE_BY_OPERATOR` status updates for operations
+// that are pending when the agent is marked as gone.
+TEST_P(MasterAPITest, OperationUpdatesUponAgentGone)
+{
+ master::Flags masterFlags = CreateMasterFlags();
+
+ Try<Owned<cluster::Master>> master = this->StartMaster(masterFlags);
+ ASSERT_SOME(master);
+
+ MockExecutor exec(DEFAULT_EXECUTOR_ID);
+ TestContainerizer containerizer(&exec);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags slaveFlags = CreateSlaveFlags();
+
+ Future<UpdateSlaveMessage> updateSlaveMessage =
+ FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave =
+ StartSlave(detector.get(), &containerizer, slaveFlags);
+
+ ASSERT_SOME(slave);
+ AWAIT_READY(updateSlaveMessage);
+
+ // Start and register a resource provider.
+ v1::ResourceProviderInfo resourceProviderInfo;
+ resourceProviderInfo.set_type("org.apache.mesos.rp.test");
+ resourceProviderInfo.set_name("test");
+
+ v1::Resource disk = v1::createDiskResource(
+ "200", "*", None(), None(), v1::createDiskSourceRaw());
+
+ Owned<v1::MockResourceProvider> resourceProvider(
+ new v1::MockResourceProvider(resourceProviderInfo, v1::Resources(disk)));
+
+ Owned<EndpointDetector> endpointDetector(
+ mesos::internal::tests::resource_provider::createEndpointDetector(
+ slave.get()->pid));
+
+ updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+
+ resourceProvider->start(endpointDetector, ContentType::PROTOBUF);
+
+ // Wait until the agent's resources have been updated to include the
+ // resource provider resources.
+ AWAIT_READY(updateSlaveMessage);
+
+ // Start and register a framework.
+ FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
+ framework.add_capabilities()->set_type(
+ FrameworkInfo::Capability::PARTITION_AWARE);
+
+ auto scheduler = std::make_shared<v1::MockHTTPScheduler>();
+
+ v1::FrameworkInfo frameworkInfo = v1::DEFAULT_FRAMEWORK_INFO;
+ frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
+
+ EXPECT_CALL(*scheduler, connected(_))
+ .WillOnce(v1::scheduler::SendSubscribe(frameworkInfo));
+
+ Future<Event::Subscribed> subscribed;
+ EXPECT_CALL(*scheduler, subscribed(_, _))
+ .WillOnce(FutureArg<1>(&subscribed));
+
+ Future<Event::Offers> offers;
+ EXPECT_CALL(*scheduler, offers(_, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ EXPECT_CALL(*scheduler, heartbeat(_))
+ .WillRepeatedly(Return()); // Ignore heartbeats.
+
+ v1::scheduler::TestMesos mesos(
+ master.get()->pid,
+ ContentType::PROTOBUF,
+ scheduler);
+
+ AWAIT_READY(subscribed);
+ v1::FrameworkID frameworkId(subscribed->framework_id());
+
+ AWAIT_READY(offers);
+ ASSERT_FALSE(offers->offers().empty());
+
+ const v1::Offer& offer = offers->offers(0);
+
+ // Don't let the message get to the agent, so it stays pending.
+ Future<ApplyOperationMessage> applyOperationMessage =
+ DROP_PROTOBUF(ApplyOperationMessage(), _, _);
+
+ // Try to reserve those resources managed by a resource provider.
+ // (because operation feedback is only supported for that case)
+ v1::Resources resources =
+ v1::Resources(offer.resources()).filter([](const v1::Resource& resource) {
+ return resource.has_provider_id();
+ });
+
+ ASSERT_FALSE(resources.empty());
+
+ v1::Resource reserved = *(resources.begin());
+ reserved.add_reservations()->CopyFrom(
+ v1::createDynamicReservationInfo(
+ frameworkInfo.roles(0), DEFAULT_CREDENTIAL.principal()));
+
+ // Explicitly set an operation id to opt-in to operation feedback.
+ v1::OperationID operationId;
+ operationId.set_value("operation");
+
+ mesos.send(
+ v1::createCallAccept(
+ frameworkId,
+ offer,
+ {v1::RESERVE(reserved, operationId)}));
+
+ AWAIT_READY(applyOperationMessage);
+
+ // Mark the agent as gone. This should result in the agent being shutdown.
+ Future<ShutdownMessage> shutdownMessage =
+ FUTURE_PROTOBUF(ShutdownMessage(), master.get()->pid, _);
+
+ // Mark the agent as gone. This should result in the master sending
+ // a 'TASK_GONE_BY_OPERATOR' update for the running task.
+ Future<v1::scheduler::Event::UpdateOperationStatus> operationGoneUpdate;
+ EXPECT_CALL(*scheduler, updateOperationStatus(_, _))
+ .WillOnce(FutureArg<1>(&operationGoneUpdate));
+
+ ContentType contentType = GetParam();
+
+ v1::AgentID agentId = offers->offers(0).agent_id();
+
+ v1::master::Call v1Call;
+ v1Call.set_type(v1::master::Call::MARK_AGENT_GONE);
+
+ v1::master::Call::MarkAgentGone* markAgentGone =
+ v1Call.mutable_mark_agent_gone();
+
+ markAgentGone->mutable_agent_id()->CopyFrom(agentId);
+
+ Future<http::Response> response = http::post(
+ master.get()->pid,
+ "api/v1",
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+ serialize(contentType, v1Call),
+ stringify(contentType));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::OK().status, response);
+
+ AWAIT_READY(shutdownMessage);
+
+ // Wait for the framework to receive the OPERATION_GONE_BY_OPERATOR update.
+ AWAIT_READY(operationGoneUpdate);
+
+ EXPECT_EQ(operationId, operationGoneUpdate->status().operation_id());
+ EXPECT_EQ(
+ v1::OPERATION_GONE_BY_OPERATOR,
+ operationGoneUpdate->status().state());
+}
+
+
class AgentAPITest
: public MesosTest,
public WithParamInterface<ContentType>
[mesos] 05/08: Added unit tests for Master HTTP endpoints.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 3955b357e7b36b1b20724d920d9138be2180b8e3
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:29:54 2019 -0800
Added unit tests for Master HTTP endpoints.
This commit adds a set of unit test to verify that
basic master HTTP endpoints still work correctly
under the presence of request caching.
Review: https://reviews.apache.org/r/69064/
---
src/Makefile.am | 3 +-
src/tests/master_load_tests.cpp | 559 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 561 insertions(+), 1 deletion(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 188a470..cd78525 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2507,7 +2507,8 @@ mesos_tests_SOURCES = \
tests/master_allocator_tests.cpp \
tests/master_authorization_tests.cpp \
tests/master_benchmarks.cpp \
- tests/master_contender_detector_tests.cpp \
+ tests/master_contender_detector_tests.cpp \
+ tests/master_load_tests.cpp \
tests/master_maintenance_tests.cpp \
tests/master_quota_tests.cpp \
tests/master_slave_reconciliation_tests.cpp \
diff --git a/src/tests/master_load_tests.cpp b/src/tests/master_load_tests.cpp
new file mode 100644
index 0000000..4e9c8e8
--- /dev/null
+++ b/src/tests/master_load_tests.cpp
@@ -0,0 +1,559 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <mesos/mesos.hpp>
+
+#include <process/async.hpp>
+#include <process/clock.hpp>
+#include <process/future.hpp>
+#include <process/gmock.hpp>
+#include <process/http.hpp>
+#include <process/owned.hpp>
+#include <process/pid.hpp>
+
+#include "tests/mesos.hpp"
+
+using mesos::authorization::VIEW_EXECUTOR;
+using mesos::authorization::VIEW_FLAGS;
+using mesos::authorization::VIEW_FRAMEWORK;
+using mesos::authorization::VIEW_ROLE;
+using mesos::authorization::VIEW_TASK;
+
+using mesos::internal::slave::Containerizer;
+using mesos::internal::slave::Fetcher;
+using mesos::internal::slave::MesosContainerizer;
+
+using mesos::master::detector::MasterDetector;
+
+using process::async;
+using process::Clock;
+using process::delay;
+using process::Future;
+using process::Message;
+using process::Owned;
+using process::PID;
+using process::Promise;
+using process::Time;
+
+using process::http::Response;
+using process::http::Request;
+using process::http::Headers;
+
+using testing::SaveArg;
+
+
+// The tests in this file are designed to verify that the caching
+// of read-only requests inside a Mesos master is implemented correctly,
+// i.e. cacheable requests are cached and non-cacheable requests will
+// return different responses.
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+class BlockingAuthorizer;
+
+
+class MasterLoadTest : public MesosTest {
+protected:
+ // Describes a given HTTP request.
+ struct RequestDescriptor {
+ std::string endpoint;
+ std::string principal;
+ std::string query;
+ process::http::Headers headers;
+
+ bool operator<(const RequestDescriptor& other) const;
+ };
+
+ // Prepare a mock cluster with 1 master, 1 agent and 1 framework,
+ // with the given authorizer being wrapped in a `BlockingAuthorizer`.
+ void prepareCluster(Authorizer* authorizer);
+
+ // This function launches a fixed number of equivalent requests per passed
+ // request descriptor, while manipulating the master in order to
+ // ensure all requests will appear consecutively in the master queue.
+ // The returned map associates each response with the descriptor it was
+ // created from.
+ std::multimap<RequestDescriptor, Future<Response>>
+ launchSimultaneousRequests(
+ const std::vector<RequestDescriptor>& descriptors);
+
+ // The "mock cluster" created by `prepareCluster()`. These are `protected`
+ // so that the test body can access them if required.
+ Owned<BlockingAuthorizer> authorizer_;
+ Owned<cluster::Master> master_;
+ Owned<MasterDetector> detector_;
+ Owned<MockScheduler> scheduler_;
+ Owned<TestingMesosSchedulerDriver> driver_;
+ Owned<cluster::Slave> slave_;
+ FrameworkID frameworkId_;
+};
+
+
+// This authorizer will not satisfy any futures from `getObjectApprover()`
+// until it is told to, presumably from the test body.
+//
+// It effectively acts as a giant gate for certain requests.
+class BlockingAuthorizerProcess
+ : public process::Process<BlockingAuthorizerProcess>
+{
+public:
+ BlockingAuthorizerProcess(Authorizer* underlying)
+ : ProcessBase(process::ID::generate("blocking-authorizer")),
+ underlying_(underlying),
+ blocked_(true) {}
+
+ Future<bool> authorized(const authorization::Request& request)
+ {
+ return underlying_->authorized(request);
+ }
+
+ Future<Owned<ObjectApprover>> getObjectApprover(
+ const Option<authorization::Subject>& subject,
+ const authorization::Action& action)
+ {
+ Future<Owned<ObjectApprover>> future =
+ underlying_->getObjectApprover(subject, action);
+
+ if (!blocked_) {
+ return future;
+ }
+
+ // The future is linked to the returned promise in `unleash()`.
+ futures_.push(future);
+ promises_.emplace();
+ return promises_.back().future();
+ }
+
+ Future<size_t> pending()
+ {
+ return promises_.size();
+ }
+
+ // Satisfies all future and prior calls made to `getObjectApprover`.
+ Future<Nothing> unleash()
+ {
+ CHECK_EQ(promises_.size(), futures_.size());
+
+ while (!promises_.empty()) {
+ promises_.front().associate(futures_.front());
+
+ futures_.pop();
+ promises_.pop();
+ }
+
+ blocked_ = false;
+
+ return Nothing();
+ }
+
+private:
+ Authorizer* underlying_;
+ std::queue<Future<Owned<ObjectApprover>>> futures_;
+ std::queue<Promise<Owned<ObjectApprover>>> promises_;
+ bool blocked_;
+};
+
+
+class BlockingAuthorizer : public Authorizer
+{
+public:
+ BlockingAuthorizer(Authorizer* underlying)
+ : process_(new BlockingAuthorizerProcess(underlying))
+ {
+ process::spawn(process_.get());
+ }
+
+ ~BlockingAuthorizer()
+ {
+ process::terminate(process_.get());
+ process::wait(process_.get());
+ }
+
+ Future<bool> authorized(const authorization::Request& request) override
+ {
+ return process::dispatch(
+ process_.get(),
+ &BlockingAuthorizerProcess::authorized,
+ request);
+ }
+
+ Future<Owned<ObjectApprover>> getObjectApprover(
+ const Option<authorization::Subject>& subject,
+ const authorization::Action& action) override
+ {
+ return process::dispatch(
+ process_.get(),
+ &BlockingAuthorizerProcess::getObjectApprover,
+ subject,
+ action);
+ }
+
+ Future<size_t> pending()
+ {
+ return process::dispatch(
+ process_.get(),
+ &BlockingAuthorizerProcess::pending);
+ }
+
+ Future<Nothing> unleash()
+ {
+ return process::dispatch(
+ process_.get(),
+ &BlockingAuthorizerProcess::unleash);
+ }
+
+private:
+ Owned<BlockingAuthorizerProcess> process_;
+};
+
+
+void MasterLoadTest::prepareCluster(Authorizer* authorizer)
+{
+ // Start a master.
+ authorizer_.reset(new BlockingAuthorizer(authorizer));
+ master::Flags masterFlags = CreateMasterFlags();
+ Try<Owned<cluster::Master>> master = StartMaster(
+ authorizer_.get(), masterFlags);
+
+ ASSERT_SOME(master);
+ master_ = master.get();
+ detector_ = master_->createDetector();
+
+ Future<FrameworkRegisteredMessage> frameworkRegisteredMessage =
+ FUTURE_PROTOBUF(FrameworkRegisteredMessage(), _, _);
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ // Start a framework.
+ scheduler_.reset(new MockScheduler());
+ driver_.reset(new TestingMesosSchedulerDriver(
+ scheduler_.get(), detector_.get()));
+
+ EXPECT_CALL(*scheduler_, registered(driver_.get(), _, _))
+ .WillOnce(SaveArg<1>(&frameworkId_));
+
+ driver_->start();
+ AWAIT_READY(frameworkRegisteredMessage);
+
+ // Start an agent.
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector_.get(), slaveFlags);
+
+ ASSERT_SOME(slave);
+ slave_ = slave.get();
+
+ AWAIT_READY(slaveRegisteredMessage);
+}
+
+
+std::multimap<MasterLoadTest::RequestDescriptor, Future<Response>>
+MasterLoadTest::launchSimultaneousRequests(
+ const std::vector<RequestDescriptor>& descriptors)
+{
+ // NOTE: On Mac, the default number of open files (and thus tcp connections)
+ // is limited to 256 by default, so this number is tweaked to stay slightly
+ // lower than that at 40*5==200 connections for the most demanding test.
+ const size_t REQUESTS_PER_DESCRIPTOR = 40;
+ const size_t totalRequests = REQUESTS_PER_DESCRIPTOR * descriptors.size();
+
+ std::multimap<RequestDescriptor, Future<Response>> requests;
+
+ // Need this wrapper since `AWAIT_READY()` expects a `void` return type.
+ [&] {
+ // Send out all http requests based on the specifications
+ // found in `descriptors` and store the result in `requests`.
+ foreach (const RequestDescriptor& descriptor, descriptors) {
+ for (size_t i=0; i < REQUESTS_PER_DESCRIPTOR; ++i) {
+ Future<Response> response = process::http::get(
+ master_->pid,
+ descriptor.endpoint,
+ descriptor.query,
+ descriptor.headers);
+
+ requests.emplace(descriptor, response);
+ }
+ }
+
+ // Wait until all the HTTP events have reached the master and are now
+ // awaiting authorization. There might be some other requests that get
+ // mixed into the authorizer, so we must have ample requests in the
+ // test body to ensure cache hits.
+ Time whileLoopStartTime = Clock::now();
+ Future<size_t> pendingHttpCalls;
+ do {
+ pendingHttpCalls = authorizer_->pending();
+ AWAIT_READY(pendingHttpCalls);
+ // Protect against a potential infinite loop introduced by future bugs.
+ ASSERT_TRUE(Clock::now() - whileLoopStartTime < Seconds(20));
+ } while (pendingHttpCalls.get() < totalRequests);
+
+
+ // Now block the master actor, since we don't want the master to start
+ // batching until it is queued up with all the HTTP requests.
+ // NOTE: This function might be out of scope when the dispatch is
+ // scheduled, so we need to pass `masterBlocker` by value.
+ auto masterBlocker = std::make_shared<Promise<Nothing>>();
+ process::dispatch(master_->pid, [masterBlocker]() {
+ masterBlocker->future().await();
+ });
+
+ // Unblock the BlockingAuthorizer.
+ // This should trigger all the deferrals onto the master from the
+ // Authorizer's thread. When this future completes, the master's queue
+ // should be full of batched requests.
+ AWAIT_READY(authorizer_->unleash());
+
+ // Unblock the master now, so it can perform the batching.
+ masterBlocker->set(Nothing());
+ }();
+
+ return requests;
+}
+
+
+bool MasterLoadTest::RequestDescriptor::operator<(
+ const RequestDescriptor& other) const
+{
+ return endpoint < other.endpoint;
+}
+
+
+// Test that simultaneous responses to various different endpoints
+// all return the expected result.
+TEST_F(MasterLoadTest, SimultaneousBatchedRequests)
+{
+ MockAuthorizer mockAuthorizer;
+ prepareCluster(&mockAuthorizer);
+
+ // Set up the actual test.
+ RequestDescriptor descriptor1;
+ descriptor1.headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+ descriptor1.endpoint = "/state";
+
+ RequestDescriptor descriptor2 = descriptor1;
+ descriptor2.endpoint = "/state-summary";
+
+ RequestDescriptor descriptor3 = descriptor1;
+ descriptor3.endpoint = "/frameworks";
+
+ RequestDescriptor descriptor4 = descriptor1;
+ descriptor4.endpoint = "/slaves";
+
+ RequestDescriptor descriptor5 = descriptor1;
+ descriptor5.endpoint = "/roles";
+
+ auto responses = launchSimultaneousRequests(
+ {descriptor1, descriptor2, descriptor3, descriptor4, descriptor5});
+
+ foreachpair (
+ const RequestDescriptor& request,
+ Future<Response>& response,
+ responses)
+ {
+ AWAIT_READY(response);
+
+ mesos::internal::master::Master* master = master_->master.get();
+ mesos::internal::master::Master::ReadOnlyHandler readOnlyHandler(master);
+
+ // TODO(bevers): Ideally we would not use HTTP at all to generate
+ // the reference response, but some master-internal function
+ // like `model(Summary<Master>)`.
+ Try<hashmap<std::string, std::string>> queryParameters_ =
+ process::http::query::decode(request.query);
+
+ ASSERT_SOME(queryParameters_);
+ hashmap<std::string, std::string> queryParameters = queryParameters_.get();
+
+ process::http::authentication::Principal principal(request.principal);
+ MockAuthorizer authorizer;
+ Owned<ObjectApprovers> approvers = ObjectApprovers::create(
+ &authorizer,
+ principal,
+ {VIEW_ROLE, VIEW_FLAGS, VIEW_FRAMEWORK, VIEW_TASK, VIEW_EXECUTOR})
+ .get();
+
+ Response reference;
+ if (request.endpoint == "/state") {
+ reference = readOnlyHandler.state(queryParameters, approvers);
+ } else if (request.endpoint == "/state-summary") {
+ reference = readOnlyHandler.stateSummary(queryParameters, approvers);
+ } else if (request.endpoint == "/roles") {
+ reference = readOnlyHandler.roles(queryParameters, approvers);
+ } else if (request.endpoint == "/frameworks") {
+ reference = readOnlyHandler.frameworks(queryParameters, approvers);
+ } else if (request.endpoint == "/slaves") {
+ reference = readOnlyHandler.slaves(queryParameters, approvers);
+ } else {
+ UNREACHABLE();
+ }
+
+ EXPECT_EQ(reference.body, response->body);
+ }
+
+ // Ensure that we actually hit the metrics code path while executing
+ // the test.
+ JSON::Object metrics = Metrics();
+ ASSERT_TRUE(metrics.values["master/http_cache_hits"].is<JSON::Number>());
+ ASSERT_GT(
+ metrics.values["master/http_cache_hits"].as<JSON::Number>().as<size_t>(),
+ 0u);
+}
+
+
+// Test that simultaneous requests on a single endpoint for two
+// different principals return different results.
+TEST_F(MasterLoadTest, Principals)
+{
+ // Set up a proper authorizer for this test.
+ master::Flags flags = CreateMasterFlags();
+
+ {
+ // Default principal is allowed to view frameworks.
+ mesos::ACL::ViewFramework* acl = flags.acls->add_view_frameworks();
+ acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+ acl->mutable_users()->set_type(mesos::ACL::Entity::ANY);
+ }
+
+ {
+ // Default principal 2 is not allowed to view frameworks.
+ mesos::ACL::ViewFramework* acl = flags.acls->add_view_frameworks();
+ acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+ acl->mutable_users()->set_type(mesos::ACL::Entity::NONE);
+ }
+
+ Authorizer* localAuthorizer = Authorizer::create(flags.acls.get()).get();
+ prepareCluster(localAuthorizer);
+
+ // Set up the requests with correct principals.
+ RequestDescriptor descriptor1;
+ descriptor1.endpoint = "/frameworks";
+ descriptor1.principal = DEFAULT_CREDENTIAL.principal();
+ descriptor1.headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+
+ RequestDescriptor descriptor2 = descriptor1;
+ descriptor2.principal = DEFAULT_CREDENTIAL_2.principal();
+ descriptor2.headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL_2);
+
+ auto responses = launchSimultaneousRequests({descriptor1, descriptor2});
+
+ JSON::Value expected = JSON::parse(
+ "{"
+ "\"frameworks\": [{"
+ "\"id\": \"" + stringify(frameworkId_) + "\""
+ "}]"
+ "}"
+ ).get();
+
+ foreachpair (
+ const RequestDescriptor& request,
+ Future<Response>& response,
+ responses)
+ {
+ AWAIT_READY(response);
+
+ Try<JSON::Value> jsonResponse = JSON::parse(response->body);
+ ASSERT_SOME(jsonResponse);
+
+ if (request.principal == DEFAULT_CREDENTIAL.principal()) {
+ EXPECT_TRUE(jsonResponse->contains(expected))
+ << "Principal " << request.principal
+ << " got HTTP response: " << response->body;
+ } else {
+ EXPECT_FALSE(jsonResponse->contains(expected))
+ << "Principal " << request.principal
+ << " got HTTP response: " << response->body;
+ }
+ }
+
+ // Ensure that we actually hit the metrics code path while executing
+ // the test.
+ JSON::Object metrics = Metrics();
+ ASSERT_TRUE(metrics.values["master/http_cache_hits"].is<JSON::Number>());
+ ASSERT_GT(
+ metrics.values["master/http_cache_hits"].as<JSON::Number>().as<size_t>(),
+ 0u);
+}
+
+
+// Test that simultaneous requests on a single endpoint with
+// different query parameters produce different results.
+TEST_F(MasterLoadTest, QueryParameters)
+{
+ MockAuthorizer mockAuthorizer;
+ prepareCluster(&mockAuthorizer);
+
+ RequestDescriptor descriptor1;
+ descriptor1.endpoint = "/frameworks";
+ descriptor1.headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+ descriptor1.query = "";
+
+ RequestDescriptor descriptor2 = descriptor1;
+ descriptor2.query = "framework_id=nonexisting-framework-id";
+
+ RequestDescriptor descriptor3 = descriptor1;
+ descriptor3.query = "jsonp=xxx";
+
+ auto responses = launchSimultaneousRequests(
+ {descriptor1, descriptor2, descriptor3});
+
+ JSON::Value expected = JSON::parse(
+ "{"
+ "\"frameworks\": [{"
+ "\"id\": \"" + stringify(frameworkId_) + "\""
+ "}]"
+ "}"
+ ).get();
+
+ foreachpair (
+ const RequestDescriptor& request,
+ Future<Response>& response,
+ responses)
+ {
+ AWAIT_READY(response);
+
+ if (strings::contains(request.query, "jsonp")) {
+ EXPECT_TRUE(strings::contains(response->body, "xxx"))
+ << "Got HTTP response: " << response->body;
+ continue;
+ }
+
+ Try<JSON::Value> jsonResponse = JSON::parse(response->body);
+ ASSERT_SOME(jsonResponse);
+
+ if (strings::contains(request.query, "framework_id")) {
+ EXPECT_FALSE(jsonResponse->contains(expected))
+ << "Got HTTP response: " << response->body;
+ } else {
+ EXPECT_TRUE(jsonResponse->contains(expected))
+ << "Got HTTP response: " << response->body;
+ }
+ }
+
+ // Ensure that we actually hit the metrics code path while executing
+ // the test.
+ JSON::Object metrics = Metrics();
+ ASSERT_TRUE(metrics.values["master/http_cache_hits"].is<JSON::Number>());
+ ASSERT_GT(
+ metrics.values["master/http_cache_hits"].as<JSON::Number>().as<size_t>(),
+ 0u);
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {
[mesos] 03/08: Added new metric for cache hits.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b4d8e7f23be3e0b182207f7573aca707bce82093
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:29:47 2019 -0800
Added new metric for cache hits.
This new metric counts the total number of cache
hits in the newly-added request batching mechanism
of the Mesos master.
Review: https://reviews.apache.org/r/69422/
---
src/master/http.cpp | 1 +
src/master/metrics.cpp | 4 ++++
src/master/metrics.hpp | 5 +++++
src/tests/master_tests.cpp | 2 ++
4 files changed, 12 insertions(+)
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 758946c..012ee4f 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2406,6 +2406,7 @@ Future<Response> Master::Http::deferBatchedRequest(
// On heavily-loaded masters, this could lead to a delay of several seconds
// before permission changes for a principal take effect.
future = it->promise.future();
+ ++master->metrics->http_cache_hits;
} else {
// Add an element to the batched state requests.
Promise<Response> promise;
diff --git a/src/master/metrics.cpp b/src/master/metrics.cpp
index bb029d3..4dd73fb 100644
--- a/src/master/metrics.cpp
+++ b/src/master/metrics.cpp
@@ -115,6 +115,8 @@ Metrics::Metrics(const Master& master)
"master/tasks_gone_by_operator"),
dropped_messages(
"master/dropped_messages"),
+ http_cache_hits(
+ "master/http_cache_hits"),
messages_register_framework(
"master/messages_register_framework"),
messages_reregister_framework(
@@ -251,6 +253,7 @@ Metrics::Metrics(const Master& master)
process::metrics::add(tasks_gone_by_operator);
process::metrics::add(dropped_messages);
+ process::metrics::add(http_cache_hits);
// Messages from schedulers.
process::metrics::add(messages_register_framework);
@@ -404,6 +407,7 @@ Metrics::~Metrics()
process::metrics::remove(tasks_gone_by_operator);
process::metrics::remove(dropped_messages);
+ process::metrics::remove(http_cache_hits);
// Messages from schedulers.
process::metrics::remove(messages_register_framework);
diff --git a/src/master/metrics.hpp b/src/master/metrics.hpp
index eca48e6..4495e65 100644
--- a/src/master/metrics.hpp
+++ b/src/master/metrics.hpp
@@ -86,6 +86,11 @@ struct Metrics
// Message counters.
process::metrics::Counter dropped_messages;
+ // HTTP cache hits.
+ // TODO(bevers): Collect these per endpoint once per-endpoint
+ // metrics get merged.
+ process::metrics::Counter http_cache_hits;
+
// Metrics specific to frameworks of a common principal.
// These metrics have names prefixed by "frameworks/<principal>/".
struct Frameworks
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 80642f4..67713c8 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -2285,6 +2285,8 @@ TEST_F(MasterTest, MetricsInMetricsEndpoint)
EXPECT_EQ(1u, snapshot.values.count("master/dropped_messages"));
+ EXPECT_EQ(1u, snapshot.values.count("master/http_cache_hits"));
+
// Messages from schedulers.
EXPECT_EQ(1u, snapshot.values.count("master/messages_register_framework"));
EXPECT_EQ(1u, snapshot.values.count("master/messages_reregister_framework"));
[mesos] 01/08: Narrowed interface of `ReadOnlyHandler` members.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 148b7483234b1989518d191b4db4365a1852ca4f
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:29:41 2019 -0800
Narrowed interface of `ReadOnlyHandler` members.
Previously, the members of ReadOnlyHandler would take a full
Request as parameter, making it hard for clients to reason
about which parts of the request are used internally, and
even harder to guarantee that behaviour into the future.
This commit changes the interface so only the query
parameters get passed.
Review: https://reviews.apache.org/r/69071/
---
src/master/http.cpp | 30 +++++++++++++++++++-----------
src/master/master.hpp | 22 +++++++++++++---------
src/master/readonly_handler.cpp | 38 +++++++++++++++++++-------------------
3 files changed, 51 insertions(+), 39 deletions(-)
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e38513b..e352ec8 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -1261,7 +1261,9 @@ Future<Response> Master::Http::frameworks(
master->self(),
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
- &Master::ReadOnlyHandler::frameworks, request, approvers);
+ &Master::ReadOnlyHandler::frameworks,
+ request.url.query,
+ approvers);
}));
}
@@ -2061,7 +2063,9 @@ Future<Response> Master::Http::slaves(
master->self(),
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
- &Master::ReadOnlyHandler::slaves, request, approvers);
+ &Master::ReadOnlyHandler::slaves,
+ request.url.query,
+ approvers);
}));
}
@@ -2368,7 +2372,7 @@ Future<Response> Master::Http::state(
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::state,
- request,
+ request.url.query,
approvers);
}));
}
@@ -2376,7 +2380,7 @@ Future<Response> Master::Http::state(
Future<Response> Master::Http::deferBatchedRequest(
ReadOnlyRequestHandler handler,
- const Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const Owned<ObjectApprovers>& approvers) const
{
bool scheduleBatch = batchedRequests.empty();
@@ -2385,7 +2389,7 @@ Future<Response> Master::Http::deferBatchedRequest(
Promise<Response> promise;
Future<Response> future = promise.future();
batchedRequests.push_back(
- BatchedRequest{handler, request, approvers, std::move(promise)});
+ BatchedRequest{handler, queryParameters, approvers, std::move(promise)});
// Schedule processing of batched requests if not yet scheduled.
if (scheduleBatch) {
@@ -2413,12 +2417,12 @@ void Master::Http::processRequestsBatch() const
foreach (BatchedRequest& request, batchedRequests) {
request.promise.associate(process::async(
[this](ReadOnlyRequestHandler handler,
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) {
- return (readonlyHandler.*handler)(request, approvers);
+ return (readonlyHandler.*handler)(queryParameters, approvers);
},
request.handler,
- request.request,
+ request.queryParameters,
request.approvers));
}
@@ -2542,7 +2546,9 @@ Future<Response> Master::Http::stateSummary(
master->self(),
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
- &Master::ReadOnlyHandler::stateSummary, request, approvers);
+ &Master::ReadOnlyHandler::stateSummary,
+ request.url.query,
+ approvers);
}));
}
@@ -2593,7 +2599,7 @@ Future<Response> Master::Http::roles(
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
&Master::ReadOnlyHandler::roles,
- request,
+ request.url.query,
approvers);
}));
}
@@ -2968,7 +2974,9 @@ Future<Response> Master::Http::tasks(
master->self(),
[this, request](const Owned<ObjectApprovers>& approvers) {
return deferBatchedRequest(
- &Master::ReadOnlyHandler::tasks, request, approvers);
+ &Master::ReadOnlyHandler::tasks,
+ request.url.query,
+ approvers);
}));
}
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 99549ab..6230d1d 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1290,6 +1290,10 @@ private:
// Inner class used to namespace HTTP handlers that do not change the
// underlying master object.
//
+ // Endpoints served by this handler are only permitted to depend on
+ // the request query parameters and the authorization filters to
+ // make caching of responses possible.
+ //
// NOTE: Most member functions of this class are not routed directly but
// dispatched from their corresponding handlers in the outer `Http` class.
// This is because deciding whether an incoming request is read-only often
@@ -1302,32 +1306,32 @@ private:
// /frameworks
process::http::Response frameworks(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
// /roles
process::http::Response roles(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
// /slaves
process::http::Response slaves(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
// /state
process::http::Response state(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
// /state-summary
process::http::Response stateSummary(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
// /tasks
process::http::Response tasks(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
private:
@@ -1800,12 +1804,12 @@ private:
typedef process::http::Response
(Master::ReadOnlyHandler::*ReadOnlyRequestHandler)(
- const process::http::Request&,
+ const hashmap<std::string, std::string>&,
const process::Owned<ObjectApprovers>&) const;
process::Future<process::http::Response> deferBatchedRequest(
ReadOnlyRequestHandler handler,
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& queryParameters,
const process::Owned<ObjectApprovers>& approvers) const;
void processRequestsBatch() const;
@@ -1813,7 +1817,7 @@ private:
struct BatchedRequest
{
ReadOnlyRequestHandler handler;
- process::http::Request request;
+ hashmap<std::string, std::string> queryParameters;
process::Owned<ObjectApprovers> approvers;
process::Promise<process::http::Response> promise;
};
diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp
index 8895374..66d6160 100644
--- a/src/master/readonly_handler.cpp
+++ b/src/master/readonly_handler.cpp
@@ -637,11 +637,11 @@ private:
process::http::Response Master::ReadOnlyHandler::frameworks(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
IDAcceptor<FrameworkID> selectFrameworkId(
- request.url.query.get("framework_id"));
+ query.get("framework_id"));
// This lambda is consumed before the outer lambda
// returns, hence capture by reference is fine here.
@@ -690,7 +690,7 @@ process::http::Response Master::ReadOnlyHandler::frameworks(
writer->field("unregistered_frameworks", [](JSON::ArrayWriter*) {});
};
- return OK(jsonify(frameworks), request.url.query.get("jsonp"));
+ return OK(jsonify(frameworks), query.get("jsonp"));
}
@@ -738,7 +738,7 @@ JSON::Object model(
process::http::Response Master::ReadOnlyHandler::roles(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
JSON::Object object;
@@ -769,24 +769,24 @@ process::http::Response Master::ReadOnlyHandler::roles(
object.values["roles"] = std::move(array);
}
- return OK(object, request.url.query.get("jsonp"));
+ return OK(object, query.get("jsonp"));
}
process::http::Response Master::ReadOnlyHandler::slaves(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
- IDAcceptor<SlaveID> selectSlaveId(request.url.query.get("slave_id"));
+ IDAcceptor<SlaveID> selectSlaveId(query.get("slave_id"));
return process::http::OK(
jsonify(SlavesWriter(master->slaves, approvers, selectSlaveId)),
- request.url.query.get("jsonp"));
+ query.get("jsonp"));
}
process::http::Response Master::ReadOnlyHandler::state(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
const Master* master = this->master;
@@ -923,12 +923,12 @@ process::http::Response Master::ReadOnlyHandler::state(
writer->field("unregistered_frameworks", [](JSON::ArrayWriter*) {});
};
- return OK(jsonify(calculateState), request.url.query.get("jsonp"));
+ return OK(jsonify(calculateState), query.get("jsonp"));
}
process::http::Response Master::ReadOnlyHandler::stateSummary(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
const Master* master = this->master;
@@ -1070,7 +1070,7 @@ process::http::Response Master::ReadOnlyHandler::stateSummary(
});
};
- return OK(jsonify(stateSummary), request.url.query.get("jsonp"));
+ return OK(jsonify(stateSummary), query.get("jsonp"));
}
@@ -1119,21 +1119,21 @@ struct TaskComparator
process::http::Response Master::ReadOnlyHandler::tasks(
- const process::http::Request& request,
+ const hashmap<std::string, std::string>& query,
const process::Owned<ObjectApprovers>& approvers) const
{
// Get list options (limit and offset).
- Result<int> result = numify<int>(request.url.query.get("limit"));
+ Result<int> result = numify<int>(query.get("limit"));
size_t limit = result.isSome() ? result.get() : TASK_LIMIT;
- result = numify<int>(request.url.query.get("offset"));
+ result = numify<int>(query.get("offset"));
size_t offset = result.isSome() ? result.get() : 0;
- Option<string> order = request.url.query.get("order");
+ Option<string> order = query.get("order");
string _order = order.isSome() && (order.get() == "asc") ? "asc" : "des";
- Option<string> frameworkId = request.url.query.get("framework_id");
- Option<string> taskId = request.url.query.get("task_id");
+ Option<string> frameworkId = query.get("framework_id");
+ Option<string> taskId = query.get("task_id");
IDAcceptor<FrameworkID> selectFrameworkId(frameworkId);
IDAcceptor<TaskID> selectTaskId(taskId);
@@ -1223,7 +1223,7 @@ process::http::Response Master::ReadOnlyHandler::tasks(
});
};
- return OK(jsonify(tasksWriter), request.url.query.get("jsonp"));
+ return OK(jsonify(tasksWriter), query.get("jsonp"));
}
} // namespace master {
[mesos] 04/08: Exposed private data members for testing.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f961ed1af62e00abe014c3362ee6221c4d6bc4bc
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Wed Jan 9 14:29:52 2019 -0800
Exposed private data members for testing.
For the unit tests introduced in the subsequent
commit, access to the underlying master object
of `cluster::Master` and to the definition of the
class `Master::ReadOnlyHandler` are required.
Review: https://reviews.apache.org/r/69421/
---
src/master/master.hpp | 2 ++
src/tests/cluster.hpp | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/master/master.hpp b/src/master/master.hpp
index d5d33ff..8de7312 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1287,6 +1287,7 @@ private:
Master* master;
};
+public:
// Inner class used to namespace HTTP handlers that do not change the
// underlying master object.
//
@@ -1338,6 +1339,7 @@ private:
const Master* master;
};
+private:
// Inner class used to namespace HTTP route handlers (see
// master/http.cpp for implementations).
class Http
diff --git a/src/tests/cluster.hpp b/src/tests/cluster.hpp
index ad2b80e..c04ee14 100644
--- a/src/tests/cluster.hpp
+++ b/src/tests/cluster.hpp
@@ -134,15 +134,15 @@ public:
// registrar behaves identically to the normal registrar.
process::Owned<MockRegistrar> registrar;
+ // The underlying master object.
+ process::Owned<master::Master> master;
+
private:
Option<std::shared_ptr<process::RateLimiter>> slaveRemovalLimiter;
// Indicates whether or not authorization callbacks were set when this master
// was constructed.
bool authorizationCallbacksSet;
-
- // The underlying master object.
- process::Owned<master::Master> master;
};