diff options
23 files changed, 103 insertions, 84 deletions
diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.h b/storage/src/vespa/storage/distributor/bucketdbupdater.h index b756c5ca7bb..7f2a9b30a33 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.h +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.h @@ -9,7 +9,6 @@ #include "operation_routing_snapshot.h" #include "outdated_nodes_map.h" #include <vespa/document/bucket/bucket.h> -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/storage/common/storagelink.h> diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.h b/storage/src/vespa/storage/distributor/operations/external/getoperation.h index 57e878d9e40..546cf7a7543 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.h @@ -6,6 +6,7 @@ #include <vespa/storage/distributor/operations/operation.h> #include <vespa/storage/bucketdb/bucketdatabase.h> #include <vespa/storageapi/messageapi/storagemessage.h> +#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storageframework/generic/clock/timer.h> #include <optional> diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.h b/storage/src/vespa/storage/distributor/operations/external/putoperation.h index 79b3dd74b82..61149839ed1 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.h @@ -3,7 +3,6 @@ #pragma once #include <vespa/storage/distributor/operations/sequenced_operation.h> -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storage/distributor/persistencemessagetracker.h> namespace document { diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index 8527ff0ffba..a98fbe98c38 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -2,7 +2,6 @@ #pragma once #include "newest_replica.h" -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storage/distributor/persistencemessagetracker.h> #include <vespa/storage/distributor/operations/sequenced_operation.h> #include <vespa/document/update/documentupdate.h> @@ -18,6 +17,7 @@ namespace storage { namespace api { class UpdateCommand; class CreateBucketReply; +class ReturnCode; } class UpdateMetricSet; diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h index 9d9bce9160b..2e69a52a644 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storage/distributor/persistencemessagetracker.h> namespace document { diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 616fd312723..0868f18e88a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -161,7 +161,7 @@ VisitorOperation::markOperationAsFailedDueToNodeError( result.getResult(), vespalib::make_string("[from content node %u] %s", fromFailingNodeIndex, - result.getMessage().c_str())); + vespalib::string(result.getMessage()).c_str())); } void diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp index 9a94a5a62ad..3bd2406cd85 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp @@ -78,7 +78,7 @@ RemoveBucketOperation::onReceiveInternal(const std::shared_ptr<api::StorageReply "%s on node %u: %s. Reinserting node into bucket db with %s", getBucketId().toString().c_str(), node, - rep->getResult().getMessage().c_str(), + vespalib::string(rep->getResult().getMessage()).c_str(), rep->getBucketInfo().toString().c_str()); _manager->getDistributorComponent().updateBucketDatabase( @@ -104,8 +104,7 @@ RemoveBucketOperation::onReceiveInternal(const std::shared_ptr<api::StorageReply void -RemoveBucketOperation::onReceive(DistributorMessageSender&, - const std::shared_ptr<api::StorageReply> &msg) +RemoveBucketOperation::onReceive(DistributorMessageSender&, const std::shared_ptr<api::StorageReply> &msg) { if (onReceiveInternal(msg)) { done(); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp index 57f8bc92316..508dcf13916 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp @@ -121,7 +121,7 @@ SplitOperation::onReceive(DistributorMessageSender&, const api::StorageReply::SP LOGBP(debug, "Split failed for %s: bucket not found. Storage and " "distributor bucket databases might be out of sync: %s", getBucketId().toString().c_str(), - rep.getResult().getMessage().c_str()); + vespalib::string(rep.getResult().getMessage()).c_str()); _ok = false; } else if (rep.getResult().isBusy()) { LOG(debug, "Split failed for %s, node was busy. Will retry later", diff --git a/storage/src/vespa/storage/distributor/operations/operation.h b/storage/src/vespa/storage/distributor/operations/operation.h index 5f4dd63c2f6..538f15d2bf2 100644 --- a/storage/src/vespa/storage/distributor/operations/operation.h +++ b/storage/src/vespa/storage/distributor/operations/operation.h @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/vdslib/state/nodetype.h> #include <vespa/storage/distributor/distributormessagesender.h> #include <vespa/storageframework/generic/clock/time.h> diff --git a/storage/src/vespa/storage/distributor/pendingmessagetracker.h b/storage/src/vespa/storage/distributor/pendingmessagetracker.h index 275eba7f20c..2dba244dc9f 100644 --- a/storage/src/vespa/storage/distributor/pendingmessagetracker.h +++ b/storage/src/vespa/storage/distributor/pendingmessagetracker.h @@ -6,7 +6,6 @@ #include <vespa/storageframework/generic/status/htmlstatusreporter.h> #include <vespa/storageframework/generic/component/componentregister.h> #include <vespa/storageframework/generic/component/component.h> -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/vespalib/stllike/hash_set.h> #include <boost/multi_index_container.hpp> diff --git a/storage/src/vespa/storage/distributor/sentmessagemap.cpp b/storage/src/vespa/storage/distributor/sentmessagemap.cpp index d74f52b2f86..6f1db429b97 100644 --- a/storage/src/vespa/storage/distributor/sentmessagemap.cpp +++ b/storage/src/vespa/storage/distributor/sentmessagemap.cpp @@ -3,6 +3,7 @@ #include "sentmessagemap.h" #include <vespa/storage/distributor/operations/operation.h> #include <sstream> +#include <set> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.map"); @@ -14,15 +15,13 @@ SentMessageMap::SentMessageMap() { } -SentMessageMap::~SentMessageMap() -{ -} +SentMessageMap::~SentMessageMap() = default; std::shared_ptr<Operation> SentMessageMap::pop() { - std::map<api::StorageMessage::Id, std::shared_ptr<Operation> >::iterator found = _map.begin(); + auto found = _map.begin(); if (found != _map.end()) { std::shared_ptr<Operation> retVal = found->second; @@ -36,11 +35,10 @@ SentMessageMap::pop() std::shared_ptr<Operation> SentMessageMap::pop(api::StorageMessage::Id id) { - std::map<api::StorageMessage::Id, std::shared_ptr<Operation> >::iterator found = _map.find(id); + auto found = _map.find(id); if (found != _map.end()) { - LOG(spam, "Found Id %" PRIu64 " in callback map: %p", id, - found->second.get()); + LOG(spam, "Found Id %" PRIu64 " in callback map: %p", id, found->second.get()); std::shared_ptr<Operation> retVal = found->second; _map.erase(found); @@ -55,8 +53,7 @@ SentMessageMap::pop(api::StorageMessage::Id id) void SentMessageMap::insert(api::StorageMessage::Id id, const std::shared_ptr<Operation> & callback) { - LOG(spam, "Inserting callback %p for message %" PRIu64 "", - callback.get(), id); + LOG(spam, "Inserting callback %p for message %" PRIu64 "", callback.get(), id); _map[id] = callback; } @@ -67,17 +64,11 @@ SentMessageMap::toString() const std::ostringstream ost; std::set<std::string> messages; - for (Map::const_iterator iter = _map.begin(); - iter != _map.end(); - ++iter) - { - messages.insert(iter->second.get()->toString()); + for (const auto & entry : _map) { + messages.insert(entry.second.get()->toString()); } - for (std::set<std::string>::const_iterator - it(messages.begin()), e(messages.end()); - it != e; ++it) - { - ost << *it << "\n"; + for (const auto & message : messages) { + ost << message << "\n"; } return ost.str(); diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 148763f8d30..2e65302ee3b 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -845,7 +845,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP if (cmd.getNodes().size() < 2) { LOG(debug, "Attempt to merge a single instance of a bucket"); - tracker->fail(ReturnCode::ILLEGAL_PARAMETERS, "Cannot merge a single copy"); + tracker->fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Cannot merge a single copy"); return tracker; } @@ -854,7 +854,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP for (uint16_t i = 0; i < cmd.getNodes().size(); ++i) { if (i == 0) { if (cmd.getNodes()[i].sourceOnly) { - tracker->fail(ReturnCode::ILLEGAL_PARAMETERS, + tracker->fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Attempted to merge a chain where the first node " "in the chain is source only."); return tracker; @@ -863,7 +863,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP if (!cmd.getNodes()[i].sourceOnly && cmd.getNodes()[i-1].sourceOnly) { - tracker->fail(ReturnCode::ILLEGAL_PARAMETERS, + tracker->fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Attempted to merge a chain where the source only " "copies are not in end of chain."); return tracker; @@ -874,7 +874,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP if (_env._fileStorHandler.isMerging(bucket.getBucket())) { const char* err = "A merge is already running on this bucket."; LOG(debug, "%s", err); - tracker->fail(ReturnCode::BUSY, err); + tracker->fail(api::ReturnCode::BUSY, err); return tracker; } checkResult(_spi.createBucket(bucket, tracker->context()), bucket, "create bucket"); @@ -891,7 +891,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP auto cmd2 = std::make_shared<api::GetBucketDiffCommand>(bucket.getBucket(), s->nodeList, s->maxTimestamp.getTime()); if (!buildBucketInfoList(bucket, s->maxTimestamp, 0, cmd2->getDiff(), tracker->context())) { LOG(debug, "Bucket non-existing in db. Failing merge."); - tracker->fail(ReturnCode::BUCKET_DELETED, "Bucket not found in buildBucketInfo step"); + tracker->fail(api::ReturnCode::BUCKET_DELETED, "Bucket not found in buildBucketInfo step"); return tracker; } _env._metrics.merge_handler_metrics.mergeMetadataReadLatency.addValue(s->startTime.getElapsedTimeAsDouble()); @@ -1053,7 +1053,7 @@ MergeHandler::handleGetBucketDiff(api::GetBucketDiffCommand& cmd, MessageTracker checkResult(_spi.createBucket(bucket, tracker->context()), bucket, "create bucket"); if (_env._fileStorHandler.isMerging(bucket.getBucket())) { - tracker->fail(ReturnCode::BUSY, "A merge is already running on this bucket."); + tracker->fail(api::ReturnCode::BUSY, "A merge is already running on this bucket."); return tracker; } uint8_t index = findOwnIndex(cmd.getNodes(), _env._nodeIndex); @@ -1065,7 +1065,7 @@ MergeHandler::handleGetBucketDiff(api::GetBucketDiffCommand& cmd, MessageTracker index, local, tracker->context())) { LOG(debug, "Bucket non-existing in db. Failing merge."); - tracker->fail(ReturnCode::BUCKET_DELETED, "Bucket not found in buildBucketInfo step"); + tracker->fail(api::ReturnCode::BUCKET_DELETED, "Bucket not found in buildBucketInfo step"); return tracker; } if (!mergeLists(remote, local, local)) { @@ -1234,8 +1234,7 @@ MergeHandler::handleApplyBucketDiff(api::ApplyBucketDiffCommand& cmd, MessageTra LOG(debug, "%s", cmd.toString().c_str()); if (_env._fileStorHandler.isMerging(bucket.getBucket())) { - tracker->fail(ReturnCode::BUSY, - "A merge is already running on this bucket."); + tracker->fail(api::ReturnCode::BUSY, "A merge is already running on this bucket."); return tracker; } @@ -1248,10 +1247,8 @@ MergeHandler::handleApplyBucketDiff(api::ApplyBucketDiffCommand& cmd, MessageTra } else { LOG(spam, "Merge(%s): Moving %zu entries, didn't need " "local data on node %u (%u).", - bucket.toString().c_str(), - cmd.getDiff().size(), - _env._nodeIndex, - index); + bucket.toString().c_str(), cmd.getDiff().size(), + _env._nodeIndex, index); } if (applyDiffHasLocallyNeededData(cmd.getDiff(), index)) { framework::MilliSecTimer startTime(_clock); diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 0449b21fc81..6e0004af88a 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -124,7 +124,7 @@ MessageTracker::checkForError(const spi::Result& response) } void -MessageTracker::fail(const ReturnCode& result) +MessageTracker::fail(const api::ReturnCode& result) { _result = result; LOG(debug, "Failing operation with error: %s", _result.toString().c_str()); diff --git a/storage/src/vespa/storage/persistence/persistenceutil.h b/storage/src/vespa/storage/persistence/persistenceutil.h index ddd5619b964..47233b0ded6 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.h +++ b/storage/src/vespa/storage/persistence/persistenceutil.h @@ -7,6 +7,7 @@ #include <vespa/storage/persistence/filestorage/filestorhandler.h> #include <vespa/storage/persistence/filestorage/filestormetrics.h> #include <vespa/storageframework/generic/clock/timer.h> +#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/storage/storageutil/utils.h> @@ -42,10 +43,10 @@ public: /** Utility function to be able to write a bit less in client. */ void fail(uint32_t result, const String& message = "") { - fail(ReturnCode((api::ReturnCode::Result)result, message)); + fail(api::ReturnCode((api::ReturnCode::Result)result, message)); } /** Set the request to fail with the given failure. */ - void fail(const ReturnCode&); + void fail(const api::ReturnCode&); /** Don't send reply for the command being processed. Used by multi chain * commands like merge. */ diff --git a/storage/src/vespa/storage/persistence/splitjoinhandler.cpp b/storage/src/vespa/storage/persistence/splitjoinhandler.cpp index a0fabca4890..c64a892d6fb 100644 --- a/storage/src/vespa/storage/persistence/splitjoinhandler.cpp +++ b/storage/src/vespa/storage/persistence/splitjoinhandler.cpp @@ -298,19 +298,19 @@ bool SplitJoinHandler::validateJoinCommand(const api::JoinBucketsCommand& cmd, MessageTracker& tracker) { if (cmd.getSourceBuckets().size() != 2) { - tracker.fail(ReturnCode::ILLEGAL_PARAMETERS, + tracker.fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Join needs exactly two buckets to be joined together" + cmd.getBucketId().toString()); return false; } // Verify that source and target buckets look sane. for (uint32_t i = 0; i < cmd.getSourceBuckets().size(); i++) { if (cmd.getSourceBuckets()[i] == cmd.getBucketId()) { - tracker.fail(ReturnCode::ILLEGAL_PARAMETERS, + tracker.fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Join had both source and target bucket " + cmd.getBucketId().toString()); return false; } if (!cmd.getBucketId().contains(cmd.getSourceBuckets()[i])) { - tracker.fail(ReturnCode::ILLEGAL_PARAMETERS, + tracker.fail(api::ReturnCode::ILLEGAL_PARAMETERS, "Source bucket " + cmd.getSourceBuckets()[i].toString() + " is not contained in target " + cmd.getBucketId().toString()); return false; diff --git a/storage/src/vespa/storage/persistence/types.h b/storage/src/vespa/storage/persistence/types.h index 6a73adc8b0c..b36155bde8f 100644 --- a/storage/src/vespa/storage/persistence/types.h +++ b/storage/src/vespa/storage/persistence/types.h @@ -7,7 +7,6 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/storage/bucketdb/storbucketdb.h> #include <vespa/storageapi/buckets/bucketinfo.h> -#include <vespa/storageapi/messageapi/returncode.h> #include <vespa/storageapi/defs.h> #include <vespa/vespalib/stllike/string.h> #include <vespa/storageframework/generic/clock/time.h> @@ -25,8 +24,6 @@ struct Types { typedef Timestamp RevertToken; typedef vespalib::string String; typedef api::BucketInfo BucketInfo; - typedef api::ReturnCode ReturnCode; - typedef StorBucketDatabase::WrappedEntry BucketDBEntry; using MessageTrackerUP = std::unique_ptr<MessageTracker>; static const framework::MicroSecTime MAX_TIMESTAMP; diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index 940bffdbcc6..bfbf2b11be3 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -660,7 +660,8 @@ CommunicationManager::sendDirectRPCReply( activate_reply.activateVersion(), activate_reply.actualVersion()); } else { request.addReturnInt(reply->getResult().getResult()); - request.addReturnString(reply->getResult().getMessage().c_str()); + vespalib::stringref m = reply->getResult().getMessage(); + request.addReturnString(m.data(), m.size()); if (reply->getType() == api::MessageType::GETNODESTATE_REPLY) { api::GetNodeStateReply& gns(static_cast<api::GetNodeStateReply&>(*reply)); diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 7225b0fd1ed..2e3c65ef70c 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -763,7 +763,7 @@ MergeThrottler::handleMessageUp( if (mergeReply.getResult().getResult() != api::ReturnCode::OK) { LOG(debug, "Merging failed for %s (%s)", mergeReply.toString().c_str(), - mergeReply.getResult().getMessage().c_str()); + vespalib::string(mergeReply.getResult().getMessage()).c_str()); } processMergeReply(msg, true, msgGuard); diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index c2ebffb23e8..01719fbd7ec 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -816,13 +816,13 @@ TEST_P(StorageProtocolTest, serialized_size_is_used_to_set_approx_size_of_storag } TEST_P(StorageProtocolTest, track_memory_footprint_for_some_messages) { - EXPECT_EQ(136u, sizeof(StorageReply)); - EXPECT_EQ(160u, sizeof(BucketReply)); - EXPECT_EQ(192u, sizeof(BucketInfoReply)); - EXPECT_EQ(336u, sizeof(PutReply)); - EXPECT_EQ(320u, sizeof(UpdateReply)); - EXPECT_EQ(312u, sizeof(RemoveReply)); - EXPECT_EQ(400u, sizeof(GetReply)); + EXPECT_EQ(80u, sizeof(StorageReply)); + EXPECT_EQ(104u, sizeof(BucketReply)); + EXPECT_EQ(136u, sizeof(BucketInfoReply)); + EXPECT_EQ(280u, sizeof(PutReply)); + EXPECT_EQ(264u, sizeof(UpdateReply)); + EXPECT_EQ(256u, sizeof(RemoveReply)); + EXPECT_EQ(344u, sizeof(GetReply)); EXPECT_EQ(80u, sizeof(StorageCommand)); EXPECT_EQ(104u, sizeof(BucketCommand)); EXPECT_EQ(104u, sizeof(BucketInfoCommand)); diff --git a/storageapi/src/vespa/storageapi/message/visitor.h b/storageapi/src/vespa/storageapi/message/visitor.h index 67c41a0cc4d..8440591ecde 100644 --- a/storageapi/src/vespa/storageapi/message/visitor.h +++ b/storageapi/src/vespa/storageapi/message/visitor.h @@ -205,18 +205,19 @@ public: VisitorInfoCommand(); ~VisitorInfoCommand() override; - void setErrorCode(const ReturnCode& code) { _error = code; } + void setErrorCode(ReturnCode && code) { _error = std::move(code); } void setCompleted() { _completed = true; } - void setBucketCompleted(const document::BucketId& id, Timestamp lastVisited) - { + void setBucketCompleted(const document::BucketId& id, Timestamp lastVisited) { _bucketsCompleted.push_back(BucketTimestampPair(id, lastVisited)); } - void setBucketsCompleted(const std::vector<BucketTimestampPair>& bc) - { _bucketsCompleted = bc; } + void setBucketsCompleted(const std::vector<BucketTimestampPair>& bc) { + _bucketsCompleted = bc; + } const ReturnCode& getErrorCode() const { return _error; } - const std::vector<BucketTimestampPair>& getCompletedBucketsList() const - { return _bucketsCompleted; } + const std::vector<BucketTimestampPair>& getCompletedBucketsList() const { + return _bucketsCompleted; + } bool visitorCompleted() const { return _completed; } void print(std::ostream& out, bool verbose, const std::string& indent) const override; diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp index d5c8cb7da68..f2bcb54ca41 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp @@ -10,17 +10,39 @@ ReturnCode::ReturnCode() _message() {} -ReturnCode::ReturnCode(const ReturnCode &) = default; -ReturnCode & ReturnCode::operator = (const ReturnCode &) = default; ReturnCode & ReturnCode::operator = (ReturnCode &&) noexcept = default; ReturnCode::~ReturnCode() = default; -ReturnCode::ReturnCode(Result result, vespalib::stringref msg) +ReturnCode::ReturnCode(Result result) : _result(result), - _message(msg) + _message() {} -vespalib::string ReturnCode::getResultString(Result result) { +ReturnCode::ReturnCode(Result result, vespalib::stringref msg) + : _result(result), + _message() +{ + if ( ! msg.empty()) { + _message = std::make_unique<vespalib::string>(msg); + } +} + +ReturnCode::ReturnCode(const ReturnCode & rhs) + : _result(rhs._result), + _message() +{ + if (rhs._message) { + _message = std::make_unique<vespalib::string>(*rhs._message); + } +} + +ReturnCode & +ReturnCode::operator = (const ReturnCode & rhs) { + return operator=(ReturnCode(rhs)); +} + +vespalib::string +ReturnCode::getResultString(Result result) { return documentapi::DocumentProtocol::getErrorName(result); } @@ -28,9 +50,9 @@ vespalib::string ReturnCode::toString() const { vespalib::string ret = "ReturnCode("; ret += getResultString(_result); - if ( ! _message.empty()) { + if ( _message && ! _message->empty()) { ret += ", "; - ret += _message; + ret += *_message; } ret += ")"; return ret; @@ -145,5 +167,21 @@ ReturnCode::isBucketDisappearance() const return false; } } +namespace { + vespalib::stringref Empty(""); +} +vespalib::stringref +ReturnCode::getMessage() const { + return _message ? _message->operator vespalib::stringref() : Empty; +} + +bool +ReturnCode::operator==(const ReturnCode& code) const { + return (_result == code._result) && (getMessage() == code.getMessage()); +} +bool +ReturnCode::operator!=(const ReturnCode& code) const { + return (_result != code._result) || (getMessage() != code.getMessage()); +} } diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.h b/storageapi/src/vespa/storageapi/messageapi/returncode.h index 0149ae29a05..bef59a334d9 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.h +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.h @@ -59,18 +59,18 @@ public: private: Result _result; - vespalib::string _message; + std::unique_ptr<vespalib::string> _message; public: ReturnCode(); - explicit ReturnCode(Result result, vespalib::stringref msg = ""); + explicit ReturnCode(Result result); + explicit ReturnCode(Result result, vespalib::stringref msg); ReturnCode(const ReturnCode &); ReturnCode & operator = (const ReturnCode &); ReturnCode(ReturnCode &&) noexcept = default; ReturnCode & operator = (ReturnCode &&) noexcept; ~ReturnCode(); - const vespalib::string& getMessage() const { return _message; } - void setMessage(vespalib::stringref message) { _message = message; } + vespalib::stringref getMessage() const; Result getResult() const { return _result; } @@ -85,10 +85,8 @@ public: bool operator==(Result res) const { return _result == res; } bool operator!=(Result res) const { return _result != res; } - bool operator==(const ReturnCode& code) const - { return _result == code._result && _message == code._message; } - bool operator!=(const ReturnCode& code) const - { return _result != code._result || _message != code._message; } + bool operator==(const ReturnCode& code) const; + bool operator!=(const ReturnCode& code) const; // To avoid lots of code matching various return codes in storage, we define // some functions they can use to match those codes that corresponds to what diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.h b/storageapi/src/vespa/storageapi/messageapi/storagereply.h index 1a3bbe35eb4..53516949110 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.h @@ -30,7 +30,7 @@ public: ~StorageReply() override; DECLARE_POINTER_TYPEDEFS(StorageReply); - void setResult(const ReturnCode& r) { _result = r; } + void setResult(ReturnCode r) { _result = std::move(r); } void setResult(ReturnCode::Result r) { _result = ReturnCode(r); } const ReturnCode& getResult() const { return _result; } void print(std::ostream& out, bool verbose, const std::string& indent) const override; |