Simplified master's `updateOperation` function. This replaces repetitive code by instead using a helper variable which allows us to avoid branching. We can then also replace a `Try<bool>` with a plain `bool` further simplifying the code.
Review: https://reviews.apache.org/r/65093/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/dd866f9f Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/dd866f9f Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/dd866f9f Branch: refs/heads/master Commit: dd866f9fd73858ef4d6369edddb4aa9a2f1c1a8f Parents: 53480e9 Author: Benjamin Bannier <[email protected]> Authored: Fri Jan 12 15:40:32 2018 -0800 Committer: Greg Mann <[email protected]> Committed: Fri Jan 12 16:53:50 2018 -0800 ---------------------------------------------------------------------- src/master/master.cpp | 48 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/dd866f9f/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index 8738e10..6965514 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -10293,37 +10293,8 @@ void Master::updateOperation( { CHECK_NOTNULL(operation); - const OperationStatus& status = update.status(); - - Option<OperationStatus> latestStatus; - if (update.has_latest_status()) { - latestStatus = update.latest_status(); - } - - // Whether the operation has just become terminated. - Option<bool> terminated; - - if (latestStatus.isSome()) { - terminated = - !protobuf::isTerminalState(operation->latest_status().state()) && - protobuf::isTerminalState(latestStatus->state()); - - // If the operation has already transitioned to a terminal state, - // do not update its state. - if (!protobuf::isTerminalState(operation->latest_status().state())) { - operation->mutable_latest_status()->CopyFrom(latestStatus.get()); - } - } else { - terminated = - !protobuf::isTerminalState(operation->latest_status().state()) && - protobuf::isTerminalState(status.state()); - - if (!protobuf::isTerminalState(operation->latest_status().state())) { - operation->mutable_latest_status()->CopyFrom(status); - } - } - - operation->add_statuses()->CopyFrom(status); + const OperationStatus& status = + update.has_latest_status() ? update.latest_status() : update.status(); LOG(INFO) << "Updating the state of operation '" << operation->info().id() << "' (uuid: " << update.operation_uuid() @@ -10331,9 +10302,20 @@ void Master::updateOperation( << " (latest state: " << operation->latest_status().state() << ", status update state: " << status.state() << ")"; - CHECK_SOME(terminated); + // Whether the operation has just become terminated. + const bool terminated = + !protobuf::isTerminalState(operation->latest_status().state()) && + protobuf::isTerminalState(status.state()); + + // If the operation has already transitioned to a terminal state, + // do not update its state. + if (!protobuf::isTerminalState(operation->latest_status().state())) { + operation->mutable_latest_status()->CopyFrom(status); + } + + operation->add_statuses()->CopyFrom(status); - if (!terminated.get()) { + if (!terminated) { return; }
