This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new 932a043 Eliminated copying of the suppressed roles during framework
subscribe.
932a043 is described below
commit 932a043f4bd304cacb77fc429f267f2c0e670357
Author: Andrei Sekretenko <asekrete...@mesosphere.io>
AuthorDate: Thu May 9 17:30:09 2019 -0400
Eliminated copying of the suppressed roles during framework subscribe.
This patch moves the suppressed roles through rather than copying
them as before. The type passed through has also been updated to
`RepeatedPtrField<string>` rather than `set<string>` to allow for
validation in https://reviews.apache.org/r/70531/.
Review: https://reviews.apache.org/r/70583/
---
src/master/http.cpp | 2 +-
src/master/master.cpp | 44 +++++++++++++++++++++++---------------------
src/master/master.hpp | 8 ++++----
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 765bbf1..c2c7b9b 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -602,7 +602,7 @@ Future<Response> Master::Http::scheduler(
StreamingHttpConnection<v1::scheduler::Event> http(
pipe.writer(), acceptType, streamId);
- master->subscribe(http, call.subscribe());
+ master->subscribe(http, std::move(*call.mutable_subscribe()));
return ok;
}
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 437cbca..b16ce8a 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -21,6 +21,7 @@
#include <fstream>
#include <functional>
#include <iomanip>
+#include <iterator>
#include <list>
#include <memory>
#include <set>
@@ -99,6 +100,7 @@
#include "watcher/whitelist_watcher.hpp"
using std::list;
+using std::make_move_iterator;
using std::reference_wrapper;
using std::set;
using std::shared_ptr;
@@ -2378,7 +2380,7 @@ void Master::receive(
}
if (call.type() == scheduler::Call::SUBSCRIBE) {
- subscribe(from, call.subscribe());
+ subscribe(from, std::move(*call.mutable_subscribe()));
return;
}
@@ -2518,7 +2520,7 @@ void Master::registerFramework(
scheduler::Call::Subscribe call;
*call.mutable_framework_info() = std::move(frameworkInfo);
- subscribe(from, call);
+ subscribe(from, std::move(call));
}
@@ -2546,7 +2548,7 @@ void Master::reregisterFramework(
*call.mutable_framework_info() = std::move(frameworkInfo);
call.set_force(reregisterFrameworkMessage.failover());
- subscribe(from, call);
+ subscribe(from, std::move(call));
}
@@ -2654,7 +2656,7 @@ Option<Error> Master::validateFrameworkSubscription(
void Master::subscribe(
StreamingHttpConnection<v1::scheduler::Event> http,
- const scheduler::Call::Subscribe& subscribe)
+ scheduler::Call::Subscribe&& subscribe)
{
// TODO(anand): Authenticate the framework.
@@ -2686,16 +2688,12 @@ void Master::subscribe(
return;
}
- set<string> suppressedRoles = set<string>(
- subscribe.suppressed_roles().begin(),
- subscribe.suppressed_roles().end());
-
// Need to disambiguate for the compiler.
void (Master::*_subscribe)(
StreamingHttpConnection<v1::scheduler::Event>,
const FrameworkInfo&,
bool,
- const set<string>&,
+ google::protobuf::RepeatedPtrField<string>&&,
const Future<bool>&) = &Self::_subscribe;
authorizeFramework(frameworkInfo)
@@ -2704,7 +2702,7 @@ void Master::subscribe(
http,
frameworkInfo,
subscribe.force(),
- suppressedRoles,
+ std::move(*subscribe.mutable_suppressed_roles()),
lambda::_1));
}
@@ -2713,7 +2711,7 @@ void Master::_subscribe(
StreamingHttpConnection<v1::scheduler::Event> http,
const FrameworkInfo& frameworkInfo,
bool force,
- const set<string>& suppressedRoles,
+ google::protobuf::RepeatedPtrField<string>&& suppressedRolesField,
const Future<bool>& authorized)
{
CHECK(!authorized.isDiscarded());
@@ -2746,6 +2744,10 @@ void Master::_subscribe(
<< (frameworkInfo.checkpoint() ? "enabled" : "disabled")
<< " and capabilities " << frameworkInfo.capabilities();
+ set<string> suppressedRoles = set<string>(
+ make_move_iterator(suppressedRolesField.begin()),
+ make_move_iterator(suppressedRolesField.end()));
+
if (!frameworkInfo.has_id() || frameworkInfo.id() == "") {
// If we are here the framework is subscribing for the first time.
// Assign a new FrameworkID.
@@ -2843,7 +2845,7 @@ void Master::_subscribe(
void Master::subscribe(
const UPID& from,
- const scheduler::Call::Subscribe& subscribe)
+ scheduler::Call::Subscribe&& subscribe)
{
FrameworkInfo frameworkInfo = subscribe.framework_info();
@@ -2865,11 +2867,11 @@ void Master::subscribe(
<< " because authentication is still in progress";
// Need to disambiguate for the compiler.
- void (Master::*f)(const UPID&, const scheduler::Call::Subscribe&)
+ void (Master::*f)(const UPID&, scheduler::Call::Subscribe&&)
= &Self::subscribe;
authenticating[from]
- .onReady(defer(self(), f, from, subscribe));
+ .onReady(defer(self(), f, from, std::move(subscribe)));
return;
}
@@ -2908,16 +2910,12 @@ void Master::subscribe(
frameworkInfo.set_principal(authenticated[from]);
}
- set<string> suppressedRoles = set<string>(
- subscribe.suppressed_roles().begin(),
- subscribe.suppressed_roles().end());
-
// Need to disambiguate for the compiler.
void (Master::*_subscribe)(
const UPID&,
const FrameworkInfo&,
bool,
- const set<string>&,
+ google::protobuf::RepeatedPtrField<string>&&,
const Future<bool>&) = &Self::_subscribe;
authorizeFramework(frameworkInfo)
@@ -2926,7 +2924,7 @@ void Master::subscribe(
from,
frameworkInfo,
subscribe.force(),
- suppressedRoles,
+ std::move(*subscribe.mutable_suppressed_roles()),
lambda::_1));
}
@@ -2935,7 +2933,7 @@ void Master::_subscribe(
const UPID& from,
const FrameworkInfo& frameworkInfo,
bool force,
- const set<string>& suppressedRoles,
+ google::protobuf::RepeatedPtrField<string>&& suppressedRolesField,
const Future<bool>& authorized)
{
CHECK(!authorized.isDiscarded());
@@ -2977,6 +2975,10 @@ void Master::_subscribe(
return;
}
+ set<string> suppressedRoles = set<string>(
+ make_move_iterator(suppressedRolesField.begin()),
+ make_move_iterator(suppressedRolesField.end()));
+
LOG(INFO) << "Subscribing framework " << frameworkInfo.name()
<< " with checkpointing "
<< (frameworkInfo.checkpoint() ? "enabled" : "disabled")
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 7d9732f..c5ee179 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1023,24 +1023,24 @@ private:
void subscribe(
StreamingHttpConnection<v1::scheduler::Event> http,
- const mesos::scheduler::Call::Subscribe& subscribe);
+ mesos::scheduler::Call::Subscribe&& subscribe);
void _subscribe(
StreamingHttpConnection<v1::scheduler::Event> http,
const FrameworkInfo& frameworkInfo,
bool force,
- const std::set<std::string>& suppressedRoles,
+ google::protobuf::RepeatedPtrField<std::string>&& suppressedRoles,
const process::Future<bool>& authorized);
void subscribe(
const process::UPID& from,
- const mesos::scheduler::Call::Subscribe& subscribe);
+ mesos::scheduler::Call::Subscribe&& subscribe);
void _subscribe(
const process::UPID& from,
const FrameworkInfo& frameworkInfo,
bool force,
- const std::set<std::string>& suppressedRoles,
+ google::protobuf::RepeatedPtrField<std::string>&& suppressedRoles,
const process::Future<bool>& authorized);
// Subscribes a client to the 'api/vX' endpoint.
