diff options
Diffstat (limited to 'storage')
16 files changed, 86 insertions, 109 deletions
diff --git a/storage/src/tests/visiting/memory_bounded_trace_test.cpp b/storage/src/tests/visiting/memory_bounded_trace_test.cpp index 543531ef55a..e43fa702fb5 100644 --- a/storage/src/tests/visiting/memory_bounded_trace_test.cpp +++ b/storage/src/tests/visiting/memory_bounded_trace_test.cpp @@ -32,12 +32,12 @@ TEST(MemoryBoundedTraceTest, memory_used_is_accumulated_recursively_for_non_leaf TEST(MemoryBoundedTraceTest, trace_nodes_can_be_moved_and_implicitly_cleared) { MemoryBoundedTrace trace(100); EXPECT_TRUE(trace.add(mbus::TraceNode("hello world", epoch))); - mbus::TraceNode target; + mbus::Trace target; trace.moveTraceTo(target); EXPECT_EQ(1, target.getNumChildren()); EXPECT_EQ(0, trace.getApproxMemoryUsed()); - mbus::TraceNode emptinessCheck; + mbus::Trace emptinessCheck; trace.moveTraceTo(emptinessCheck); EXPECT_EQ(0, emptinessCheck.getNumChildren()); } @@ -52,7 +52,7 @@ TEST(MemoryBoundedTraceTest, trace_nodes_can_be_moved_and_implicitly_cleared) { TEST(MemoryBoundedTraceTest, moved_trace_tree_is_marked_as_strict) { MemoryBoundedTrace trace(100); EXPECT_TRUE(trace.add(mbus::TraceNode("hello world", epoch))); - mbus::TraceNode target; + mbus::Trace target; trace.moveTraceTo(target); EXPECT_EQ(1, target.getNumChildren()); EXPECT_TRUE(target.getChild(0).isStrict()); @@ -69,7 +69,7 @@ TEST(MemoryBoundedTraceTest, can_not_add_more_nodes_when_memory_used_exceeds_upp "the freeway", epoch))); EXPECT_EQ(11, trace.getApproxMemoryUsed()); - mbus::TraceNode target; + mbus::Trace target; trace.moveTraceTo(target); // Twice nested node (root -> added trace tree -> leaf with txt). EXPECT_EQ(1, target.getNumChildren()); @@ -82,7 +82,7 @@ TEST(MemoryBoundedTraceTest, moved_tree_includes_stats_node_when_nodes_omitted) EXPECT_TRUE(trace.add(mbus::TraceNode("abcdef", epoch))); EXPECT_FALSE(trace.add(mbus::TraceNode("ghijkjlmn", epoch))); - mbus::TraceNode target; + mbus::Trace target; trace.moveTraceTo(target); EXPECT_EQ(1, target.getNumChildren()); EXPECT_EQ(2, target.getChild(0).getNumChildren()); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 72668564345..f727cdf8eb2 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -799,15 +799,16 @@ TEST_F(VisitorTest, no_mbus_tracing_if_trace_level_is_zero) { cmd->getTrace().setLevel(0); std::shared_ptr<api::CreateVisitorReply> reply; ASSERT_NO_FATAL_FAILURE(doCompleteVisitingSession(cmd, reply)); - EXPECT_TRUE(reply->getTrace().getRoot().isEmpty()); + EXPECT_TRUE(reply->getTrace().isEmpty()); } TEST_F(VisitorTest, reply_contains_trace_if_trace_level_above_zero) { std::shared_ptr<api::CreateVisitorCommand> cmd(makeCreateVisitor()); cmd->getTrace().setLevel(1); + cmd->getTrace().trace(1,"at least one trace."); std::shared_ptr<api::CreateVisitorReply> reply; ASSERT_NO_FATAL_FAILURE(doCompleteVisitingSession(cmd, reply)); - EXPECT_FALSE(reply->getTrace().getRoot().isEmpty()); + EXPECT_FALSE(reply->getTrace().isEmpty()); } TEST_F(VisitorTest, no_more_iterators_sent_while_memory_used_above_limit) { diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp index c4fb9b8228c..c72e5731a59 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -144,9 +144,7 @@ GetOperation::onReceive(DistributorMessageSender& sender, const std::shared_ptr< LOG(debug, "Received %s", msg->toString(true).c_str()); - if ( ! getreply->getTrace().getRoot().isEmpty()) { - _msg->getTrace().getRoot().addChild(getreply->getTrace().getRoot()); - } + _msg->getTrace().addChild(getreply->steal_trace()); bool allDone = true; for (auto& response : _responses) { for (uint32_t i = 0; i < response.second.size(); i++) { diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 3866ee4e6f7..d7dd0be7f4f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -27,21 +27,22 @@ TwoPhaseUpdateOperation::TwoPhaseUpdateOperation( DistributorMetricSet& metrics, SequencingHandle sequencingHandle) : SequencedOperation(std::move(sequencingHandle)), - _updateMetric(metrics.updates[msg->getLoadType()]), - _putMetric(metrics.update_puts[msg->getLoadType()]), - _getMetric(metrics.update_gets[msg->getLoadType()]), - _metadata_get_metrics(metrics.update_metadata_gets[msg->getLoadType()]), - _updateCmd(std::move(msg)), - _updateReply(), - _manager(manager), - _bucketSpace(bucketSpace), - _sendState(SendState::NONE_SENT), - _mode(Mode::FAST_PATH), - _single_get_latency_timer(), - _fast_path_repair_source_node(0xffff), - _use_initial_cheap_metadata_fetch_phase( + _updateMetric(metrics.updates[msg->getLoadType()]), + _putMetric(metrics.update_puts[msg->getLoadType()]), + _getMetric(metrics.update_gets[msg->getLoadType()]), + _metadata_get_metrics(metrics.update_metadata_gets[msg->getLoadType()]), + _updateCmd(std::move(msg)), + _updateReply(), + _manager(manager), + _bucketSpace(bucketSpace), + _sendState(SendState::NONE_SENT), + _mode(Mode::FAST_PATH), + _trace(_updateCmd->getTrace().getLevel()), + _single_get_latency_timer(), + _fast_path_repair_source_node(0xffff), + _use_initial_cheap_metadata_fetch_phase( _manager.getDistributor().getConfig().enable_metadata_only_fetch_phase_for_inconsistent_updates()), - _replySent(false) + _replySent(false) { document::BucketIdFactory idFactory; _updateDocBucketId = idFactory.getBucketId(_updateCmd->getDocumentId()); @@ -132,9 +133,7 @@ TwoPhaseUpdateOperation::sendReply( std::shared_ptr<api::StorageReply>& reply) { assert(!_replySent); - if (!_trace.isEmpty()) { - reply->getTrace().getRoot().addChild(_trace); - } + reply->getTrace().addChild(std::move(_trace)); sender.sendReply(reply); _replySent = true; } @@ -645,11 +644,9 @@ TwoPhaseUpdateOperation::satisfiesUpdateTimestampConstraint(api::Timestamp ts) c } void -TwoPhaseUpdateOperation::addTraceFromReply(const api::StorageReply& reply) +TwoPhaseUpdateOperation::addTraceFromReply(api::StorageReply & reply) { - if ( ! reply.getTrace().getRoot().isEmpty()) { - _trace.addChild(reply.getTrace().getRoot()); - } + _trace.addChild(reply.steal_trace()); } void diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index 2d8f3e8494d..8527ff0ffba 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -125,7 +125,7 @@ private: DistributorMessageSender& sender, const document::Document& candidateDoc); bool satisfiesUpdateTimestampConstraint(api::Timestamp) const; - void addTraceFromReply(const api::StorageReply& reply); + void addTraceFromReply(api::StorageReply& reply); bool hasTasCondition() const noexcept; void replyWithTasFailure(DistributorMessageSender& sender, vespalib::stringref message); @@ -146,7 +146,7 @@ private: SentMessageMap _sentMessageMap; SendState _sendState; Mode _mode; - mbus::TraceNode _trace; + mbus::Trace _trace; document::BucketId _updateDocBucketId; std::vector<std::pair<document::BucketId, uint16_t>> _replicas_at_get_send_time; std::optional<framework::MilliSecTimer> _single_get_latency_timer; diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 5a03e05d563..616fd312723 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -171,7 +171,7 @@ VisitorOperation::onReceive( { api::CreateVisitorReply& reply = static_cast<api::CreateVisitorReply&>(*r); - _trace.add(reply.getTrace().getRoot()); + _trace.add(reply.steal_trace()); SentMessagesMap::iterator iter = _sentMessages.find(reply.getMsgId()); assert(iter != _sentMessages.end()); @@ -828,8 +828,8 @@ VisitorOperation::sendReply(const api::ReturnCode& code, DistributorMessageSende { if (!_sentReply) { // Send create visitor reply - api::CreateVisitorReply::SP reply(new api::CreateVisitorReply(*_msg)); - _trace.moveTraceTo(reply->getTrace().getRoot()); + auto reply = std::make_shared<api::CreateVisitorReply>(*_msg); + _trace.moveTraceTo(reply->getTrace()); reply->setLastBucket(getLastBucketVisited()); reply->setResult(code); diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index 42b0bd56b9e..24c7157b481 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -38,7 +38,7 @@ public: const Config& config, VisitorMetricSet& metrics); - ~VisitorOperation(); + ~VisitorOperation() override; void onClose(DistributorMessageSender& sender) override; void onStart(DistributorMessageSender& sender) override; diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index 6362db6c002..584cb379400 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -22,6 +22,7 @@ PersistenceMessageTrackerImpl::PersistenceMessageTrackerImpl( _reply(std::move(reply)), _manager(link), _revertTimestamp(revertTimestamp), + _trace(_reply->getTrace().getLevel()), _requestTimer(link.getClock()), _n_persistence_replies_total(0), _n_successful_persistence_replies(0), @@ -163,25 +164,18 @@ PersistenceMessageTrackerImpl::addBucketInfoFromReply( bucket.toString().c_str(), bucketInfo.toString().c_str(), node); - _remapBucketInfo[bucket].push_back( - BucketCopy(_manager.getUniqueTimestamp(), - node, - bucketInfo)); + _remapBucketInfo[bucket].emplace_back(_manager.getUniqueTimestamp(), node, bucketInfo); } else { LOG(debug, "Bucket %s: Received bucket info %s from node %d", bucket.toString().c_str(), bucketInfo.toString().c_str(), node); - _bucketInfo[bucket].push_back( - BucketCopy(_manager.getUniqueTimestamp(), - node, - bucketInfo)); + _bucketInfo[bucket].emplace_back(_manager.getUniqueTimestamp(), node, bucketInfo); } } void -PersistenceMessageTrackerImpl::logSuccessfulReply(uint16_t node, - const api::BucketInfoReply& reply) const +PersistenceMessageTrackerImpl::logSuccessfulReply(uint16_t node, const api::BucketInfoReply& reply) const { LOG(spam, "Bucket %s: Received successful reply %s", reply.getBucketId().toString().c_str(), @@ -230,7 +224,7 @@ PersistenceMessageTrackerImpl::sendReply(MessageSender& sender) updateMetrics(); _trace.setStrict(false); if ( ! _trace.isEmpty()) { - _reply->getTrace().getRoot().addChild(_trace); + _reply->getTrace().addChild(std::move(_trace)); } sender.sendReply(_reply); @@ -292,9 +286,7 @@ PersistenceMessageTrackerImpl::updateFromReply( api::BucketInfoReply& reply, uint16_t node) { - if ( ! reply.getTrace().getRoot().isEmpty()) { - _trace.addChild(reply.getTrace().getRoot()); - } + _trace.addChild(reply.steal_trace()); if (reply.getType() == api::MessageType::CREATEBUCKET_REPLY) { handleCreateBucketReply(reply, node); diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h index 4392aa1fc30..cf9f4017eda 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h @@ -73,7 +73,7 @@ private: DistributorComponent& _manager; api::Timestamp _revertTimestamp; std::vector<BucketNodePair> _revertNodes; - mbus::TraceNode _trace; + mbus::Trace _trace; framework::MilliSecTimer _requestTimer; uint32_t _n_persistence_replies_total; uint32_t _n_successful_persistence_replies; diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index cba3969cd68..54c1c7606b2 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -676,7 +676,7 @@ FileStorManager::onInternal(const shared_ptr<api::InternalCommand>& msg) spi::Context context(msg->getLoadType(), msg->getPriority(), msg->getTrace().getLevel()); shared_ptr<DestroyIteratorCommand> cmd(std::static_pointer_cast<DestroyIteratorCommand>(msg)); _provider->destroyIterator(cmd->getIteratorId(), context); - msg->getTrace().getRoot().addChild(context.getTrace().getRoot()); + msg->getTrace().addChild(context.steal_trace()); return true; } case ReadBucketList::ID: diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 42d67573763..9a7107fb583 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -93,9 +93,7 @@ MessageTracker::sendReply() { _msg->toString(true).c_str(), vespalib::to_s(duration)); } if (hasReply()) { - if ( ! _context.getTrace().getRoot().isEmpty()) { - getReply().getTrace().getRoot().addChild(_context.getTrace().getRoot()); - } + getReply().getTrace().addChild(_context.steal_trace()); if (_updateBucketInfo) { if (getReply().getResult().success()) { _env.setBucketInfo(*this, _bucketLock->getBucket()); @@ -104,13 +102,10 @@ MessageTracker::sendReply() { if (getReply().getResult().success()) { _metric->latency.addValue(_timer.getElapsedTimeAsDouble()); } - LOG(spam, "Sending reply up: %s %" PRIu64, - getReply().toString().c_str(), getReply().getMsgId()); + LOG(spam, "Sending reply up: %s %" PRIu64, getReply().toString().c_str(), getReply().getMsgId()); _replySender.sendReplyDirectly(std::move(_reply)); } else { - if ( ! _context.getTrace().getRoot().isEmpty()) { - _msg->getTrace().getRoot().addChild(_context.getTrace().getRoot()); - } + _msg->getTrace().addChild(_context.steal_trace()); } } diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index a35b9d1d59a..940bffdbcc6 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -102,7 +102,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg) return; } - cmd->setTrace(docMsgPtr->getTrace()); + cmd->setTrace(docMsgPtr->steal_trace()); cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(docMsgPtr))); enqueue_or_process(std::move(cmd)); @@ -114,7 +114,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg) //TODO: Can it be moved ? std::shared_ptr<api::StorageCommand> cmd = storMsgPtr->getCommand(); cmd->setTimeout(storMsgPtr->getTimeRemaining()); - cmd->setTrace(storMsgPtr->getTrace()); + cmd->setTrace(storMsgPtr->steal_trace()); cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(storMsgPtr))); enqueue_or_process(std::move(cmd)); @@ -203,12 +203,12 @@ CommunicationManager::handleReply(std::unique_ptr<mbus::Reply> reply) _docApiConverter.toStorageAPI(static_cast<documentapi::DocumentReply&>(*reply), *originalCommand)); if (sar) { - sar->setTrace(reply->getTrace()); + sar->setTrace(reply->steal_trace()); receiveStorageReply(sar); } } else if (protocolName == mbusprot::StorageProtocol::NAME) { mbusprot::StorageReply* sr(static_cast<mbusprot::StorageReply*>(reply.get())); - sr->getReply()->setTrace(reply->getTrace()); + sr->getReply()->setTrace(reply->steal_trace()); receiveStorageReply(sr->getReply()); } else { LOGBM(warning, "Received unsupported reply type %d for protocol '%s'.", @@ -583,7 +583,7 @@ CommunicationManager::sendCommand( cmd->setContext(mbus::Context(msg->getMsgId())); cmd->setRetryEnabled(false); cmd->setTimeRemaining(msg->getTimeout()); - cmd->setTrace(msg->getTrace()); + cmd->setTrace(msg->steal_trace()); sendMessageBusMessage(msg, std::move(cmd), address.getRoute()); } break; @@ -596,7 +596,7 @@ CommunicationManager::sendCommand( if (mbusMsg) { MBUS_TRACE(msg->getTrace(), 7, "Communication manager: Converted OK"); - mbusMsg->setTrace(msg->getTrace()); + mbusMsg->setTrace(msg->steal_trace()); mbusMsg->setRetryEnabled(false); { @@ -690,13 +690,13 @@ CommunicationManager::sendMessageBusReply( if (reply->getResult().getResult() == api::ReturnCode::WRONG_DISTRIBUTION) { replyUP = std::make_unique<documentapi::WrongDistributionReply>(reply->getResult().getMessage()); replyUP->swapState(*context._docAPIMsg); - replyUP->setTrace(reply->getTrace()); + replyUP->setTrace(reply->steal_trace()); replyUP->addError(mbus::Error(documentapi::DocumentProtocol::ERROR_WRONG_DISTRIBUTION, reply->getResult().getMessage())); } else { replyUP = context._docAPIMsg->createReply(); replyUP->swapState(*context._docAPIMsg); - replyUP->setTrace(reply->getTrace()); + replyUP->setTrace(reply->steal_trace()); replyUP->setMessage(std::move(context._docAPIMsg)); _docApiConverter.transferReplyState(*reply, *replyUP); } @@ -707,7 +707,7 @@ CommunicationManager::sendMessageBusReply( } replyUP->swapState(*context._storageProtocolMsg); - replyUP->setTrace(reply->getTrace()); + replyUP->setTrace(reply->steal_trace()); replyUP->setMessage(std::move(context._storageProtocolMsg)); } diff --git a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp index cb1d0380bf1..c465315a5a6 100644 --- a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp @@ -208,7 +208,7 @@ void StorageApiRpcService::encode_rpc_v1_response(FRT_RPCRequest& request, api:: // TODO skip encoding header altogether if no relevant fields set? protobuf::ResponseHeader hdr; if (reply.getTrace().getLevel() > 0) { - hdr.set_trace_payload(reply.getTrace().getRoot().encode()); + hdr.set_trace_payload(reply.getTrace().encode()); } // TODO consistent naming... encode_header_into_rpc_params(hdr, *ret); @@ -289,7 +289,7 @@ void StorageApiRpcService::RequestDone(FRT_RPCRequest* raw_req) { assert(reply); if (!hdr.trace_payload().empty()) { - cmd.getTrace().getRoot().addChild(mbus::TraceNode::decode(hdr.trace_payload())); + cmd.getTrace().addChild(mbus::TraceNode::decode(hdr.trace_payload())); } if (cmd.getTrace().shouldTrace(TraceLevel::SEND_RECEIVE)) { cmd.getTrace().trace(TraceLevel::SEND_RECEIVE, diff --git a/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp b/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp index 9f9eb2942c1..05bb026f238 100644 --- a/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp +++ b/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp @@ -6,7 +6,7 @@ namespace storage { MemoryBoundedTrace::MemoryBoundedTrace(size_t softMemoryUpperBound) - : _node(), + : _trace(), _currentMemoryUsed(0), _omittedNodes(0), _omittedBytes(0), @@ -14,53 +14,49 @@ MemoryBoundedTrace::MemoryBoundedTrace(size_t softMemoryUpperBound) { } -namespace { - -size_t -computeTraceTreeMemoryUsage(const mbus::TraceNode& node) +bool +MemoryBoundedTrace::add(const mbus::TraceNode& node) { - if (node.isLeaf()) { - return node.getNote().size(); - } - size_t childSum = 0; - const uint32_t childCount = node.getNumChildren(); - for (uint32_t i = 0; i < childCount; ++i) { - childSum += computeTraceTreeMemoryUsage(node.getChild(i)); + const size_t nodeFootprint = node.computeMemoryUsage(); + + if (_currentMemoryUsed >= _softMemoryUpperBound) { + ++_omittedNodes; + _omittedBytes += nodeFootprint; + return false; } - return childSum; + _trace.addChild(vespalib::TraceNode(node)); + _currentMemoryUsed += nodeFootprint; + return true; } -} // anon ns - bool -MemoryBoundedTrace::add(const mbus::TraceNode& node) +MemoryBoundedTrace::add(mbus::Trace && node) { - const size_t nodeFootprint = computeTraceTreeMemoryUsage(node); + const size_t nodeFootprint = node.computeMemoryUsage(); if (_currentMemoryUsed >= _softMemoryUpperBound) { ++_omittedNodes; _omittedBytes += nodeFootprint; return false; } - _node.addChild(node); + _trace.addChild(std::move(node)); _currentMemoryUsed += nodeFootprint; return true; } void -MemoryBoundedTrace::moveTraceTo(mbus::TraceNode& out) +MemoryBoundedTrace::moveTraceTo(mbus::Trace& out) { - if (_node.isEmpty()) { + if (_trace.isEmpty()) { return; } if (_omittedNodes > 0) { - _node.addChild(vespalib::make_string( + _trace.trace(0, vespalib::make_string( "Trace too large; omitted %zu subsequent trace trees " "containing a total of %zu bytes", - _omittedNodes, _omittedBytes)); + _omittedNodes, _omittedBytes), false); } - out.addChild(_node); // XXX rvalue support should be added to TraceNode. - _node.clear(); + out.addChild(std::move(_trace)); _currentMemoryUsed = 0; _omittedNodes = 0; _omittedBytes = 0; diff --git a/storage/src/vespa/storage/visiting/memory_bounded_trace.h b/storage/src/vespa/storage/visiting/memory_bounded_trace.h index 71ce1be51ac..2c75c809af6 100644 --- a/storage/src/vespa/storage/visiting/memory_bounded_trace.h +++ b/storage/src/vespa/storage/visiting/memory_bounded_trace.h @@ -23,6 +23,7 @@ public: * otherwise. */ bool add(const mbus::TraceNode& node); + bool add(mbus::Trace && trace); /** * Append current trace tree to the output trace node and clear internal @@ -33,14 +34,14 @@ public: * * If current trace is empty, no nodes are added to `out`. */ - void moveTraceTo(mbus::TraceNode& out); + void moveTraceTo(mbus::Trace& out); size_t getApproxMemoryUsed() const noexcept { return _currentMemoryUsed; } private: - mbus::TraceNode _node; + mbus::Trace _trace; size_t _currentMemoryUsed; size_t _omittedNodes; size_t _omittedBytes; diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 982bcd78f6f..9dee798de84 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -146,8 +146,7 @@ Visitor::BucketIterationState::~BucketIterationState() { if (_iteratorId != 0) { // Making the assumption that this is effectively nothrow. - std::shared_ptr<DestroyIteratorCommand> cmd( - new DestroyIteratorCommand(_iteratorId)); + auto cmd = std::make_shared<DestroyIteratorCommand>(_iteratorId); cmd->setLoadType(_visitor._initiatingCmd->getLoadType()); cmd->getTrace().setLevel(_visitor._traceLevel); cmd->setPriority(0); @@ -178,7 +177,7 @@ Visitor::VisitorTarget::VisitorTarget() { } -Visitor::VisitorTarget::~VisitorTarget() {} +Visitor::VisitorTarget::~VisitorTarget() = default; Visitor::Visitor(StorageComponent& component) : _component(component), @@ -372,10 +371,9 @@ Visitor::sendReplyOnce() std::shared_ptr<api::StorageReply> reply(_initiatingCmd->makeReply()); _hitCounter->updateVisitorStatistics(_visitorStatistics); - static_cast<api::CreateVisitorReply*>(reply.get()) - ->setVisitorStatistics(_visitorStatistics); + static_cast<api::CreateVisitorReply*>(reply.get())->setVisitorStatistics(_visitorStatistics); if (shouldAddMbusTrace()) { - _trace.moveTraceTo(reply->getTrace().getRoot()); + _trace.moveTraceTo(reply->getTrace()); } reply->setResult(_result); LOG(debug, "Sending %s", reply->toString(true).c_str()); @@ -603,15 +601,14 @@ bool Visitor::addBoundedTrace(uint32_t level, const vespalib::string &message) { mbus::Trace tempTrace; tempTrace.trace(level, message); - return _trace.add(tempTrace.getRoot()); + return _trace.add(std::move(tempTrace)); } void -Visitor::handleDocumentApiReply(mbus::Reply::UP reply, - VisitorThreadMetrics& metrics) +Visitor::handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& metrics) { if (shouldAddMbusTrace()) { - _trace.add(reply->getTrace().getRoot()); + _trace.add(reply->steal_trace()); } mbus::Message::UP message = reply->getMessage(); @@ -832,7 +829,7 @@ Visitor::onGetIterReply(const std::shared_ptr<GetIterReply>& reply, } if (shouldAddMbusTrace()) { - _trace.add(reply->getTrace().getRoot()); + _trace.add(reply->steal_trace()); } LOG(debug, "Continuing visitor %s.", _id.c_str()); |