diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-11-15 15:17:37 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-15 15:17:37 +0100 |
commit | 241e9149b0a61f35467d39cc8c66d6a1962c8465 (patch) | |
tree | 3547d75f851334ce4df9b6e78201ecb220f1a329 | |
parent | a094adcba606c2121cb9971a7f4dca13b9692aa9 (diff) | |
parent | 5330601cead4872151bdd446d94112faf24ee42a (diff) |
Merge pull request #29337 from vespa-engine/vekterli/add-metric-for-remove-by-gid-ops
Add fundamental metrics for async "remove by GID" SPI operation
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; |