diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2023-10-17 13:20:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-10-17 13:20:31 +0200 |
commit | 09257dea71a05086a5013d3e212242e78b58e0e7 (patch) | |
tree | 6ae19d0a3e975f22076e977265453090bd590f86 /storage | |
parent | fa67a8031b86744291641b4c7c61ffddce3664c0 (diff) | |
parent | 35795f558e8386a817f72d996a94ec2fb37a5032 (diff) |
Merge pull request #28964 from vespa-engine/vekterli/make-operation-priority-mapping-static
Remove unused configurability of operation priorities
Diffstat (limited to 'storage')
10 files changed, 99 insertions, 111 deletions
diff --git a/storage/src/tests/storageserver/documentapiconvertertest.cpp b/storage/src/tests/storageserver/documentapiconvertertest.cpp index 5e70c00cc5f..eb4789b25d4 100644 --- a/storage/src/tests/storageserver/documentapiconvertertest.cpp +++ b/storage/src/tests/storageserver/documentapiconvertertest.cpp @@ -77,7 +77,7 @@ struct DocumentApiConverterTest : Test { } void SetUp() override { - _converter = std::make_unique<DocumentApiConverter>(config::ConfigUri("raw:"), _bucketResolver); + _converter = std::make_unique<DocumentApiConverter>(_bucketResolver); }; template <typename DerivedT, typename BaseT> diff --git a/storage/src/tests/storageserver/priorityconvertertest.cpp b/storage/src/tests/storageserver/priorityconvertertest.cpp index 5462c83d2a2..69f9d313242 100644 --- a/storage/src/tests/storageserver/priorityconvertertest.cpp +++ b/storage/src/tests/storageserver/priorityconvertertest.cpp @@ -1,7 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/storage/storageserver/priorityconverter.h> -#include <tests/common/testhelper.h> #include <vespa/vespalib/gtest/gtest.h> using namespace ::testing; @@ -12,8 +11,7 @@ struct PriorityConverterTest : Test { std::unique_ptr<PriorityConverter> _converter; void SetUp() override { - vdstestlib::DirConfig config(getStandardConfig(true)); - _converter = std::make_unique<PriorityConverter>(config::ConfigUri(config.getConfigId())); + _converter = std::make_unique<PriorityConverter>(); }; }; diff --git a/storage/src/tests/storageserver/testvisitormessagesession.h b/storage/src/tests/storageserver/testvisitormessagesession.h index 86d4dc92a58..cc7dab7ef9e 100644 --- a/storage/src/tests/storageserver/testvisitormessagesession.h +++ b/storage/src/tests/storageserver/testvisitormessagesession.h @@ -49,9 +49,11 @@ struct TestVisitorMessageSessionFactory : public VisitorMessageSessionFactory bool _createAutoReplyVisitorSessions; PriorityConverter _priConverter; - TestVisitorMessageSessionFactory(vespalib::stringref configId = "") + TestVisitorMessageSessionFactory() : _createAutoReplyVisitorSessions(false), - _priConverter(config::ConfigUri(configId)) {} + _priConverter() + { + } VisitorMessageSession::UP createSession(Visitor& v, VisitorThread& vt) override { std::lock_guard lock(_accessLock); diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 991b98e5489..2b7039f36ea 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -83,7 +83,7 @@ VisitorManagerTest::initializeTest(bool defer_manager_thread_start) vdstestlib::DirConfig config(getStandardConfig(true)); config.getConfig("stor-visitor").set("visitorthreads", "1"); - _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(config.getConfigId()); + _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); _node = std::make_unique<TestServiceLayerApp>(config.getConfigId()); _node->setupDummyPersistence(); _node->getStateUpdater().setClusterState(std::make_shared<lib::ClusterState>("storage:1 distributor:1")); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 1f1d27ab4cb..49f1bc778fc 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -161,7 +161,7 @@ VisitorTest::initializeTest(const TestParams& params) std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str()))); std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d1", rootFolder.c_str()))); - _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(config.getConfigId()); + _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) { _messageSessionFactory->_autoReplyError = params._autoReplyError; _messageSessionFactory->_createAutoReplyVisitorSessions = true; diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index c126ec01dc6..bbd4e87cb40 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -230,7 +230,7 @@ CommunicationManager::CommunicationManager(StorageComponentRegister& compReg, _mbus(), _configUri(configUri), _closed(false), - _docApiConverter(configUri, std::make_shared<PlaceHolderBucketResolver>()), // TODO wire config from outside + _docApiConverter(std::make_shared<PlaceHolderBucketResolver>()), _thread() { _component.registerMetricUpdateHook(*this, 5s); diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index 04b3d8b6ce7..ca46e87285b 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -23,9 +23,8 @@ using document::BucketSpace; namespace storage { -DocumentApiConverter::DocumentApiConverter(const config::ConfigUri &configUri, - std::shared_ptr<const BucketResolver> bucketResolver) - : _priConverter(std::make_unique<PriorityConverter>(configUri)), +DocumentApiConverter::DocumentApiConverter(std::shared_ptr<const BucketResolver> bucketResolver) + : _priConverter(std::make_unique<PriorityConverter>()), _bucketResolver(std::move(bucketResolver)) {} diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.h b/storage/src/vespa/storage/storageserver/documentapiconverter.h index 5990d6f9017..96b119ff44e 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.h +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.h @@ -22,18 +22,17 @@ class PriorityConverter; class DocumentApiConverter { public: - DocumentApiConverter(const config::ConfigUri &configUri, - std::shared_ptr<const BucketResolver> bucketResolver); + explicit DocumentApiConverter(std::shared_ptr<const BucketResolver> bucketResolver); ~DocumentApiConverter(); - std::unique_ptr<api::StorageCommand> toStorageAPI(documentapi::DocumentMessage& msg); - std::unique_ptr<api::StorageReply> toStorageAPI(documentapi::DocumentReply& reply, api::StorageCommand& originalCommand); + [[nodiscard]] std::unique_ptr<api::StorageCommand> toStorageAPI(documentapi::DocumentMessage& msg); + [[nodiscard]] std::unique_ptr<api::StorageReply> toStorageAPI(documentapi::DocumentReply& reply, api::StorageCommand& originalCommand); void transferReplyState(storage::api::StorageReply& from, mbus::Reply& to); - std::unique_ptr<mbus::Message> toDocumentAPI(api::StorageCommand& cmd); + [[nodiscard]] std::unique_ptr<mbus::Message> toDocumentAPI(api::StorageCommand& cmd); const PriorityConverter& getPriorityConverter() const { return *_priConverter; } // BucketResolver getter and setter are both thread safe. - std::shared_ptr<const BucketResolver> bucketResolver() const; + [[nodiscard]] std::shared_ptr<const BucketResolver> bucketResolver() const; void setBucketResolver(std::shared_ptr<const BucketResolver> resolver); private: mutable std::mutex _mutex; diff --git a/storage/src/vespa/storage/storageserver/priorityconverter.cpp b/storage/src/vespa/storage/storageserver/priorityconverter.cpp index 13ab572c561..fe7570ff53a 100644 --- a/storage/src/vespa/storage/storageserver/priorityconverter.cpp +++ b/storage/src/vespa/storage/storageserver/priorityconverter.cpp @@ -1,85 +1,91 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "priorityconverter.h" -#include <vespa/config/subscription/configuri.h> -#include <vespa/config/helper/configfetcher.hpp> - +#include <map> namespace storage { -PriorityConverter::PriorityConverter(const config::ConfigUri & configUri) - : _configFetcher(std::make_unique<config::ConfigFetcher>(configUri.getContext())) +PriorityConverter::PriorityConverter() + : _mapping(), + _reverse_mapping() { - _configFetcher->subscribe<vespa::config::content::core::StorPrioritymappingConfig>(configUri.getConfigId(), this); - _configFetcher->start(); + init_static_priority_mappings(); } PriorityConverter::~PriorityConverter() = default; -uint8_t -PriorityConverter::toStoragePriority(documentapi::Priority::Value documentApiPriority) const +void +PriorityConverter::init_static_priority_mappings() { - const uint32_t index(static_cast<uint32_t>(documentApiPriority)); - if (index >= PRI_ENUM_SIZE) { - return 255; - } + // Defaults from `stor-prioritymapping` config + constexpr uint8_t highest = 50; + constexpr uint8_t very_high = 60; + constexpr uint8_t high_1 = 70; + constexpr uint8_t high_2 = 80; + constexpr uint8_t high_3 = 90; + constexpr uint8_t normal_1 = 100; + constexpr uint8_t normal_2 = 110; + constexpr uint8_t normal_3 = 120; + constexpr uint8_t normal_4 = 130; + constexpr uint8_t normal_5 = 140; + constexpr uint8_t normal_6 = 150; + constexpr uint8_t low_1 = 160; + constexpr uint8_t low_2 = 170; + constexpr uint8_t low_3 = 180; + constexpr uint8_t very_low = 190; + constexpr uint8_t lowest = 200; - return _mapping[index]; -} + _mapping[documentapi::Priority::PRI_HIGHEST] = highest; + _mapping[documentapi::Priority::PRI_VERY_HIGH] = very_high; + _mapping[documentapi::Priority::PRI_HIGH_1] = high_1; + _mapping[documentapi::Priority::PRI_HIGH_2] = high_2; + _mapping[documentapi::Priority::PRI_HIGH_3] = high_3; + _mapping[documentapi::Priority::PRI_NORMAL_1] = normal_1; + _mapping[documentapi::Priority::PRI_NORMAL_2] = normal_2; + _mapping[documentapi::Priority::PRI_NORMAL_3] = normal_3; + _mapping[documentapi::Priority::PRI_NORMAL_4] = normal_4; + _mapping[documentapi::Priority::PRI_NORMAL_5] = normal_5; + _mapping[documentapi::Priority::PRI_NORMAL_6] = normal_6; + _mapping[documentapi::Priority::PRI_LOW_1] = low_1; + _mapping[documentapi::Priority::PRI_LOW_2] = low_2; + _mapping[documentapi::Priority::PRI_LOW_3] = low_3; + _mapping[documentapi::Priority::PRI_VERY_LOW] = very_low; + _mapping[documentapi::Priority::PRI_LOWEST] = lowest; -documentapi::Priority::Value -PriorityConverter::toDocumentPriority(uint8_t storagePriority) const -{ - std::lock_guard guard(_mutex); - std::map<uint8_t, documentapi::Priority::Value>::const_iterator iter = - _reverseMapping.lower_bound(storagePriority); + std::map<uint8_t, documentapi::Priority::Value> reverse_map_helper; + reverse_map_helper[highest] = documentapi::Priority::PRI_HIGHEST; + reverse_map_helper[very_high] = documentapi::Priority::PRI_VERY_HIGH; + reverse_map_helper[high_1] = documentapi::Priority::PRI_HIGH_1; + reverse_map_helper[high_2] = documentapi::Priority::PRI_HIGH_2; + reverse_map_helper[high_3] = documentapi::Priority::PRI_HIGH_3; + reverse_map_helper[normal_1] = documentapi::Priority::PRI_NORMAL_1; + reverse_map_helper[normal_2] = documentapi::Priority::PRI_NORMAL_2; + reverse_map_helper[normal_3] = documentapi::Priority::PRI_NORMAL_3; + reverse_map_helper[normal_4] = documentapi::Priority::PRI_NORMAL_4; + reverse_map_helper[normal_5] = documentapi::Priority::PRI_NORMAL_5; + reverse_map_helper[normal_6] = documentapi::Priority::PRI_NORMAL_6; + reverse_map_helper[low_1] = documentapi::Priority::PRI_LOW_1; + reverse_map_helper[low_2] = documentapi::Priority::PRI_LOW_2; + reverse_map_helper[low_3] = documentapi::Priority::PRI_LOW_3; + reverse_map_helper[very_low] = documentapi::Priority::PRI_VERY_LOW; + reverse_map_helper[lowest] = documentapi::Priority::PRI_LOWEST; - if (iter != _reverseMapping.end()) { - return iter->second; + // Precompute a 1-1 LUT to avoid having to lower-bound lookup values in a fixed map + _reverse_mapping.resize(256); + for (size_t i = 0; i < 256; ++i) { + auto iter = reverse_map_helper.lower_bound(static_cast<uint8_t>(i)); + _reverse_mapping[i] = (iter != reverse_map_helper.cend()) ? iter->second : documentapi::Priority::PRI_LOWEST; } - - return documentapi::Priority::PRI_LOWEST; } -void -PriorityConverter::configure(std::unique_ptr<vespa::config::content::core::StorPrioritymappingConfig> config) +uint8_t +PriorityConverter::toStoragePriority(documentapi::Priority::Value documentApiPriority) const noexcept { - // Data race free; _mapping is an array of std::atomic. - _mapping[documentapi::Priority::PRI_HIGHEST] = config->highest; - _mapping[documentapi::Priority::PRI_VERY_HIGH] = config->veryHigh; - _mapping[documentapi::Priority::PRI_HIGH_1] = config->high1; - _mapping[documentapi::Priority::PRI_HIGH_2] = config->high2; - _mapping[documentapi::Priority::PRI_HIGH_3] = config->high3; - _mapping[documentapi::Priority::PRI_NORMAL_1] = config->normal1; - _mapping[documentapi::Priority::PRI_NORMAL_2] = config->normal2; - _mapping[documentapi::Priority::PRI_NORMAL_3] = config->normal3; - _mapping[documentapi::Priority::PRI_NORMAL_4] = config->normal4; - _mapping[documentapi::Priority::PRI_NORMAL_5] = config->normal5; - _mapping[documentapi::Priority::PRI_NORMAL_6] = config->normal6; - _mapping[documentapi::Priority::PRI_LOW_1] = config->low1; - _mapping[documentapi::Priority::PRI_LOW_2] = config->low2; - _mapping[documentapi::Priority::PRI_LOW_3] = config->low3; - _mapping[documentapi::Priority::PRI_VERY_LOW] = config->veryLow; - _mapping[documentapi::Priority::PRI_LOWEST] = config->lowest; - - std::lock_guard guard(_mutex); - _reverseMapping.clear(); - _reverseMapping[config->highest] = documentapi::Priority::PRI_HIGHEST; - _reverseMapping[config->veryHigh] = documentapi::Priority::PRI_VERY_HIGH; - _reverseMapping[config->high1] = documentapi::Priority::PRI_HIGH_1; - _reverseMapping[config->high2] = documentapi::Priority::PRI_HIGH_2; - _reverseMapping[config->high3] = documentapi::Priority::PRI_HIGH_3; - _reverseMapping[config->normal1] = documentapi::Priority::PRI_NORMAL_1; - _reverseMapping[config->normal2] = documentapi::Priority::PRI_NORMAL_2; - _reverseMapping[config->normal3] = documentapi::Priority::PRI_NORMAL_3; - _reverseMapping[config->normal4] = documentapi::Priority::PRI_NORMAL_4; - _reverseMapping[config->normal5] = documentapi::Priority::PRI_NORMAL_5; - _reverseMapping[config->normal6] = documentapi::Priority::PRI_NORMAL_6; - _reverseMapping[config->low1] = documentapi::Priority::PRI_LOW_1; - _reverseMapping[config->low2] = documentapi::Priority::PRI_LOW_2; - _reverseMapping[config->low3] = documentapi::Priority::PRI_LOW_3; - _reverseMapping[config->veryLow] = documentapi::Priority::PRI_VERY_LOW; - _reverseMapping[config->lowest] = documentapi::Priority::PRI_LOWEST; + const auto index = static_cast<uint32_t>(documentApiPriority); + if (index >= PRI_ENUM_SIZE) { + return 255; + } + return _mapping[index]; } } // storage diff --git a/storage/src/vespa/storage/storageserver/priorityconverter.h b/storage/src/vespa/storage/storageserver/priorityconverter.h index 47326e54243..48c7424433b 100644 --- a/storage/src/vespa/storage/storageserver/priorityconverter.h +++ b/storage/src/vespa/storage/storageserver/priorityconverter.h @@ -2,50 +2,34 @@ #pragma once -#include <vespa/storage/config/config-stor-prioritymapping.h> -#include <vespa/config/helper/ifetchercallback.h> #include <vespa/documentapi/messagebus/priority.h> -#include <atomic> #include <array> -#include <mutex> - -namespace config { - class ConfigUri; - class ConfigFetcher; -} +#include <vector> namespace storage { -class PriorityConverter - : public config::IFetcherCallback< - vespa::config::content::core::StorPrioritymappingConfig> -{ +class PriorityConverter { public: - using Config = vespa::config::content::core::StorPrioritymappingConfig; - - explicit PriorityConverter(const config::ConfigUri& configUri); - ~PriorityConverter() override; + PriorityConverter(); + ~PriorityConverter(); /** Converts the given priority into a storage api priority number. */ - uint8_t toStoragePriority(documentapi::Priority::Value) const; + [[nodiscard]] uint8_t toStoragePriority(documentapi::Priority::Value) const noexcept; /** Converts the given priority into a document api priority number. */ - documentapi::Priority::Value toDocumentPriority(uint8_t) const; - - void configure(std::unique_ptr<Config> config) override; + [[nodiscard]] documentapi::Priority::Value toDocumentPriority(uint8_t storage_priority) const noexcept { + return _reverse_mapping[storage_priority]; + } private: - static_assert(documentapi::Priority::PRI_ENUM_SIZE == 16, - "Unexpected size of priority enumeration"); - static_assert(documentapi::Priority::PRI_LOWEST == 15, - "Priority enum value out of bounds"); - static constexpr size_t PRI_ENUM_SIZE = documentapi::Priority::PRI_ENUM_SIZE; + void init_static_priority_mappings(); - std::array<std::atomic<uint8_t>, PRI_ENUM_SIZE> _mapping; - std::map<uint8_t, documentapi::Priority::Value> _reverseMapping; - mutable std::mutex _mutex; + static_assert(documentapi::Priority::PRI_ENUM_SIZE == 16, "Unexpected size of priority enumeration"); + static_assert(documentapi::Priority::PRI_LOWEST == 15, "Priority enum value out of bounds"); + static constexpr size_t PRI_ENUM_SIZE = documentapi::Priority::PRI_ENUM_SIZE; - std::unique_ptr<config::ConfigFetcher> _configFetcher; + std::array<uint8_t, PRI_ENUM_SIZE> _mapping; + std::vector<documentapi::Priority::Value> _reverse_mapping; }; } // storage |