diff options
14 files changed, 72 insertions, 94 deletions
diff --git a/messagebus/src/vespa/messagebus/routing/routingnode.cpp b/messagebus/src/vespa/messagebus/routing/routingnode.cpp index f24afbc07ca..3a433b05bc5 100644 --- a/messagebus/src/vespa/messagebus/routing/routingnode.cpp +++ b/messagebus/src/vespa/messagebus/routing/routingnode.cpp @@ -184,7 +184,7 @@ RoutingNode::setReply(Reply::UP reply) { if (reply) { _shouldRetry = _resender != nullptr && _resender->shouldRetry(*reply); - if ( ! reply->getTrace().getRoot().isEmpty()) { + if ( ! reply->getTrace().isEmpty()) { _trace.getRoot().addChild(std::move(reply->getTrace().getRoot())); reply->getTrace().clear(); } diff --git a/messagebus/src/vespa/messagebus/sendproxy.cpp b/messagebus/src/vespa/messagebus/sendproxy.cpp index c884932664b..fa485e19bb8 100644 --- a/messagebus/src/vespa/messagebus/sendproxy.cpp +++ b/messagebus/src/vespa/messagebus/sendproxy.cpp @@ -53,8 +53,7 @@ SendProxy::handleReply(Reply::UP reply) } else if (logger.wants(ns_log::Logger::spam)) { LOG(spam, "Trace for reply:\n%s", reply->getTrace().toString().c_str()); } - Trace empty; - trace.swap(empty); + trace.clear(); } else if (trace.getLevel() > 0) { trace.getRoot().addChild(reply->getTrace().getRoot()); trace.getRoot().normalize(); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 72668564345..8b16f3c92b6 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -799,7 +799,7 @@ 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) { @@ -807,7 +807,7 @@ TEST_F(VisitorTest, reply_contains_trace_if_trace_level_above_zero) { cmd->getTrace().setLevel(1); 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..cf7a0e4741e 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -144,7 +144,7 @@ GetOperation::onReceive(DistributorMessageSender& sender, const std::shared_ptr< LOG(debug, "Received %s", msg->toString(true).c_str()); - if ( ! getreply->getTrace().getRoot().isEmpty()) { + if ( ! getreply->getTrace().isEmpty()) { _msg->getTrace().getRoot().addChild(getreply->getTrace().getRoot()); } bool allDone = true; diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 3866ee4e6f7..0b7d035ec0d 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -647,7 +647,7 @@ TwoPhaseUpdateOperation::satisfiesUpdateTimestampConstraint(api::Timestamp ts) c void TwoPhaseUpdateOperation::addTraceFromReply(const api::StorageReply& reply) { - if ( ! reply.getTrace().getRoot().isEmpty()) { + if ( ! reply.getTrace().isEmpty()) { _trace.addChild(reply.getTrace().getRoot()); } } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index 6362db6c002..6253beda6e1 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -292,7 +292,7 @@ PersistenceMessageTrackerImpl::updateFromReply( api::BucketInfoReply& reply, uint16_t node) { - if ( ! reply.getTrace().getRoot().isEmpty()) { + if ( ! reply.getTrace().isEmpty()) { _trace.addChild(reply.getTrace().getRoot()); } diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 42d67573763..37ef4c39597 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -93,7 +93,7 @@ MessageTracker::sendReply() { _msg->toString(true).c_str(), vespalib::to_s(duration)); } if (hasReply()) { - if ( ! _context.getTrace().getRoot().isEmpty()) { + if ( ! _context.getTrace().isEmpty()) { getReply().getTrace().getRoot().addChild(_context.getTrace().getRoot()); } if (_updateBucketInfo) { @@ -108,7 +108,7 @@ MessageTracker::sendReply() { getReply().toString().c_str(), getReply().getMsgId()); _replySender.sendReplyDirectly(std::move(_reply)); } else { - if ( ! _context.getTrace().getRoot().isEmpty()) { + if ( ! _context.getTrace().isEmpty()) { _msg->getTrace().getRoot().addChild(_context.getTrace().getRoot()); } } diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 982bcd78f6f..2684ca03462 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), diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp index d9bbf34141a..3e886ac872e 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp @@ -11,7 +11,11 @@ StorageCommand::StorageCommand(const StorageCommand& other) _timeout(other._timeout), _sourceIndex(other._sourceIndex) { - setTrace(other.getTrace()); + if ( !other.getTrace().isEmpty()) { + setTrace(other.getTrace()); + } else { + getTrace().setLevel(other.getTrace().getLevel()); + } } StorageCommand::StorageCommand(const MessageType& type, Priority p) diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp index 81cdadb3623..57dab64e0bd 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp @@ -14,7 +14,11 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code) if (cmd.getAddress()) { setAddress(*cmd.getAddress()); } - setTrace(cmd.getTrace()); + if ( ! cmd.getTrace().isEmpty()) { + setTrace(cmd.getTrace()); + } else { + getTrace().setLevel(cmd.getTrace().getLevel()); + } setTransportContext(cmd.getTransportContext()); } diff --git a/vespalib/src/vespa/vespalib/trace/trace.cpp b/vespalib/src/vespa/vespalib/trace/trace.cpp index 8ca2ef6561c..2a9afc9b36b 100644 --- a/vespalib/src/vespa/vespalib/trace/trace.cpp +++ b/vespalib/src/vespa/vespalib/trace/trace.cpp @@ -6,45 +6,6 @@ namespace vespalib { -Trace::Trace() : - _level(0), - _root() -{ - // empty -} - -Trace::Trace(uint32_t level) : - _level(level), - _root() -{ - // empty -} - -Trace::~Trace() = default; - -Trace & -Trace::clear() -{ - _level = 0; - _root.clear(); - return *this; -} - -Trace & -Trace::swap(Trace &other) -{ - std::swap(_level, other._level); - _root.swap(other._root); - return *this; -} - -Trace & -Trace::setLevel(uint32_t level) -{ - _level = std::min(level, 9u); - return *this; -} - bool Trace::trace(uint32_t level, const string ¬e, bool addTime) { @@ -53,7 +14,7 @@ Trace::trace(uint32_t level, const string ¬e, bool addTime) } if (addTime) { struct timeval tv; - gettimeofday(&tv, NULL); + gettimeofday(&tv, nullptr); _root.addChild(make_string("[%ld.%06ld] %s", tv.tv_sec, static_cast<long>(tv.tv_usec), note.c_str())); } else { _root.addChild(note); diff --git a/vespalib/src/vespa/vespalib/trace/trace.h b/vespalib/src/vespa/vespalib/trace/trace.h index d1ca5c5d7e7..6676be4a81e 100644 --- a/vespalib/src/vespa/vespalib/trace/trace.h +++ b/vespalib/src/vespa/vespalib/trace/trace.h @@ -19,30 +19,32 @@ namespace vespalib { */ class Trace { private: - uint32_t _level; TraceNode _root; - + uint32_t _level; public: /** * Create an empty Trace with level set to 0 (no tracing) */ - Trace(); - ~Trace(); + Trace() : Trace(0) {} /** * Create an empty trace with given level. * * @param level Level to set. */ - explicit Trace(uint32_t level); + explicit Trace(uint32_t level) : _root(), _level(level) { } /** * Remove all trace information and set the trace level to 0. * * @return This, to allow chaining. */ - Trace &clear(); + Trace &clear() { + _level = 0; + _root.clear(); + return *this; + } /** * Swap the internals of this with another. @@ -50,7 +52,11 @@ public: * @param other The trace to swap internals with. * @return This, to allow chaining. */ - Trace &swap(Trace &other); + Trace &swap(Trace &other) { + std::swap(_level, other._level); + _root.swap(other._root); + return *this; + } /** * Set the trace level. 0 means no tracing, 9 means enable all tracing. @@ -58,7 +64,10 @@ public: * @param level The level to set. * @return This, to allow chaining. */ - Trace &setLevel(uint32_t level); + Trace &setLevel(uint32_t level) { + _level = std::min(level, 9u); + return *this; + } /** * Returns the trace level. @@ -105,6 +114,8 @@ public: */ const TraceNode &getRoot() const { return _root; } + bool isEmpty() const { return _root.isEmpty(); } + /** * Returns a string representation of the contained trace tree. This is a * readable, non-parseable string. diff --git a/vespalib/src/vespa/vespalib/trace/tracenode.cpp b/vespalib/src/vespa/vespalib/trace/tracenode.cpp index d34b45025d3..0f845ca646a 100644 --- a/vespalib/src/vespa/vespalib/trace/tracenode.cpp +++ b/vespalib/src/vespa/vespalib/trace/tracenode.cpp @@ -40,22 +40,22 @@ struct Cmp { } // namespace <unnamed> -TraceNode::TraceNode() : - _parent(nullptr), - _strict(true), - _hasNote(false), - _note(""), - _children(), - _timestamp() +TraceNode::TraceNode() + : _note(""), + _children(), + _parent(nullptr), + _timestamp(), + _strict(true), + _hasNote(false) { } -TraceNode::TraceNode(const TraceNode &rhs) : - _parent(nullptr), - _strict(rhs._strict), - _hasNote(rhs._hasNote), - _note(rhs._note), - _children(), - _timestamp(rhs._timestamp) +TraceNode::TraceNode(const TraceNode &rhs) + : _note(rhs._note), + _children(), + _parent(nullptr), + _timestamp(rhs._timestamp), + _strict(rhs._strict), + _hasNote(rhs._hasNote) { addChildren(rhs._children); } @@ -65,22 +65,22 @@ TraceNode & TraceNode::operator =(const TraceNode &) = default; TraceNode::~TraceNode() = default; -TraceNode::TraceNode(const string ¬e, system_time timestamp) : - _parent(nullptr), - _strict(true), - _hasNote(true), - _note(note), - _children(), - _timestamp(timestamp) +TraceNode::TraceNode(const string ¬e, system_time timestamp) + : _note(note), + _children(), + _parent(nullptr), + _timestamp(timestamp), + _strict(true), + _hasNote(true) { } -TraceNode::TraceNode(system_time timestamp) : - _parent(nullptr), - _strict(true), - _hasNote(false), - _note(""), - _children(), - _timestamp(timestamp) +TraceNode::TraceNode(system_time timestamp) + : _note(""), + _children(), + _parent(nullptr), + _timestamp(timestamp), + _strict(true), + _hasNote(false) { } TraceNode & diff --git a/vespalib/src/vespa/vespalib/trace/tracenode.h b/vespalib/src/vespa/vespalib/trace/tracenode.h index 63e7bcd6dc0..405c7d994da 100644 --- a/vespalib/src/vespa/vespalib/trace/tracenode.h +++ b/vespalib/src/vespa/vespalib/trace/tracenode.h @@ -23,12 +23,12 @@ struct TraceVisitor; */ class TraceNode { private: - TraceNode *_parent; - bool _strict; - bool _hasNote; string _note; std::vector<TraceNode> _children; + TraceNode *_parent; system_time _timestamp; + bool _strict; + bool _hasNote; public: /** |