[
https://issues.apache.org/jira/browse/MESOS-9336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16657585#comment-16657585
]
Benjamin Mahler commented on MESOS-9336:
----------------------------------------
Updated the master task label decorator to do as little copying as possible
given the current interface:
{noformat}
commit bc54d3825ed6b7d8d4015bd77df2cb41129b2926
Author: Benjamin Mahler <[email protected]>
Date: Fri Oct 19 13:25:08 2018 -0700
Eliminated excessive label copying in the task label decorator hook.
This eliminates copying of the task labels in three cases:
* In the master when mutating the task with the hook's labels.
* In the hook manager when mutating labels on a copy of the task.
* In the hook manager when returning the labels.
We still copy the entire task info once and then each hook
has to copy the labels, but eliminating these requires an
interface change.
Review: https://reviews.apache.org/r/69089
{noformat}
> Hook interfaces and implementation performs unnecessary protobuf copying.
> -------------------------------------------------------------------------
>
> Key: MESOS-9336
> URL: https://issues.apache.org/jira/browse/MESOS-9336
> Project: Mesos
> Issue Type: Improvement
> Components: modules
> Reporter: Benjamin Mahler
> Priority: Major
> Labels: performance
>
> The interfaces and implementation of hooks perform significant protobuf
> copying and would benefit from copy elimination. Consider the case of the
> master task label decorator:
> https://github.com/apache/mesos/blob/1.7.0/src/hook/manager.cpp#L108-L138
> {code}
> Labels HookManager::masterLaunchTaskLabelDecorator(
> const TaskInfo& taskInfo,
> const FrameworkInfo& frameworkInfo,
> const SlaveInfo& slaveInfo)
> {
> synchronized (mutex) {
> // We need a mutable copy of the task info and set the new
> // labels after each hook invocation. Otherwise, the last hook
> // will be the only effective hook setting the labels.
> TaskInfo taskInfo_ = taskInfo;
> foreachpair (const string& name, Hook* hook, availableHooks) {
> const Result<Labels> result =
> hook->masterLaunchTaskLabelDecorator(
> taskInfo_,
> frameworkInfo,
> slaveInfo);
> // NOTE: If the hook returns None(), the task labels won't be
> // changed.
> if (result.isSome()) {
> taskInfo_.mutable_labels()->CopyFrom(result.get());
> } else if (result.isError()) {
> LOG(WARNING) << "Master label decorator hook failed for module '"
> << name << "': " << result.error();
> }
> }
> return taskInfo_.labels();
> }
> }
> {code}
> https://github.com/apache/mesos/blob/1.7.0/src/master/master.cpp#L5619-L5634
> {code}
> foreach (
> TaskInfo& task, *message.mutable_task_group()->mutable_tasks()) {
> taskIds.insert(task.task_id());
> totalResources += task.resources();
> addTask(task, framework, slave);
> if (HookManager::hooksAvailable()) {
> // Set labels retrieved from label-decorator hooks.
> task.mutable_labels()->CopyFrom(
> HookManager::masterLaunchTaskLabelDecorator(
> task,
> framework->info,
> slave->info));
> }
> }
> {code}
> # We copy the entire task info.
> # We copy the labels of the task info twice for each label decorator hook
> (the hook itself has to copy all existing ones since the API will re-write
> the labels, and then we copy the result back into the task info).
> # We copy the labels back out to the caller.
> # The master then copies the labels back into the task.
> In the case of 1 hook, that's 1 copy of entire the task info (including
> labels) and 4 copies of only the labels.
> Without changing the interface, we can eliminate 3 of the label copying
> cases, and we would be left with 1 copy of the entire task info (including
> labels), and 1 copy of the labels (per hook).
> With changing the interface, we can achieve 0 copies. This can be done by
> making it a mutation interface, or making it truly a label decorator (labels
> can only be added) with the modification that hooks don't get to see the
> additional labels added by previous hooks. These would all be breaking
> changes that require adjusting hooks.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
