Add fundamental metrics for async "remove by GID" SPI operation

Tracks invocation count, latency and failures (although we don't
expect to see any failures in this pipeline, since the remove ops
logically happen atomically with the bucket iteration).

4 files changed, 25 insertions, 4 deletions

diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
index 96618eb9206..4911ad88692 100644
--- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
+++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
@@ -1421,6 +1421,14 @@ void FileStorManagerTest::do_test_delete_bucket(bool use_throttled_delete) {
         StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo"));
         EXPECT_FALSE(entry.exists());
     }
+    if (use_throttled_delete) {
+        auto& metrics = thread_metrics_of(*c.manager)->remove_by_gid;
+        EXPECT_EQ(metrics.failed.getValue(), 0);
+        EXPECT_EQ(metrics.count.getValue(), 1);
+        // We can't reliably test the actual latency here without wiring mock clock bumping into
+        // the async remove by GID execution, but we can at least test that we updated the metric.
+        EXPECT_EQ(metrics.latency.getCount(), 1);
+    }
 }
 
 // TODO remove once throttled behavior is the default
diff --git a/storage/src/vespa/storage/persistence/asynchandler.cpp b/storage/src/vespa/storage/persistence/asynchandler.cpp
index bb3952c8c33..8e962a59809 100644
--- a/storage/src/vespa/storage/persistence/asynchandler.cpp
+++ b/storage/src/vespa/storage/persistence/asynchandler.cpp
@@ -300,14 +300,25 @@ AsyncHandler::handle_delete_bucket_throttling(api::DeleteBucketCommand& cmd, Mes
         });
         _spi.deleteBucketAsync(spi_bucket, std::make_unique<ResultTaskOperationDone>(_sequencedExecutor, bucket.getBucketId(), std::move(task)));
     });
+
     auto& throttler = _env._fileStorHandler.operation_throttler();
+    auto* remove_by_gid_metric = &_env._metrics.remove_by_gid;
+
     for (auto& meta : meta_entries) {
         auto token = throttler.blocking_acquire_one();
+        remove_by_gid_metric->count.inc();
         std::vector<spi::DocTypeGidAndTimestamp> to_remove = {{meta->getDocumentType(), meta->getGid(), meta->getTimestamp()}};
-        auto task = makeResultTask([bucket = cmd.getBucket(), token = std::move(token), invoke_delete_on_zero_refs]([[maybe_unused]] spi::Result::UP ignored) {
-            LOG(spam, "%s: completed removeByGidAsync operation", bucket.toString().c_str());
-            // Nothing else clever to do here. Throttle token and deleteBucket dispatch refs dropped implicitly.
-        });
+        auto task = makeResultTask([bucket = cmd.getBucket(), token = std::move(token),
+                                    invoke_delete_on_zero_refs, remove_by_gid_metric,
+                                    op_timer = framework::MilliSecTimer(_env._component.getClock())]
+            (spi::Result::UP result) {
+                if (result->hasError()) {
+                    remove_by_gid_metric->failed.inc();
+                }
+                remove_by_gid_metric->latency.addValue(op_timer.getElapsedTimeAsDouble());
+                LOG(spam, "%s: completed removeByGidAsync operation", bucket.toString().c_str());
+                // Nothing else clever to do here. Throttle token and deleteBucket dispatch refs dropped implicitly.
+            });
         LOG(spam, "%s: about to invoke removeByGidAsync(%s, %s, %zu)", cmd.getBucket().toString().c_str(),
             vespalib::string(meta->getDocumentType()).c_str(), meta->getGid().toString().c_str(), meta->getTimestamp().getValue());
         _spi.removeByGidAsync(spi_bucket, std::move(to_remove), std::make_unique<ResultTaskOperationDone>(_sequencedExecutor, cmd.getBucketId(), std::move(task)));
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp
index 6072a1dbc3e..b8ccb15eb29 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp
@@ -159,6 +159,7 @@ FileStorThreadMetrics::FileStorThreadMetrics(const std::string& name, const std:
       visit(this),
       createBuckets("createbuckets", "Number of buckets that has been created.", this),
       deleteBuckets("deletebuckets", "Number of buckets that has been deleted.", this),
+      remove_by_gid("remove_by_gid", "Internal single-document remove operations used by DeleteBucket", this),
       repairs("bucketverified", "Number of times buckets have been checked.", this),
       repairFixed("bucketfixed", {}, "Number of times bucket has been fixed because of corruption", this),
       recheckBucketInfo("recheckbucketinfo",
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.h b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.h
index 85a3813c9eb..6a60ea44a45 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.h
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.h
@@ -103,6 +103,7 @@ struct FileStorThreadMetrics : public metrics::MetricSet
     Visitor visit;
     Op createBuckets;
     Op deleteBuckets;
+    Op remove_by_gid;
     Op repairs;
     metrics::LongCountMetric repairFixed;
     Op recheckBucketInfo;