From 9a676ff3b35353424d9a02a4c09d9dd1bb655f46 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Fri, 29 Jan 2021 14:29:59 +0000 Subject: Wire reporting of attribute resource usage all the way to the cluster controller via the host info API. --- .../persistence/spi/attribute_resource_usage.h | 1 + .../attribute_usage_filter_test.cpp | 47 +++++++++++++++++----- .../proton/attribute/address_space_usage_stats.cpp | 10 +++++ .../proton/attribute/address_space_usage_stats.h | 12 +++++- .../proton/attribute/attribute_usage_filter.cpp | 14 ++++++- .../proton/attribute/attribute_usage_filter.h | 6 ++- .../proton/attribute/attribute_usage_stats.cpp | 8 ++++ .../proton/attribute/attribute_usage_stats.h | 15 ++++--- .../vespa/searchcore/proton/server/documentdb.cpp | 13 ++++-- .../vespa/searchcore/proton/server/documentdb.h | 2 + .../src/vespa/searchcore/proton/server/proton.cpp | 7 +++- .../service_layer_host_info_reporter_test.cpp | 22 ++++++++++ .../service_layer_host_info_reporter.cpp | 42 ++++++++++++++++--- vespalib/src/vespa/vespalib/util/address_space.cpp | 2 +- 14 files changed, 170 insertions(+), 31 deletions(-) diff --git a/persistence/src/vespa/persistence/spi/attribute_resource_usage.h b/persistence/src/vespa/persistence/spi/attribute_resource_usage.h index 8d28370e504..02e0c248e3d 100644 --- a/persistence/src/vespa/persistence/spi/attribute_resource_usage.h +++ b/persistence/src/vespa/persistence/spi/attribute_resource_usage.h @@ -29,6 +29,7 @@ public: double get_usage() const noexcept { return _usage; } const vespalib::string& get_name() const noexcept { return _name; } + bool valid() const noexcept { return !_name.empty(); } bool operator==(const AttributeResourceUsage& rhs) const noexcept { return ((_usage == rhs._usage) && (_name == rhs._name)); diff --git a/searchcore/src/tests/proton/attribute/attribute_usage_filter/attribute_usage_filter_test.cpp b/searchcore/src/tests/proton/attribute/attribute_usage_filter/attribute_usage_filter_test.cpp index 0fced6e0bff..b35027eac2a 100644 --- a/searchcore/src/tests/proton/attribute/attribute_usage_filter/attribute_usage_filter_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_usage_filter/attribute_usage_filter_test.cpp @@ -3,9 +3,13 @@ LOG_SETUP("attribute_usage_filter_test"); #include #include +#include using proton::AttributeUsageFilter; using proton::AttributeUsageStats; +using proton::IAttributeUsageListener; +using search::AddressSpaceUsage; +using vespalib::AddressSpace; namespace { @@ -38,33 +42,47 @@ public: } }; +class MyListener : public IAttributeUsageListener { +public: + AttributeUsageStats stats; + MyListener() : stats() {} + void notify_attribute_usage(const AttributeUsageStats &stats_in) override { + stats = stats_in; + } +}; + struct Fixture { - AttributeUsageFilter _filter; + AttributeUsageFilter filter; + const MyListener* listener; using State = AttributeUsageFilter::State; using Config = AttributeUsageFilter::Config; Fixture() - : _filter() + : filter(), + listener() { + auto my_listener = std::make_unique(); + listener = my_listener.get(); + filter.set_listener(std::move(my_listener)); } void testWrite(const vespalib::string &exp) { if (exp.empty()) { - EXPECT_TRUE(_filter.acceptWriteOperation()); - State state = _filter.getAcceptState(); + EXPECT_TRUE(filter.acceptWriteOperation()); + State state = filter.getAcceptState(); EXPECT_TRUE(state.acceptWriteOperation()); EXPECT_EQUAL(exp, state.message()); } else { - EXPECT_FALSE(_filter.acceptWriteOperation()); - State state = _filter.getAcceptState(); + EXPECT_FALSE(filter.acceptWriteOperation()); + State state = filter.getAcceptState(); EXPECT_FALSE(state.acceptWriteOperation()); EXPECT_EQUAL(exp, state.message()); } } void setAttributeStats(const AttributeUsageStats &stats) { - _filter.setAttributeStats(stats); + filter.setAttributeStats(stats); } }; @@ -78,7 +96,7 @@ TEST_F("Check that default filter allows write", Fixture) TEST_F("Check that enum store limit can be reached", Fixture) { - f._filter.setConfig(Fixture::Config(0.8, 1.0)); + f.filter.setConfig(Fixture::Config(0.8, 1.0)); MyAttributeStats stats; stats.triggerEnumStoreLimit(); f.setAttributeStats(stats); @@ -95,7 +113,7 @@ TEST_F("Check that enum store limit can be reached", Fixture) TEST_F("Check that multivalue limit can be reached", Fixture) { - f._filter.setConfig(Fixture::Config(1.0, 0.8)); + f.filter.setConfig(Fixture::Config(1.0, 0.8)); MyAttributeStats stats; stats.triggerMultiValueLimit(); f.setAttributeStats(stats); @@ -113,7 +131,7 @@ TEST_F("Check that multivalue limit can be reached", Fixture) TEST_F("Check that both enumstore limit and multivalue limit can be reached", Fixture) { - f._filter.setConfig(Fixture::Config(0.8, 0.8)); + f.filter.setConfig(Fixture::Config(0.8, 0.8)); MyAttributeStats stats; stats.triggerEnumStoreLimit(); stats.triggerMultiValueLimit(); @@ -139,4 +157,13 @@ TEST_F("Check that both enumstore limit and multivalue limit can be reached", "attributeName: \"multiValueName\", subdb: \"ready\"}"); } +TEST_F("listener is updated when attribute stats change", Fixture) +{ + AttributeUsageStats stats; + AddressSpaceUsage usage(AddressSpace(12, 10, 15), AddressSpace(22, 20, 25)); + stats.merge(usage, "my_attr", "my_subdb"); + f.setAttributeStats(stats); + EXPECT_EQUAL(stats, f.listener->stats); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.cpp b/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.cpp index 3390447e26a..1acfa64285c 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "address_space_usage_stats.h" +#include namespace proton { @@ -25,4 +26,13 @@ AddressSpaceUsageStats::merge(const vespalib::AddressSpace &usage, } } +std::ostream& +operator<<(std::ostream& out, const AddressSpaceUsageStats& rhs) +{ + out << "{usage=" << rhs.getUsage() << + ", attribute_name=" << rhs.getAttributeName() << + ", subdb_name=" << rhs.getSubDbName() << "}"; + return out; +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.h b/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.h index 400c7cde03f..9ed68693ec1 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/address_space_usage_stats.h @@ -7,8 +7,8 @@ namespace proton { -/* - * class representing usage of a single address space (enum store or +/** + * Class representing usage of a single address space (enum store or * multi value) and the most largest attribute in that respect, relative * to the limit. */ @@ -28,6 +28,14 @@ public: const vespalib::AddressSpace &getUsage() const { return _usage; } const vespalib::string &getAttributeName() const { return _attributeName; } const vespalib::string &getSubDbName() const { return _subDbName; } + + bool operator==(const AddressSpaceUsageStats& rhs) const { + return (_usage == rhs._usage) && + (_attributeName == rhs._attributeName) && + (_subDbName == rhs._subDbName); + } }; +std::ostream& operator<<(std::ostream &out, const AddressSpaceUsageStats& rhs); + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.cpp index 2852497cf3f..e19e7976227 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "attribute_usage_filter.h" +#include "i_attribute_usage_listener.h" #include namespace proton { @@ -85,7 +86,8 @@ AttributeUsageFilter::AttributeUsageFilter() _attributeStats(), _config(), _state(), - _acceptWrite(true) + _acceptWrite(true), + _listener() { } @@ -97,6 +99,9 @@ AttributeUsageFilter::setAttributeStats(AttributeUsageStats attributeStats_in) Guard guard(_lock); _attributeStats = attributeStats_in; recalcState(guard); + if (_listener) { + _listener->notify_attribute_usage(_attributeStats); + } } AttributeUsageStats @@ -114,6 +119,13 @@ AttributeUsageFilter::setConfig(Config config_in) recalcState(guard); } +void +AttributeUsageFilter::set_listener(std::unique_ptr listener) +{ + Guard guard(_lock); + _listener = std::move(listener); +} + double AttributeUsageFilter::getEnumStoreUsedRatio() const { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.h index ecc95d4ee02..cb9687e31a1 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_filter.h @@ -10,7 +10,9 @@ namespace proton { -/* +class IAttributeUsageListener; + +/** * Class to filter write operations based on sampled information about * attribute resource usage (e.g. enum store and multivalue mapping). * If resource limit is reached then further writes are denied in @@ -29,6 +31,7 @@ private: Config _config; State _state; std::atomic _acceptWrite; + std::unique_ptr _listener; void recalcState(const Guard &guard); // called with _lock held public: @@ -37,6 +40,7 @@ public: void setAttributeStats(AttributeUsageStats attributeStats_in); AttributeUsageStats getAttributeUsageStats() const; void setConfig(Config config); + void set_listener(std::unique_ptr listener); double getEnumStoreUsedRatio() const; double getMultiValueUsedRatio() const; bool acceptWriteOperation() const override; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.cpp index 4d59c9081e0..f3da5486d3e 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "attribute_usage_stats.h" +#include namespace proton { @@ -19,5 +20,12 @@ AttributeUsageStats::merge(const search::AddressSpaceUsage &usage, _multiValueUsage.merge(usage.multiValueUsage(), attributeName, subDbName); } +std::ostream& +operator<<(std::ostream& out, const AttributeUsageStats& rhs) +{ + out << "{enum_store=" << rhs.enumStoreUsage() << + ", multi_value=" << rhs.multiValueUsage() << "}"; + return out; +} } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.h index f1805809e80..1eb6a9cc6be 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_stats.h @@ -7,7 +7,7 @@ namespace proton { -/* +/** * Class representing aggregated attribute usage, with info about * the most bloated attributes with regards to enum store and * multivalue mapping. @@ -23,10 +23,15 @@ public: const vespalib::string &attributeName, const vespalib::string &subDbName); - const AddressSpaceUsageStats & - enumStoreUsage() const { return _enumStoreUsage; } - const AddressSpaceUsageStats & - multiValueUsage() const { return _multiValueUsage; } + const AddressSpaceUsageStats& enumStoreUsage() const { return _enumStoreUsage; } + const AddressSpaceUsageStats& multiValueUsage() const { return _multiValueUsage; } + + bool operator==(const AttributeUsageStats& rhs) const { + return (_enumStoreUsage == rhs._enumStoreUsage) && + (_multiValueUsage == rhs._multiValueUsage); + } }; +std::ostream& operator<<(std::ostream& out, const AttributeUsageStats& rhs); + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index cd517fe7c60..60878c2b314 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -6,16 +6,17 @@ #include "document_subdb_collection_explorer.h" #include "documentdb.h" #include "documentdbconfigscout.h" +#include "feedhandler.h" #include "idocumentdbowner.h" #include "idocumentsubdb.h" #include "lid_space_compaction_handler.h" #include "maintenance_jobs_injector.h" #include "reconfig_params.h" -#include "feedhandler.h" -#include #include +#include #include #include +#include #include #include #include @@ -26,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -34,7 +36,6 @@ #include #include #include -#include #include #include @@ -1113,4 +1114,10 @@ DocumentDB::transient_memory_usage_provider() return _transient_memory_usage_provider; } +void +DocumentDB::set_attribute_usage_listener(std::unique_ptr listener) +{ + _writeFilter.set_listener(std::move(listener)); +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h index 9c2facdddb0..b5d975884a3 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h @@ -416,6 +416,8 @@ public: IDiskMemUsageListener *diskMemUsageListener() { return &_dmUsageForwarder; } std::shared_ptr transient_memory_usage_provider(); ExecutorThreadingService & getWriteService() { return _writeService; } + + void set_attribute_usage_listener(std::unique_ptr listener); }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 6706581f52c..5aa5d88eda9 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -18,14 +18,16 @@ #include #include #include +#include +#include #include #include #include #include +#include #include #include #include -#include #include #include #include @@ -36,7 +38,6 @@ #include #include #include -#include #include #include @@ -630,6 +631,8 @@ Proton::addDocumentDB(const document::DocumentType &docType, } // TODO: Fix race with new cluster state setting. _persistenceEngine->putHandler(persistenceWGuard, bucketSpace, docTypeName, persistenceHandler); + ret->set_attribute_usage_listener( + _persistenceEngine->get_resource_usage_tracker().make_attribute_usage_listener(docTypeName.getName())); } auto searchHandler = std::make_shared(ret); _summaryEngine->putSearchHandler(docTypeName, searchHandler); diff --git a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp index 86595fb44e8..a54c5c8ccf9 100644 --- a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp +++ b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp @@ -102,6 +102,28 @@ TEST_F(ServiceLayerHostInfoReporterTest, request_almost_immediate_node_state_as_ EXPECT_EQ(ResourceUsage(0.8, 0.7, {0.1, attr_es_name}, {0.2, attr_mv_name}), get_usage()); } +TEST_F(ServiceLayerHostInfoReporterTest, + first_valid_attribute_enum_store_sample_triggers_immediate_node_state_when_below_slack_diff) +{ + // TODO: Assert this is below slack diff when that becomes configurable. + constexpr double usage_below_slack_diff = 0.00001; + notify(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}); + EXPECT_EQ(1, requested_almost_immediate_replies()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_usage()); +} + +TEST_F(ServiceLayerHostInfoReporterTest, + first_valid_attribute_multi_value_sample_triggers_immediate_node_state_when_below_slack_diff) +{ + // TODO: Assert this is below slack diff when that becomes configurable. + constexpr double usage_below_slack_diff = 0.00001; + notify(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}); + EXPECT_EQ(1, requested_almost_immediate_replies()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_usage()); +} + TEST_F(ServiceLayerHostInfoReporterTest, json_report_generated) { EXPECT_EQ(ResourceUsage(0.0, 0.0), get_slime_usage()); diff --git a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp index c78b24bee78..88cdcc2b42f 100644 --- a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp @@ -1,8 +1,13 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "service_layer_host_info_reporter.h" +#include #include #include +#include + +#include +LOG_SETUP(".persistence.filestor.service_layer_host_info_reporter"); namespace storage { @@ -33,13 +38,22 @@ void write_attribute_usage(vespalib::JsonStream& output, const vespalib::string output << End(); } -bool want_immediate_report(const spi::ResourceUsage& old_resource_usage, const spi::ResourceUsage& resource_usage) +bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::ResourceUsage& new_usage) { - auto disk_usage_diff = fabs(resource_usage.get_disk_usage() - old_resource_usage.get_disk_usage()); - auto memory_usage_diff = fabs(resource_usage.get_memory_usage() - old_resource_usage.get_memory_usage()); - auto attribute_enum_store_diff = fabs(resource_usage.get_attribute_enum_store_usage().get_usage() - old_resource_usage.get_attribute_enum_store_usage().get_usage()); - auto attribute_multivalue_diff = fabs(resource_usage.get_attribute_multivalue_usage().get_usage() - old_resource_usage.get_attribute_multivalue_usage().get_usage()); - return (disk_usage_diff > diff_slack || memory_usage_diff > diff_slack || attribute_enum_store_diff > diff_slack || attribute_multivalue_diff > diff_slack); + auto disk_usage_diff = fabs(new_usage.get_disk_usage() - old_usage.get_disk_usage()); + auto memory_usage_diff = fabs(new_usage.get_memory_usage() - old_usage.get_memory_usage()); + auto enum_store_diff = fabs(new_usage.get_attribute_enum_store_usage().get_usage() - old_usage.get_attribute_enum_store_usage().get_usage()); + auto multivalue_diff = fabs(new_usage.get_attribute_multivalue_usage().get_usage() - old_usage.get_attribute_multivalue_usage().get_usage()); + bool enum_store_got_valid = !old_usage.get_attribute_enum_store_usage().valid() && + new_usage.get_attribute_enum_store_usage().valid(); + bool multivalue_got_valid = !old_usage.get_attribute_multivalue_usage().valid() && + new_usage.get_attribute_multivalue_usage().valid(); + return ((disk_usage_diff > diff_slack) || + (memory_usage_diff > diff_slack) || + (enum_store_diff > diff_slack) || + (multivalue_diff > diff_slack) || + enum_store_got_valid || + multivalue_got_valid); } } @@ -58,10 +72,25 @@ ServiceLayerHostInfoReporter::~ServiceLayerHostInfoReporter() spi::ResourceUsageListener::reset(); // detach } +namespace { + +vespalib::string +to_string(const spi::ResourceUsage& usage) +{ + std::ostringstream oss; + oss << usage; + return oss.str(); +} + +} + void ServiceLayerHostInfoReporter::update_resource_usage(const spi::ResourceUsage& resource_usage) { bool immediate_report = want_immediate_report(_old_resource_usage, resource_usage); + LOG(debug, "update_resource_usage(): immediate_report=%s, old_usage=%s, new_usage=%s", + (immediate_report ? "true" : "false"), to_string(_old_resource_usage).c_str(), + to_string(resource_usage).c_str()); if (immediate_report) { _old_resource_usage = resource_usage; } @@ -82,6 +111,7 @@ ServiceLayerHostInfoReporter::report(vespalib::JsonStream& output) { std::lock_guard guard(_lock); auto& usage = get_usage(); + LOG(debug, "report(): usage=%s", to_string(usage).c_str()); write_usage(output, memory_label, usage.get_memory_usage()); write_usage(output, disk_label, usage.get_disk_usage()); write_attribute_usage(output, attribute_enum_store_label, usage.get_attribute_enum_store_usage()); diff --git a/vespalib/src/vespa/vespalib/util/address_space.cpp b/vespalib/src/vespa/vespalib/util/address_space.cpp index 058c00e835e..113e4ba1478 100644 --- a/vespalib/src/vespa/vespalib/util/address_space.cpp +++ b/vespalib/src/vespa/vespalib/util/address_space.cpp @@ -16,7 +16,7 @@ AddressSpace::AddressSpace(size_t used_, size_t dead_, size_t limit_) std::ostream &operator << (std::ostream &out, const AddressSpace &rhs) { - return out << "used=" << rhs.used() << ", dead=" << rhs.dead() << ", limit=" << rhs.limit(); + return out << "{used=" << rhs.used() << ", dead=" << rhs.dead() << ", limit=" << rhs.limit() << "}"; } } // namespace vespalib -- cgit v1.2.3