Merge pull request #28964 from vespa-engine/vekterli/make-operation-priority-mapping-static

Remove unused configurability of operation priorities

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