diff options
author | Tor Brede Vekterli <vekterli@vespa.ai> | 2023-11-15 10:47:10 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@vespa.ai> | 2023-11-15 10:47:10 +0000 |
commit | 5330601cead4872151bdd446d94112faf24ee42a (patch) | |
tree | a5847f2b383cdafcf77c51a819005985a8adafcc /storage/src | |
parent | d7fea86dfce26c92c4365cbb7c2500cbcabbf54a (diff) |
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).
Diffstat (limited to 'storage/src')
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; |