diff options
author | Tor Brede Vekterli <vekterli@vespa.ai> | 2024-05-30 13:13:30 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@vespa.ai> | 2024-05-30 13:16:35 +0000 |
commit | 00ffba12feb1a327a5e4d734a9c4a7632c29d57f (patch) | |
tree | b50300c828be5b781be60692efd39f3edf32f55e | |
parent | 0371db9a7a46bbc80dc0420bc7bcec4bf5543f80 (diff) |
Less copying of cluster state bundles
Already decoded into a `shared_ptr` during RPC request
handling; might as well use it.
3 files changed, 30 insertions, 18 deletions
diff --git a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp index 4d74eb1974b..3b53c0c9584 100644 --- a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp @@ -124,7 +124,8 @@ void ClusterControllerApiRpcService::RPC_setSystemState2(FRT_RPCRequest* req) { req->GetParams()->GetValue(0)._string._len); lib::ClusterState systemState(systemStateStr); - auto cmd = std::make_shared<api::SetSystemStateCommand>(lib::ClusterStateBundle(systemState)); + auto bundle = std::make_shared<const lib::ClusterStateBundle>(systemState); + auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(bundle)); cmd->setPriority(api::StorageMessage::VERYHIGH); detach_and_forward_to_enqueuer(std::move(cmd), req); @@ -167,8 +168,7 @@ void ClusterControllerApiRpcService::RPC_setDistributionStates(FRT_RPCRequest* r } LOG(debug, "Got state bundle %s", state_bundle->toString().c_str()); - // TODO add constructor taking in shared_ptr directly instead? - auto cmd = std::make_shared<api::SetSystemStateCommand>(*state_bundle); + auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(state_bundle)); cmd->setPriority(api::StorageMessage::VERYHIGH); detach_and_forward_to_enqueuer(std::move(cmd), req); diff --git a/storage/src/vespa/storageapi/message/state.cpp b/storage/src/vespa/storageapi/message/state.cpp index 5a50167f584..b4e8655d783 100644 --- a/storage/src/vespa/storageapi/message/state.cpp +++ b/storage/src/vespa/storageapi/message/state.cpp @@ -5,8 +5,7 @@ #include <vespa/vdslib/state/clusterstate.h> #include <ostream> -namespace storage { -namespace api { +namespace storage::api { IMPLEMENT_COMMAND(GetNodeStateCommand, GetNodeStateReply) IMPLEMENT_REPLY(GetNodeStateReply) @@ -45,7 +44,7 @@ GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd) GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd, const lib::NodeState& state) : StorageReply(cmd), - _state(new lib::NodeState(state)) + _state(std::make_unique<lib::NodeState>(state)) { } @@ -64,23 +63,31 @@ GetNodeStateReply::print(std::ostream& out, bool verbose, } } +SetSystemStateCommand::SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state) + : StorageCommand(MessageType::SETSYSTEMSTATE), + _state(std::move(state)) +{ +} + SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterStateBundle& state) : StorageCommand(MessageType::SETSYSTEMSTATE), - _state(state) + _state(std::make_shared<const lib::ClusterStateBundle>(state)) { } SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterState& state) : StorageCommand(MessageType::SETSYSTEMSTATE), - _state(state) + _state(std::make_shared<const lib::ClusterStateBundle>(state)) { } +SetSystemStateCommand::~SetSystemStateCommand() = default; + void SetSystemStateCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "SetSystemStateCommand(" << *_state.getBaselineClusterState() << ")"; + out << "SetSystemStateCommand(" << *_state->getBaselineClusterState() << ")"; if (verbose) { out << " : "; StorageCommand::print(out, verbose, indent); @@ -89,7 +96,7 @@ SetSystemStateCommand::print(std::ostream& out, bool verbose, SetSystemStateReply::SetSystemStateReply(const SetSystemStateCommand& cmd) : StorageReply(cmd), - _state(cmd.getClusterStateBundle()) + _state(cmd.cluster_state_bundle_ptr()) { } @@ -138,5 +145,4 @@ void ActivateClusterStateVersionReply::print(std::ostream& out, bool verbose, } } -} // api -} // storage +} // storage::api diff --git a/storage/src/vespa/storageapi/message/state.h b/storage/src/vespa/storageapi/message/state.h index 900355b12a2..4aa4c8a8f31 100644 --- a/storage/src/vespa/storageapi/message/state.h +++ b/storage/src/vespa/storageapi/message/state.h @@ -61,13 +61,19 @@ public: * put/get/remove etx) */ class SetSystemStateCommand : public StorageCommand { - lib::ClusterStateBundle _state; + std::shared_ptr<const lib::ClusterStateBundle> _state; public: + explicit SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state); explicit SetSystemStateCommand(const lib::ClusterStateBundle &state); explicit SetSystemStateCommand(const lib::ClusterState &state); - const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); } - const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; } + ~SetSystemStateCommand() override; + + [[nodiscard]] const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); } + [[nodiscard]] const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; } + [[nodiscard]] std::shared_ptr<const lib::ClusterStateBundle> cluster_state_bundle_ptr() const noexcept { + return _state; + } void print(std::ostream& out, bool verbose, const std::string& indent) const override; DECLARE_STORAGECOMMAND(SetSystemStateCommand, onSetSystemState) @@ -80,14 +86,14 @@ public: * @brief Reply received after a SetSystemStateCommand. */ class SetSystemStateReply : public StorageReply { - lib::ClusterStateBundle _state; + std::shared_ptr<const lib::ClusterStateBundle> _state; public: explicit SetSystemStateReply(const SetSystemStateCommand& cmd); // Not serialized. Available locally - const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); } - const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; } + const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); } + const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; } void print(std::ostream& out, bool verbose, const std::string& indent) const override; DECLARE_STORAGEREPLY(SetSystemStateReply, onSetSystemStateReply) |