diff options
4 files changed, 36 insertions, 34 deletions
diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp index 01b8515400b..7d7153e8461 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp @@ -7,7 +7,6 @@ #include "replymerger.h" #include <vespa/document/util/stringutil.h> #include <vespa/documentapi/documentapi.h> -#include <vespa/messagebus/emptyreply.h> #include <vespa/vespalib/util/exceptions.h> #include <sstream> @@ -97,7 +96,7 @@ DocumentProtocol::createPolicy(const mbus::string &name, const mbus::string &par DocumentProtocol & DocumentProtocol::putRoutingPolicyFactory(const string &name, IRoutingPolicyFactory::SP factory) { - _routingPolicyRepository->putFactory(name, factory); + _routingPolicyRepository->putFactory(name, std::move(factory)); return *this; } @@ -140,7 +139,7 @@ DocumentProtocol & DocumentProtocol::putRoutableFactory(uint32_t type, IRoutableFactory::SP factory, const vespalib::VersionSpecification &version) { - _routableRepository->putFactory(version, type, factory); + _routableRepository->putFactory(version, type, std::move(factory)); return *this; } @@ -215,7 +214,7 @@ DocumentProtocol::merge(mbus::RoutingContext& ctx, ctx.setReply(ctx.getChildIterator().skip(okIdx).removeReply()); } else { assert(res.hasGeneratedReply()); - ctx.setReply(mbus::Reply::UP(res.releaseGeneratedReply().release())); + ctx.setReply(res.releaseGeneratedReply()); } } diff --git a/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp b/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp index 69c1b00295d..4bcf2619031 100644 --- a/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp @@ -13,30 +13,23 @@ namespace documentapi { ReplyMerger::ReplyMerger() : _error(), _ignored(), - _successReply(0), + _successReply(nullptr), _successIndex(0) { } -ReplyMerger::~ReplyMerger() {} +ReplyMerger::~ReplyMerger() = default; -ReplyMerger::Result::Result(uint32_t successIdx, - std::unique_ptr<mbus::Reply> generatedReply) +ReplyMerger::Result::Result(uint32_t successIdx, std::unique_ptr<mbus::Reply> generatedReply) : _generatedReply(std::move(generatedReply)), _successIdx(successIdx) { } -ReplyMerger::Result::Result(Result&& o) - : _generatedReply(std::move(o._generatedReply)), - _successIdx(o._successIdx) -{ -} - bool ReplyMerger::Result::hasGeneratedReply() const { - return (_generatedReply.get() != 0); + return (bool)_generatedReply; } bool @@ -53,7 +46,7 @@ ReplyMerger::Result::releaseGeneratedReply() } uint32_t -ReplyMerger::Result::getSuccessfulReplyIndex() +ReplyMerger::Result::getSuccessfulReplyIndex() const { assert(!hasGeneratedReply()); return _successIdx; @@ -70,7 +63,7 @@ ReplyMerger::merge(uint32_t idx, const mbus::Reply& r) } bool -ReplyMerger::resourceWasFound(const mbus::Reply& r) const +ReplyMerger::resourceWasFound(const mbus::Reply& r) { switch (r.getType()) { case DocumentProtocol::REPLY_REMOVEDOCUMENT: @@ -112,8 +105,8 @@ ReplyMerger::mergeAllReplyErrors(const mbus::Reply& r) if (handleReplyWithOnlyIgnoredErrors(r)) { return; } - if (!_error.get()) { - _error.reset(new mbus::EmptyReply()); + if ( ! _error ) { + _error = std::make_unique<mbus::EmptyReply>(); } for (uint32_t i = 0; i < r.getNumErrors(); ++i) { _error->addError(r.getError(i)); @@ -124,8 +117,8 @@ bool ReplyMerger::handleReplyWithOnlyIgnoredErrors(const mbus::Reply& r) { if (DocumentProtocol::hasOnlyErrorsOfType(r, DocumentProtocol::ERROR_MESSAGE_IGNORED)) { - if (!_ignored.get()) { - _ignored.reset(new mbus::EmptyReply()); + if ( ! _ignored) { + _ignored = std::make_unique<mbus::EmptyReply>(); } _ignored->addError(r.getError(0)); return true; @@ -136,7 +129,7 @@ ReplyMerger::handleReplyWithOnlyIgnoredErrors(const mbus::Reply& r) bool ReplyMerger::shouldReturnErrorReply() const { - if (_error.get()) { + if (_error) { return true; } return (_ignored.get() && !_successReply); @@ -145,7 +138,7 @@ ReplyMerger::shouldReturnErrorReply() const std::unique_ptr<mbus::Reply> ReplyMerger::releaseGeneratedErrorReply() { - if (_error.get()) { + if (_error) { return std::move(_error); } else { assert(_ignored.get()); @@ -156,13 +149,13 @@ ReplyMerger::releaseGeneratedErrorReply() bool ReplyMerger::successfullyMergedAtLeastOneReply() const { - return (_successReply != 0); + return (_successReply != nullptr); } ReplyMerger::Result -ReplyMerger::createEmptyReplyResult() const +ReplyMerger::createEmptyReplyResult() { - return Result(0u, std::unique_ptr<mbus::Reply>(new mbus::EmptyReply())); + return Result(0u, std::make_unique<mbus::EmptyReply>()); } ReplyMerger::Result diff --git a/documentapi/src/vespa/documentapi/messagebus/replymerger.h b/documentapi/src/vespa/documentapi/messagebus/replymerger.h index f9574d56c2c..b14bf6a5f73 100644 --- a/documentapi/src/vespa/documentapi/messagebus/replymerger.h +++ b/documentapi/src/vespa/documentapi/messagebus/replymerger.h @@ -14,15 +14,17 @@ public: std::unique_ptr<mbus::Reply> _generatedReply; uint32_t _successIdx; - Result(uint32_t successIdx, - std::unique_ptr<mbus::Reply> generatedReply); + Result(uint32_t successIdx, std::unique_ptr<mbus::Reply> generatedReply); public: - Result(Result&&); + Result(Result&& o) noexcept + : _generatedReply(std::move(o._generatedReply)), + _successIdx(o._successIdx) + { } bool hasGeneratedReply() const; bool isSuccessful() const; std::unique_ptr<mbus::Reply> releaseGeneratedReply(); - uint32_t getSuccessfulReplyIndex(); + uint32_t getSuccessfulReplyIndex() const; }; private: std::unique_ptr<mbus::Reply> _error; @@ -38,8 +40,8 @@ private: void setCurrentBestReply(uint32_t idx, const mbus::Reply& r); void updateStateWithSuccessfulReply(uint32_t idx, const mbus::Reply& r); bool successfullyMergedAtLeastOneReply() const; - Result createEmptyReplyResult() const; - bool resourceWasFound(const mbus::Reply& r) const; + static Result createEmptyReplyResult(); + static bool resourceWasFound(const mbus::Reply& r); public: ReplyMerger(); ~ReplyMerger(); diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp index e1909252c8f..ed396cd522e 100644 --- a/storage/src/vespa/storage/persistence/testandsethelper.cpp +++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp @@ -5,6 +5,7 @@ #include <vespa/storage/persistence/testandsethelper.h> #include <vespa/document/select/parser.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/vespalib/util/stringfmt.h> using namespace std::string_literals; @@ -45,6 +46,7 @@ TestAndSetHelper::TestAndSetHelper(PersistenceThread & thread, const api::TestAn _component(thread._env._component), _cmd(cmd), _docId(cmd.getDocumentId()), + _docTypePtr(nullptr), _missingDocumentImpliesMatch(missingDocumentImpliesMatch) { getDocumentType(); @@ -66,7 +68,10 @@ TestAndSetHelper::retrieveAndMatch(spi::Context & context) { if (result.hasDocument()) { auto docPtr = result.getDocumentPtr(); if (_docSelectionUp->contains(*docPtr) != document::select::Result::True) { - return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, "Condition did not match document"); + return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, + vespalib::make_string("Condition did not match document partition=%d, nodeIndex=%d bucket=%lx %s", + _thread._env._partition, _thread._env._nodeIndex, _cmd.getBucketId().getRawId(), + _cmd.hasBeenRemapped() ? "remapped" : "")); } // Document matches @@ -75,7 +80,10 @@ TestAndSetHelper::retrieveAndMatch(spi::Context & context) { return api::ReturnCode(); } - return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, "Document does not exist"); + return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, + vespalib::make_string("Document does not exist partition=%d, nodeIndex=%d bucket=%lx %s", + _thread._env._partition, _thread._env._nodeIndex, _cmd.getBucketId().getRawId(), + _cmd.hasBeenRemapped() ? "remapped" : "")); } } // storage |