2 files changed, 31 insertions, 8 deletions
diff --git a/storage/src/tests/distributor/garbagecollectiontest.cpp b/storage/src/tests/distributor/garbagecollectiontest.cpp
index 65c1ac726b5..776cfc14d84 100644
--- a/storage/src/tests/distributor/garbagecollectiontest.cpp
+++ b/storage/src/tests/distributor/garbagecollectiontest.cpp
@@ -3,6 +3,7 @@
 #include <vespa/storageapi/message/removelocation.h>
 #include <vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h>
 #include <vespa/storage/distributor/idealstatemanager.h>
+#include <vespa/storage/distributor/idealstatemetricsset.h>
 #include <tests/distributor/distributortestutil.h>
 #include <vespa/storage/distributor/distributor.h>
 #include <vespa/document/test/make_document_bucket.h>
@@ -35,11 +36,13 @@ struct GarbageCollectionOperationTest : Test, DistributorTestUtil {
     }
 
     // FIXME fragile to assume that send order == node index, but that's the way it currently works
-    void reply_to_nth_request(GarbageCollectionOperation& op, size_t n, uint32_t bucket_info_checksum) {
+    void reply_to_nth_request(GarbageCollectionOperation& op, size_t n,
+                              uint32_t bucket_info_checksum, uint32_t n_docs_removed) {
         auto msg = _sender.command(n);
         assert(msg->getType() == api::MessageType::REMOVELOCATION);
         std::shared_ptr<api::StorageReply> reply(msg->makeReply());
         auto& gc_reply = dynamic_cast<api::RemoveLocationReply&>(*reply);
+        gc_reply.set_documents_removed(n_docs_removed);
         gc_reply.setBucketInfo(api::BucketInfo(bucket_info_checksum, 90, 500));
 
         op.receive(_sender, reply);
@@ -56,6 +59,13 @@ struct GarbageCollectionOperationTest : Test, DistributorTestUtil {
                     << entry->getNode(i)->getBucketInfo();
         }
     }
+
+    uint32_t gc_removed_documents_metric() {
+        auto metric_base = getIdealStateManager().getMetrics().operations[IdealStateOperation::GARBAGE_COLLECTION];
+        auto gc_metrics = std::dynamic_pointer_cast<GcMetricSet>(metric_base);
+        assert(gc_metrics);
+        return gc_metrics->documents_removed.getValue();
+    }
 };
 
 TEST_F(GarbageCollectionOperationTest, simple) {
@@ -63,29 +73,34 @@ TEST_F(GarbageCollectionOperationTest, simple) {
     op->start(_sender, framework::MilliSecTime(0));
 
     ASSERT_EQ(2, _sender.commands().size());
+    EXPECT_EQ(0u, gc_removed_documents_metric());
 
     for (uint32_t i = 0; i < 2; ++i) {
         std::shared_ptr<api::StorageCommand> msg = _sender.command(i);
         ASSERT_EQ(msg->getType(), api::MessageType::REMOVELOCATION);
         auto& tmp = dynamic_cast<api::RemoveLocationCommand&>(*msg);
         EXPECT_EQ("music.date < 34", tmp.getDocumentSelection());
-        reply_to_nth_request(*op, i, 777 + i);
+        reply_to_nth_request(*op, i, 777 + i, 50);
     }
     ASSERT_NO_FATAL_FAILURE(assert_bucket_db_contains({api::BucketInfo(777, 90, 500), api::BucketInfo(778, 90, 500)}, 34));
+    EXPECT_EQ(50u, gc_removed_documents_metric());
 }
 
 TEST_F(GarbageCollectionOperationTest, replica_bucket_info_not_added_to_db_until_all_replies_received) {
     auto op = create_op();
     op->start(_sender, framework::MilliSecTime(0));
     ASSERT_EQ(2, _sender.commands().size());
+    EXPECT_EQ(0u, gc_removed_documents_metric());
 
     // Respond to 1st request. Should _not_ cause bucket info to be merged into the database yet
-    reply_to_nth_request(*op, 0, 1234);
+    reply_to_nth_request(*op, 0, 1234, 70);
     ASSERT_NO_FATAL_FAILURE(assert_bucket_db_contains({api::BucketInfo(250, 50, 300), api::BucketInfo(250, 50, 300)}, 0));
 
     // Respond to 2nd request. This _should_ cause bucket info to be merged into the database.
-    reply_to_nth_request(*op, 1, 4567);
+    reply_to_nth_request(*op, 1, 4567, 60);
     ASSERT_NO_FATAL_FAILURE(assert_bucket_db_contains({api::BucketInfo(1234, 90, 500), api::BucketInfo(4567, 90, 500)}, 34));
+
+    EXPECT_EQ(70u, gc_removed_documents_metric()); // Use max of received metrics
 }
 
 TEST_F(GarbageCollectionOperationTest, gc_bucket_info_does_not_overwrite_later_sequenced_bucket_info_writes) {
@@ -93,10 +108,10 @@ TEST_F(GarbageCollectionOperationTest, gc_bucket_info_does_not_overwrite_later_s
     op->start(_sender, framework::MilliSecTime(0));
     ASSERT_EQ(2, _sender.commands().size());
 
-    reply_to_nth_request(*op, 0, 1234);
+    reply_to_nth_request(*op, 0, 1234, 0);
     // Change to replica on node 0 happens after GC op, but before GC info is merged into the DB. Must not be lost.
     insertBucketInfo(op->getBucketId(), 0, 7777, 100, 2000);
-    reply_to_nth_request(*op, 1, 4567);
+    reply_to_nth_request(*op, 1, 4567, 0);
     // Bucket info for node 0 is that of the later sequenced operation, _not_ from the earlier GC op.
     ASSERT_NO_FATAL_FAILURE(assert_bucket_db_contains({api::BucketInfo(7777, 100, 2000), api::BucketInfo(4567, 90, 500)}, 34));
 }
diff --git a/storage/src/tests/persistence/processalltest.cpp b/storage/src/tests/persistence/processalltest.cpp
index 8c0f8853d2d..83f243ed1b2 100644
--- a/storage/src/tests/persistence/processalltest.cpp
+++ b/storage/src/tests/persistence/processalltest.cpp
@@ -23,11 +23,15 @@ TEST_F(ProcessAllHandlerTest, remove_location) {
     api::RemoveLocationCommand removeLocation("id.user == 4", makeDocumentBucket(bucketId));
     ProcessAllHandler handler(getEnv(), getPersistenceProvider());
     spi::Context context(documentapi::LoadType::DEFAULT, 0, 0);
-    handler.handleRemoveLocation(removeLocation, context);
+    auto tracker = handler.handleRemoveLocation(removeLocation, context);
 
     EXPECT_EQ("DocEntry(1234, 1, id:mail:testdoctype1:n=4:3619.html)\n"
               "DocEntry(2345, 1, id:mail:testdoctype1:n=4:4008.html)\n",
               dumpBucket(bucketId));
+
+    auto reply = std::dynamic_pointer_cast<api::RemoveLocationReply>(tracker->getReply());
+    ASSERT_TRUE(reply.get() != nullptr);
+    EXPECT_EQ(2u, reply->documents_removed());
 }
 
 TEST_F(ProcessAllHandlerTest, remove_location_document_subset) {
@@ -44,7 +48,7 @@ TEST_F(ProcessAllHandlerTest, remove_location_document_subset) {
     api::RemoveLocationCommand
         removeLocation("testdoctype1.headerval % 2 == 0", makeDocumentBucket(bucketId));
     spi::Context context(documentapi::LoadType::DEFAULT, 0, 0);
-    handler.handleRemoveLocation(removeLocation, context);
+    auto tracker = handler.handleRemoveLocation(removeLocation, context);
 
     EXPECT_EQ("DocEntry(100, 1, id:mail:testdoctype1:n=4:3619.html)\n"
               "DocEntry(101, 0, Doc(id:mail:testdoctype1:n=4:33113.html))\n"
@@ -57,6 +61,10 @@ TEST_F(ProcessAllHandlerTest, remove_location_document_subset) {
               "DocEntry(108, 1, id:mail:testdoctype1:n=4:42967.html)\n"
               "DocEntry(109, 0, Doc(id:mail:testdoctype1:n=4:6925.html))\n",
               dumpBucket(bucketId));
+
+    auto reply = std::dynamic_pointer_cast<api::RemoveLocationReply>(tracker->getReply());
+    ASSERT_TRUE(reply.get() != nullptr);
+    EXPECT_EQ(5u, reply->documents_removed());
 }
 
 TEST_F(ProcessAllHandlerTest, remove_location_throws_exception_on_unknown_doc_type) {