Add class comments and resolve PR comments.

5 files changed, 27 insertions, 15 deletions

diff --git a/persistence/src/vespa/persistence/spi/operationcomplete.h b/persistence/src/vespa/persistence/spi/operationcomplete.h
index 1a548e613dd..fa386e274f2 100644
--- a/persistence/src/vespa/persistence/spi/operationcomplete.h
+++ b/persistence/src/vespa/persistence/spi/operationcomplete.h
@@ -1,4 +1,5 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.#pragma once
+
 #pragma once
 
 #include <memory>
diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
index 02fb1bb4719..c60ac615644 100644
--- a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
+++ b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
@@ -9,7 +9,7 @@ PersistenceProvider::~PersistenceProvider() = default;
 
 class CatchResult : public OperationComplete {
 public:
-    std::future<Result::UP> waitResult() {
+    std::future<Result::UP> future_result() {
         return promisedResult.get_future();
     }
     void onComplete(Result::UP result) override {
@@ -18,13 +18,15 @@ public:
 private:
     std::promise<Result::UP> promisedResult;
 };
+
 Result
 PersistenceProvider::put(const Bucket& bucket, Timestamp timestamp, DocumentSP doc, Context& context) {
     auto catcher = std::make_unique<CatchResult>();
-    auto future = catcher->waitResult();
+    auto future = catcher->future_result();
     putAsync(bucket, timestamp, std::move(doc), context, std::move(catcher));
     return *future.get();
 }
+
 void
 PersistenceProvider::putAsync(const Bucket &bucket, Timestamp timestamp, DocumentSP doc, Context &context,
                               OperationComplete::UP onComplete) {
diff --git a/searchcore/src/vespa/searchcore/proton/common/feedtoken.h b/searchcore/src/vespa/searchcore/proton/common/feedtoken.h
index 363886b9f31..cf6c1f5a7e9 100644
--- a/searchcore/src/vespa/searchcore/proton/common/feedtoken.h
+++ b/searchcore/src/vespa/searchcore/proton/common/feedtoken.h
@@ -9,19 +9,23 @@ namespace proton {
 
 typedef std::unique_ptr<storage::spi::Result> ResultUP;
 
+namespace feedtoken {
+
 /**
  * This class is used by the FeedEngine to encapsulate the necessary information
  * for an IFeedHandler to perform an async reply to an operation. A unique
  * instance of this class is passed to every invokation of the IFeedHandler.
  */
-namespace feedtoken {
-
 class ITransport {
 public:
     virtual ~ITransport() { }
     virtual void send(ResultUP result, bool documentWasFound) = 0;
 };
 
+/**
+ * This holds the result of the feed operation until it is either failed or acked.
+ * Guarantees that the result is propagated back to the invoker via ITransport interface.
+ */
 class State : public search::IDestructorCallback {
 public:
     State(const State &) = delete;
@@ -42,6 +46,11 @@ private:
     bool                  _documentWasFound;
     std::atomic<bool>     _alreadySent;
 };
+
+/**
+ * This takes ownership ov the transport object, so that it can be used fully asynchronous
+ * without invoker needing to hold any state.
+ */
 class OwningState : public State {
 public:
     OwningState(std::unique_ptr<ITransport> transport)
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h
index b3b3bc43aa7..da4e19d3584 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h
@@ -10,8 +10,7 @@
 namespace proton {
 
 /**
- * Implementation of FeedToken::ITransport for handling the async reply for an operation.
- * Uses an internal count down latch to keep track the number of outstanding replies.
+ * Base implementation for merging results from multiple sources.
  */
 
 class TransportMerger : public feedtoken::ITransport {
@@ -29,14 +28,17 @@ private:
     void mergeWithLock(ResultUP result, bool documentWasFound);
     std::unique_ptr<std::mutex> _lock;
 };
+
+/**
+ * Implementation of FeedToken::ITransport for handling the async reply for an operation.
+ * Uses an internal count down latch to keep track the number of outstanding replies.
+ */
 class TransportLatch : public TransportMerger {
 private:
-
     using UpdateResult = storage::spi::UpdateResult;
     using RemoveResult = storage::spi::RemoveResult;
     vespalib::CountDownLatch _latch;
 
-
 public:
     TransportLatch(uint32_t cnt);
     ~TransportLatch() override;
diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp
index c46b9ab7059..9095b351f64 100644
--- a/storage/src/vespa/storage/persistence/persistenceutil.cpp
+++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp
@@ -52,12 +52,7 @@ MessageTracker::setMetric(FileStorThreadMetrics::Op& metric) {
     _metric = &metric;
 }
 
-MessageTracker::~MessageTracker()
-{
-    if (_reply.get() && _reply->getResult().success()) {
-        _metric->latency.addValue(_timer.getElapsedTimeAsDouble());
-    }
-}
+MessageTracker::~MessageTracker() = default;
 
 void
 MessageTracker::sendReply() {
@@ -70,6 +65,9 @@ MessageTracker::sendReply() {
                 _env.setBucketInfo(*this, _bucketLock->getBucket());
             }
         }
+        if (getReply().getResult().success()) {
+            _metric->latency.addValue(_timer.getElapsedTimeAsDouble());
+        }
         LOG(spam, "Sending reply up: %s %" PRIu64,
             getReply().toString().c_str(), getReply().getMsgId());
         _env._fileStorHandler.sendReply(std::move(_reply));