diff options
Diffstat (limited to 'storage/src/tests')
42 files changed, 737 insertions, 538 deletions
diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp index 92547a83d25..437e8a14e3b 100644 --- a/storage/src/tests/bucketdb/bucketmanagertest.cpp +++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp @@ -3,6 +3,7 @@ #include <tests/common/dummystoragelink.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> +#include <tests/common/storage_config_set.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/config/config-documenttypes.h> #include <vespa/document/datatype/documenttype.h> @@ -23,7 +24,6 @@ #include <vespa/vdslib/state/random.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/config-stor-filestor.h> #include <future> #include <vespa/log/log.h> @@ -59,6 +59,7 @@ struct TestParams; struct BucketManagerTest : public Test { public: + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<DummyStorageLink> _top; BucketManager *_manager; @@ -69,8 +70,7 @@ public: ~BucketManagerTest() override; - void setupTestEnvironment(bool fakePersistenceLayer = true, - bool noDelete = false); + void setupTestEnvironment(); void addBucketsToDB(uint32_t count); bool wasBlockedDueToLastModified(api::StorageMessage* msg, uint64_t lastModified); void insertSingleBucket(const document::BucketId& bucket, const api::BucketInfo& info); @@ -125,46 +125,24 @@ BucketManagerTest::~BucketManagerTest() = default; FAIL() << ost.str(); \ } -std::string getMkDirDisk(const std::string & rootFolder, int disk) { - std::ostringstream os; - os << "mkdir -p " << rootFolder << "/disks/d" << disk; - return os.str(); -} - -void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noDelete) +void BucketManagerTest::setupTestEnvironment() { - vdstestlib::DirConfig config(getStandardConfig(true, "bucketmanagertest")); - std::string rootFolder = getRootFolder(config); - if (!noDelete) { - assert(system(("rm -rf " + rootFolder).c_str()) == 0); - } - assert(system(getMkDirDisk(rootFolder, 0).c_str()) == 0); - assert(system(getMkDirDisk(rootFolder, 1).c_str()) == 0); - + _config = StorageConfigSet::make_storage_node_config(); auto repo = std::make_shared<const DocumentTypeRepo>( *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec("../config-doctypes.cfg"))); _top = std::make_unique<DummyStorageLink>(); - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), config.getConfigId()); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); _node->setTypeRepo(repo); _node->setupDummyPersistence(); // Set up the 3 links - auto config_uri = config::ConfigUri(config.getConfigId()); using vespa::config::content::core::StorServerConfig; - auto manager = std::make_unique<BucketManager>(*config_from<StorServerConfig>(config_uri), _node->getComponentRegister()); + auto manager = std::make_unique<BucketManager>(*config_from<StorServerConfig>(_config->config_uri()), _node->getComponentRegister()); _manager = manager.get(); _top->push_back(std::move(manager)); - if (fakePersistenceLayer) { - auto bottom = std::make_unique<DummyStorageLink>(); - _bottom = bottom.get(); - _top->push_back(std::move(bottom)); - } else { - using StorFilestorConfig = vespa::config::content::internal::InternalStorFilestorType; - auto bottom = std::make_unique<FileStorManager>(*config_from<StorFilestorConfig>(config_uri), - _node->getPersistenceProvider(), _node->getComponentRegister(), - *_node, _node->get_host_info()); - _top->push_back(std::move(bottom)); - } - // Generate a doc to use for testing.. + auto bottom = std::make_unique<DummyStorageLink>(); + _bottom = bottom.get(); + _top->push_back(std::move(bottom)); + const DocumentType &type(*_node->getTypeRepo()->getDocumentType("text/html")); _document = std::make_shared<document::Document>(*_node->getTypeRepo(), type, document::DocumentId("id:ns:text/html::ntnu")); } diff --git a/storage/src/tests/common/CMakeLists.txt b/storage/src/tests/common/CMakeLists.txt index 36487660cce..0e075e39194 100644 --- a/storage/src/tests/common/CMakeLists.txt +++ b/storage/src/tests/common/CMakeLists.txt @@ -3,7 +3,7 @@ vespa_add_library(storage_testcommon TEST SOURCES dummystoragelink.cpp message_sender_stub.cpp - testhelper.cpp + storage_config_set.cpp testnodestateupdater.cpp teststorageapp.cpp DEPENDS diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp index 899c1979e86..6ca84f0304a 100644 --- a/storage/src/tests/common/metricstest.cpp +++ b/storage/src/tests/common/metricstest.cpp @@ -1,5 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> +#include <tests/common/teststorageapp.h> +#include <tests/common/testhelper.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageframework/defaultimplementation/clock/fakeclock.h> #include <vespa/storage/bucketdb/bucketmanager.h> @@ -7,9 +11,6 @@ #include <vespa/storage/persistence/filestorage/filestormanager.h> #include <vespa/storage/persistence/filestorage/filestormetrics.h> #include <vespa/storage/visiting/visitormetrics.h> -#include <tests/common/teststorageapp.h> -#include <tests/common/testhelper.h> -#include <tests/common/dummystoragelink.h> #include <vespa/metrics/metricmanager.h> #include <vespa/config/common/exceptions.h> #include <vespa/vespalib/gtest/gtest.h> @@ -26,10 +27,10 @@ namespace storage { struct MetricsTest : public Test { framework::defaultimplementation::FakeClock* _clock; + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<DummyStorageLink> _top; std::unique_ptr<StatusMetricConsumer> _metricsConsumer; - std::unique_ptr<vdstestlib::DirConfig> _config; std::unique_ptr<metrics::MetricSet> _topSet; std::unique_ptr<metrics::MetricManager> _metricManager; std::shared_ptr<FileStorMetrics> _filestorMetrics; @@ -66,17 +67,13 @@ MetricsTest::MetricsTest() MetricsTest::~MetricsTest() = default; void MetricsTest::SetUp() { - _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "metricstest")); - std::filesystem::remove_all(std::filesystem::path(getRootFolder(*_config))); - try { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); - _node->setupDummyPersistence(); - _clock = &_node->getClock(); - _clock->setAbsoluteTimeInSeconds(1000000); - _top = std::make_unique<DummyStorageLink>(); - } catch (config::InvalidConfigException& e) { - fprintf(stderr, "%s\n", e.what()); - } + _config = StorageConfigSet::make_storage_node_config(); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); + _node->setupDummyPersistence(); + _clock = &_node->getClock(); + _clock->setAbsoluteTimeInSeconds(1000000); + _top = std::make_unique<DummyStorageLink>(); + _metricManager = std::make_unique<metrics::MetricManager>(std::make_unique<MetricClock>(*_clock)); _topSet.reset(new metrics::MetricSet("vds", {}, "")); { @@ -96,7 +93,7 @@ void MetricsTest::SetUp() { _visitorMetrics = std::make_shared<VisitorMetrics>(); _visitorMetrics->initThreads(4); _topSet->registerMetric(*_visitorMetrics); - _metricManager->init(config::ConfigUri(_config->getConfigId())); + _metricManager->init(_config->config_uri()); } void MetricsTest::TearDown() { diff --git a/storage/src/tests/common/storage_config_set.cpp b/storage/src/tests/common/storage_config_set.cpp new file mode 100644 index 00000000000..ed10e113867 --- /dev/null +++ b/storage/src/tests/common/storage_config_set.cpp @@ -0,0 +1,153 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "storage_config_set.h" +#include <vespa/config-bucketspaces.h> +#include <vespa/config-persistence.h> +#include <vespa/config-slobroks.h> +#include <vespa/config-stor-distribution.h> +#include <vespa/config-stor-filestor.h> +#include <vespa/config-upgrading.h> +#include <vespa/document/repo/configbuilder.h> +#include <vespa/document/repo/document_type_repo_factory.h> +#include <vespa/document/repo/documenttyperepo.h> +#include <vespa/messagebus/config-messagebus.h> +#include <vespa/messagebus/testlib/slobrok.h> +#include <vespa/metrics/config-metricsmanager.h> +#include <vespa/storage/config/config-stor-bouncer.h> +#include <vespa/storage/config/config-stor-communicationmanager.h> +#include <vespa/storage/config/config-stor-distributormanager.h> +#include <vespa/storage/config/config-stor-prioritymapping.h> +#include <vespa/storage/config/config-stor-server.h> +#include <vespa/storage/config/config-stor-status.h> +#include <vespa/storage/config/config-stor-visitordispatcher.h> +#include <vespa/storage/visiting/config-stor-visitor.h> +#include <vespa/document/base/testdocrepo.h> +#include <vespa/vespalib/util/stringfmt.h> + +namespace storage { + +StorageConfigSet::StorageConfigSet(vespalib::string config_id_str, bool is_storage_node) + : _document_type_config(std::make_unique<DocumenttypesConfigBuilder>()), + _slobroks_config(std::make_unique<SlobroksConfigBuilder>()), + _messagebus_config(std::make_unique<MessagebusConfigBuilder>()), + _metrics_config(std::make_unique<MetricsmanagerConfigBuilder>()), + _persistence_config(std::make_unique<PersistenceConfigBuilder>()), + _distribution_config(std::make_unique<StorDistributionConfigBuilder>()), + _filestor_config(std::make_unique<StorFilestorConfigBuilder>()), + _upgrading_config(std::make_unique<UpgradingConfigBuilder>()), + _bucket_spaces_config(std::make_unique<BucketspacesConfigBuilder>()), + _bouncer_config(std::make_unique<StorBouncerConfigBuilder>()), + _communication_manager_config(std::make_unique<StorCommunicationmanagerConfigBuilder>()), + _distributor_manager_config(std::make_unique<StorDistributormanagerConfigBuilder>()), + _priority_mapping_config(std::make_unique<StorPrioritymappingConfigBuilder>()), + _server_config(std::make_unique<StorServerConfigBuilder>()), + _status_config(std::make_unique<StorStatusConfigBuilder>()), + _visitor_config(std::make_unique<StorVisitorConfigBuilder>()), + _visitor_dispatcher_config(std::make_unique<StorVisitordispatcherConfigBuilder>()), + _config_id_str(std::move(config_id_str)), + _config_ctx(std::make_shared<config::ConfigContext>(_config_set)), + _config_uri(_config_id_str, _config_ctx) +{ + _config_set.addBuilder(_config_id_str, _document_type_config.get()); + _config_set.addBuilder(_config_id_str, _slobroks_config.get()); + _config_set.addBuilder(_config_id_str, _messagebus_config.get()); + _config_set.addBuilder(_config_id_str, _metrics_config.get()); + _config_set.addBuilder(_config_id_str, _persistence_config.get()); + _config_set.addBuilder(_config_id_str, _distribution_config.get()); + _config_set.addBuilder(_config_id_str, _filestor_config.get()); + _config_set.addBuilder(_config_id_str, _upgrading_config.get()); + _config_set.addBuilder(_config_id_str, _bucket_spaces_config.get()); + _config_set.addBuilder(_config_id_str, _bouncer_config.get()); + _config_set.addBuilder(_config_id_str, _communication_manager_config.get()); + _config_set.addBuilder(_config_id_str, _distributor_manager_config.get()); + _config_set.addBuilder(_config_id_str, _priority_mapping_config.get()); + _config_set.addBuilder(_config_id_str, _server_config.get()); + _config_set.addBuilder(_config_id_str, _status_config.get()); + _config_set.addBuilder(_config_id_str, _visitor_config.get()); + _config_set.addBuilder(_config_id_str, _visitor_dispatcher_config.get()); + + init_default_configs(is_storage_node); + _config_ctx->reload(); +} + +StorageConfigSet::~StorageConfigSet() = default; + +void StorageConfigSet::init_default_configs(bool is_storage_node) { + // Most configs are left with their default values, with explicit values being a + // union of the legacy DirConfig test helpers. + *_document_type_config = document::TestDocRepo().getTypeConfig(); + + add_metric_consumer("status", {"*"}); + add_metric_consumer("statereporter", {"*"}); + + add_distribution_config(50); + add_bucket_space_mapping("testdoctype1", "default"); + + _communication_manager_config->rpcport = 0; + _communication_manager_config->mbusport = 0; + + _distributor_manager_config->splitcount = 1000; + _distributor_manager_config->splitsize = 10000000; + _distributor_manager_config->joincount = 500; + _distributor_manager_config->joinsize = 5000000; + _distributor_manager_config->maxClusterClockSkewSec = 0; + + _filestor_config->numThreads = 1; + _filestor_config->numResponseThreads = 1; + + _persistence_config->abortOperationsWithChangedBucketOwnership = true; + + _server_config->clusterName = "storage"; + _server_config->nodeIndex = 0; + _server_config->isDistributor = !is_storage_node; + _server_config->maxMergesPerNode = 25; + _server_config->maxMergeQueueSize = 20; + _server_config->resourceExhaustionMergeBackPressureDurationSecs = 15.0; + _server_config->writePidFileOnStartup = false; + + _status_config->httpport = 0; + + _visitor_config->maxconcurrentvisitorsFixed = 4; + _visitor_config->maxconcurrentvisitorsVariable = 0; +} + +void StorageConfigSet::add_bucket_space_mapping(vespalib::string doc_type, vespalib::string bucket_space_name) { + BucketspacesConfigBuilder::Documenttype type; + type.name = std::move(doc_type); + type.bucketspace = std::move(bucket_space_name); + _bucket_spaces_config->documenttype.emplace_back(std::move(type)); +} + +void StorageConfigSet::add_distribution_config(uint16_t nodes_in_top_level_group) { + StorDistributionConfigBuilder::Group group; + group.name = "invalid"; + group.index = "invalid"; + for (uint16_t i = 0; i < nodes_in_top_level_group; ++i) { + StorDistributionConfigBuilder::Group::Nodes node; + node.index = i; + group.nodes.emplace_back(std::move(node)); + } + _distribution_config->group.clear(); + _distribution_config->group.emplace_back(std::move(group)); + _distribution_config->redundancy = 2; +} + +void StorageConfigSet::add_metric_consumer(vespalib::string name, const std::vector<vespalib::string>& added_metrics) { + MetricsmanagerConfigBuilder::Consumer consumer; + consumer.name = std::move(name); + consumer.addedmetrics.assign(added_metrics.begin(), added_metrics.end()); + _metrics_config->consumer.emplace_back(std::move(consumer)); +} + +void StorageConfigSet::set_node_index(uint16_t node_index) { + _server_config->nodeIndex = node_index; +} + +void StorageConfigSet::set_slobrok_config_port(int slobrok_port) { + SlobroksConfigBuilder::Slobrok slobrok; + slobrok.connectionspec = vespalib::make_string("tcp/localhost:%d", slobrok_port); + _slobroks_config->slobrok.clear(); + _slobroks_config->slobrok.emplace_back(std::move(slobrok)); +} + +} // storage diff --git a/storage/src/tests/common/storage_config_set.h b/storage/src/tests/common/storage_config_set.h new file mode 100644 index 00000000000..66cdeaf527f --- /dev/null +++ b/storage/src/tests/common/storage_config_set.h @@ -0,0 +1,122 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/config/common/configcontext.h> +#include <vespa/config/subscription/configuri.h> +#include <vespa/config/subscription/sourcespec.h> +#include <memory> + +// FIXME "internal" here is very counter-productive since it precludes easy fwd decls. +// Currently have to punch holes in internal abstractions to make this work at all. +namespace cloud::config::internal { class InternalSlobroksType; } +namespace messagebus::internal { class InternalMessagebusType; } +namespace metrics::internal { class InternalMetricsmanagerType; } +namespace document::config::internal { class InternalDocumenttypesType; } +namespace vespa::config::content::internal { +class InternalPersistenceType; +class InternalStorDistributionType; +class InternalStorFilestorType; +class InternalUpgradingType; +} +namespace vespa::config::content::core::internal { +class InternalBucketspacesType; +class InternalStorBouncerType; +class InternalStorCommunicationmanagerType; +class InternalStorDistributormanagerType; +class InternalStorPrioritymappingType; +class InternalStorServerType; +class InternalStorStatusType; +class InternalStorVisitorType; +class InternalStorVisitordispatcherType; +} + +namespace storage { + +class StorageConfigSet { + using SlobroksConfigBuilder = cloud::config::internal::InternalSlobroksType; + using MessagebusConfigBuilder = messagebus::internal::InternalMessagebusType; + using MetricsmanagerConfigBuilder = metrics::internal::InternalMetricsmanagerType; + using DocumenttypesConfigBuilder = document::config::internal::InternalDocumenttypesType; + using PersistenceConfigBuilder = vespa::config::content::internal::InternalPersistenceType; + using StorDistributionConfigBuilder = vespa::config::content::internal::InternalStorDistributionType; + using StorFilestorConfigBuilder = vespa::config::content::internal::InternalStorFilestorType; + using UpgradingConfigBuilder = vespa::config::content::internal::InternalUpgradingType; + using BucketspacesConfigBuilder = vespa::config::content::core::internal::InternalBucketspacesType; + using StorBouncerConfigBuilder = vespa::config::content::core::internal::InternalStorBouncerType; + using StorCommunicationmanagerConfigBuilder = vespa::config::content::core::internal::InternalStorCommunicationmanagerType; + using StorDistributormanagerConfigBuilder = vespa::config::content::core::internal::InternalStorDistributormanagerType; + using StorPrioritymappingConfigBuilder = vespa::config::content::core::internal::InternalStorPrioritymappingType; + using StorServerConfigBuilder = vespa::config::content::core::internal::InternalStorServerType; + using StorStatusConfigBuilder = vespa::config::content::core::internal::InternalStorStatusType; + using StorVisitorConfigBuilder = vespa::config::content::core::internal::InternalStorVisitorType; + using StorVisitordispatcherConfigBuilder = vespa::config::content::core::internal::InternalStorVisitordispatcherType; + + std::unique_ptr<DocumenttypesConfigBuilder> _document_type_config; + std::unique_ptr<SlobroksConfigBuilder> _slobroks_config; + std::unique_ptr<MessagebusConfigBuilder> _messagebus_config; + std::unique_ptr<MetricsmanagerConfigBuilder> _metrics_config; + std::unique_ptr<PersistenceConfigBuilder> _persistence_config; + std::unique_ptr<StorDistributionConfigBuilder> _distribution_config; + std::unique_ptr<StorFilestorConfigBuilder> _filestor_config; + std::unique_ptr<UpgradingConfigBuilder> _upgrading_config; + std::unique_ptr<BucketspacesConfigBuilder> _bucket_spaces_config; + std::unique_ptr<StorBouncerConfigBuilder> _bouncer_config; + std::unique_ptr<StorCommunicationmanagerConfigBuilder> _communication_manager_config; + std::unique_ptr<StorDistributormanagerConfigBuilder> _distributor_manager_config; + std::unique_ptr<StorPrioritymappingConfigBuilder> _priority_mapping_config; // TODO removable? + std::unique_ptr<StorServerConfigBuilder> _server_config; + std::unique_ptr<StorStatusConfigBuilder> _status_config; + std::unique_ptr<StorVisitorConfigBuilder> _visitor_config; + std::unique_ptr<StorVisitordispatcherConfigBuilder> _visitor_dispatcher_config; + + vespalib::string _config_id_str; + config::ConfigSet _config_set; + std::shared_ptr<config::ConfigContext> _config_ctx; + config::ConfigUri _config_uri; + +public: + StorageConfigSet(vespalib::string config_id_str, bool is_storage_node); + ~StorageConfigSet(); + + void init_default_configs(bool is_storage_node); + void add_bucket_space_mapping(vespalib::string doc_type, vespalib::string bucket_space_name); + void add_metric_consumer(vespalib::string name, const std::vector<vespalib::string>& added_metrics); + void add_distribution_config(uint16_t nodes_in_top_level_group); + void set_slobrok_config_port(int slobrok_port); + void set_node_index(uint16_t node_index); + + [[nodiscard]] const config::ConfigUri& config_uri() const noexcept { + return _config_uri; + } + + DocumenttypesConfigBuilder& document_type_config() noexcept { return *_document_type_config; } + SlobroksConfigBuilder& slobroks_config() noexcept { return *_slobroks_config; } + MessagebusConfigBuilder& messagebus_config() noexcept {return *_messagebus_config; } + MetricsmanagerConfigBuilder& metrics_config() noexcept { return *_metrics_config; } + PersistenceConfigBuilder& persistence_config() noexcept { return *_persistence_config; } + StorDistributionConfigBuilder& distribution_config() noexcept { return *_distribution_config; } + StorFilestorConfigBuilder& filestor_config() noexcept { return *_filestor_config; } + BucketspacesConfigBuilder& bucket_spaces_config() noexcept { return *_bucket_spaces_config; } + StorBouncerConfigBuilder& bouncer_config() noexcept { return *_bouncer_config; }; + StorCommunicationmanagerConfigBuilder& communication_manager_config() noexcept { return *_communication_manager_config; } + StorDistributormanagerConfigBuilder& distributor_manager_config() noexcept { return *_distributor_manager_config; } + StorServerConfigBuilder& server_config() noexcept { return *_server_config; } + StorStatusConfigBuilder& status_config() noexcept { return *_status_config; } + StorVisitorConfigBuilder& visitor_config() noexcept { return *_visitor_config; } + StorVisitordispatcherConfigBuilder& visitor_dispatcher_config() noexcept { return *_visitor_dispatcher_config; } + + [[nodiscard]] static std::unique_ptr<StorageConfigSet> make_node_config(bool is_storage_node) { + return std::make_unique<StorageConfigSet>("my-node", is_storage_node); + } + + [[nodiscard]] static std::unique_ptr<StorageConfigSet> make_storage_node_config() { + return make_node_config(true); + } + + [[nodiscard]] static std::unique_ptr<StorageConfigSet> make_distributor_node_config() { + return make_node_config(false); + } +}; + +} diff --git a/storage/src/tests/common/testhelper.cpp b/storage/src/tests/common/testhelper.cpp deleted file mode 100644 index 4ca935b7904..00000000000 --- a/storage/src/tests/common/testhelper.cpp +++ /dev/null @@ -1,148 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <tests/common/testhelper.h> - -#include <vespa/log/log.h> -#include <vespa/vespalib/io/fileutil.h> -#include <vespa/vespalib/testkit/test_kit.h> - -LOG_SETUP(".testhelper"); - -namespace storage { - -void addStorageDistributionConfig(vdstestlib::DirConfig& dc) -{ - vdstestlib::DirConfig::Config* config; - config = &dc.getConfig("stor-distribution", true); - config->clear(); - config->set("group[1]"); - config->set("group[0].name", "invalid"); - config->set("group[0].index", "invalid"); - config->set("group[0].nodes[50]"); - config->set("redundancy", "2"); - - for (uint32_t i = 0; i < 50; i++) { - std::ostringstream key; key << "group[0].nodes[" << i << "].index"; - std::ostringstream val; val << i; - config->set(key.str(), val.str()); - } -} - -std::string getRootFolder(vdstestlib::DirConfig & dc) { - std::string defaultValue(""); - return dc.getConfig("stor-server").getValue("root_folder", defaultValue); -} - -vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & rootOfRoot) { - std::string clusterName("storage"); - vdstestlib::DirConfig dc; - vdstestlib::DirConfig::Config* config; - config = &dc.addConfig("fleetcontroller"); - config->set("cluster_name", clusterName); - config->set("index", "0"); - config->set("zookeeper_server", "\"\""); - config->set("total_distributor_count", "10"); - config->set("total_storage_count", "10"); - config = &dc.addConfig("upgrading"); - config = &dc.addConfig("load-type"); - config = &dc.addConfig("bucket"); - config = &dc.addConfig("messagebus"); - config = &dc.addConfig("stor-prioritymapping"); - config = &dc.addConfig("stor-bucketdbupdater"); - config = &dc.addConfig("metricsmanager"); - config->set("consumer[2]"); - config->set("consumer[0].name", "\"status\""); - config->set("consumer[0].addedmetrics[1]"); - config->set("consumer[0].addedmetrics[0]", "\"*\""); - config->set("consumer[1].name", "\"statereporter\""); - config->set("consumer[1].addedmetrics[1]"); - config->set("consumer[1].addedmetrics[0]", "\"*\""); - config = &dc.addConfig("stor-communicationmanager"); - config->set("rpcport", "0"); - config->set("mbusport", "0"); - config = &dc.addConfig("stor-distributormanager"); - config->set("splitcount", "1000"); - config->set("splitsize", "10000000"); - config->set("joincount", "500"); - config->set("joinsize", "5000000"); - config->set("max_clock_skew_sec", "0"); - config = &dc.addConfig("persistence"); - config->set("abort_operations_with_changed_bucket_ownership", "true"); - config = &dc.addConfig("stor-filestor"); - // Easier to see what goes wrong with only 1 thread per disk. - config->set("num_threads", "1"); - config->set("num_response_threads", "1"); - config->set("maximum_versions_of_single_document_stored", "0"); - config->set("keep_remove_time_period", "2000000000"); - config->set("revert_time_period", "2000000000"); - // Don't want test to call exit() - config->set("fail_disk_after_error_count", "0"); - config = &dc.addConfig("stor-bouncer"); - config = &dc.addConfig("stor-server"); - config->set("cluster_name", clusterName); - config->set("enable_dead_lock_detector", "false"); - config->set("enable_dead_lock_detector_warnings", "false"); - config->set("max_merges_per_node", "25"); - config->set("max_merge_queue_size", "20"); - config->set("resource_exhaustion_merge_back_pressure_duration_secs", "15.0"); - vespalib::string rootFolder = rootOfRoot + "_"; - rootFolder += (storagenode ? "vdsroot" : "vdsroot.distributor"); - config->set("root_folder", rootFolder); - config->set("is_distributor", (storagenode ? "false" : "true")); - config = &dc.addConfig("stor-devices"); - config->set("root_folder", rootFolder); - config = &dc.addConfig("stor-status"); - config->set("httpport", "0"); - config = &dc.addConfig("stor-visitor"); - config->set("defaultdocblocksize", "8192"); - // By default, need "old" behaviour of maxconcurrent - config->set("maxconcurrentvisitors_fixed", "4"); - config->set("maxconcurrentvisitors_variable", "0"); - config = &dc.addConfig("stor-visitordispatcher"); - addFileConfig(dc, "documenttypes", TEST_PATH("config-doctypes.cfg")); - addStorageDistributionConfig(dc); - return dc; -} - -void addSlobrokConfig(vdstestlib::DirConfig& dc, - const mbus::Slobrok& slobrok) -{ - std::ostringstream ost; - ost << "tcp/localhost:" << slobrok.port(); - vdstestlib::DirConfig::Config* config; - config = &dc.getConfig("slobroks", true); - config->clear(); - config->set("slobrok[1]"); - config->set("slobrok[0].connectionspec", ost.str()); -} - -void addFileConfig(vdstestlib::DirConfig& dc, - const std::string& configDefName, - const std::string& fileName) -{ - vdstestlib::DirConfig::Config* config; - config = &dc.getConfig(configDefName, true); - config->clear(); - std::ifstream in(fileName.c_str()); - std::string line; - while (std::getline(in, line, '\n')) { - std::string::size_type pos = line.find(' '); - if (pos == std::string::npos) { - config->set(line); - } else { - config->set(line.substr(0, pos), line.substr(pos + 1)); - } - } - in.close(); -} - -TestName::TestName(const std::string& n) - : name(n) -{ - LOG(debug, "Starting test %s", name.c_str()); -} - -TestName::~TestName() { - LOG(debug, "Done with test %s", name.c_str()); -} - -} // storage diff --git a/storage/src/tests/common/testhelper.h b/storage/src/tests/common/testhelper.h index 1f83e938409..9f9b50652f9 100644 --- a/storage/src/tests/common/testhelper.h +++ b/storage/src/tests/common/testhelper.h @@ -1,39 +1,15 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once + #include <vespa/config/helper/configgetter.h> -#include <vespa/messagebus/testlib/slobrok.h> -#include <vespa/vdstestlib/config/dirconfig.h> -#include <fstream> -#include <sstream> +#include <vespa/config/subscription/configuri.h> namespace storage { -void addFileConfig(vdstestlib::DirConfig& dc, - const std::string& configDefName, - const std::string& fileName); - - -void addStorageDistributionConfig(vdstestlib::DirConfig& dc); - -vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & rootFolder = "todo-make-unique"); - -std::string getRootFolder(vdstestlib::DirConfig & dc); - -void addSlobrokConfig(vdstestlib::DirConfig& dc, - const mbus::Slobrok& slobrok); - template <typename ConfigT> std::unique_ptr<ConfigT> config_from(const ::config::ConfigUri& cfg_uri) { return ::config::ConfigGetter<ConfigT>::getConfig(cfg_uri.getConfigId(), cfg_uri.getContext()); } -// Class used to print start and end of test. Enable debug when you want to see -// which test creates what output or where we get stuck -struct TestName { - std::string name; - TestName(const std::string& n); - ~TestName(); -}; - } // storage diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index e2e2de10702..d811f100aec 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.cpp @@ -26,31 +26,31 @@ namespace storage { TestStorageApp::TestStorageApp(StorageComponentRegisterImpl::UP compReg, const lib::NodeType& type, NodeIndex index, - vespalib::stringref configId) + const config::ConfigUri& config_uri) : TestComponentRegister(ComponentRegisterImpl::UP(std::move(compReg))), _compReg(dynamic_cast<StorageComponentRegisterImpl&>(TestComponentRegister::getComponentRegister())), _docMan(), _nodeStateUpdater(type), - _configId(configId), + _configId(config_uri.getConfigId()), _node_identity("test_cluster", type, index), _initialized(false) { - // Use config to adjust values + // Use config to adjust values vespalib::string clusterName = "mycluster"; uint32_t redundancy = 2; uint32_t nodeCount = 10; - if (!configId.empty()) { - config::ConfigUri uri(configId); - std::unique_ptr<vespa::config::content::core::StorServerConfig> serverConfig = config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(uri.getConfigId(), uri.getContext()); - clusterName = serverConfig->clusterName; - if (index == 0xffff) index = serverConfig->nodeIndex; - redundancy = config::ConfigGetter<vespa::config::content::StorDistributionConfig>::getConfig(uri.getConfigId(), uri.getContext())->redundancy; - nodeCount = config::ConfigGetter<vespa::config::content::FleetcontrollerConfig>::getConfig(uri.getConfigId(), uri.getContext())->totalStorageCount; - } else { - if (index == 0xffff) index = 0; + auto serverConfig = config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(config_uri.getConfigId(), config_uri.getContext()); + clusterName = serverConfig->clusterName; + if (index == 0xffff) { + index = serverConfig->nodeIndex; + } + redundancy = config::ConfigGetter<vespa::config::content::StorDistributionConfig>::getConfig(config_uri.getConfigId(), config_uri.getContext())->redundancy; + if (index >= nodeCount) { + nodeCount = index + 1; + } + if (redundancy > nodeCount) { + redundancy = nodeCount; } - if (index >= nodeCount) nodeCount = index + 1; - if (redundancy > nodeCount) redundancy = nodeCount; _compReg.setNodeInfo(clusterName, type, index); _compReg.setNodeStateUpdater(_nodeStateUpdater); @@ -84,21 +84,17 @@ TestStorageApp::setClusterState(const lib::ClusterState& c) } namespace { -NodeIndex getIndexFromConfig(vespalib::stringref configId) { - if (!configId.empty()) { - config::ConfigUri uri(configId); - return NodeIndex( - config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(uri.getConfigId(), uri.getContext())->nodeIndex); - } - return NodeIndex(0); + +NodeIndex node_index_from_config(const config::ConfigUri& uri) { + return NodeIndex(config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(uri.getConfigId(), uri.getContext())->nodeIndex); } VESPA_THREAD_STACK_TAG(test_executor) } -TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) +TestServiceLayerApp::TestServiceLayerApp(NodeIndex index, const config::ConfigUri& config_uri) : TestStorageApp(std::make_unique<ServiceLayerComponentRegisterImpl>(ContentBucketDbOptions()), - lib::NodeType::STORAGE, getIndexFromConfig(configId), configId), + lib::NodeType::STORAGE, index, config_uri), _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _persistenceProvider(), _executor(vespalib::SequencedTaskExecutor::create(test_executor, 1)), @@ -108,17 +104,9 @@ TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) _nodeStateUpdater.setReportedNodeState(ns); } -TestServiceLayerApp::TestServiceLayerApp(NodeIndex index, - vespalib::stringref configId) - : TestStorageApp(std::make_unique<ServiceLayerComponentRegisterImpl>(ContentBucketDbOptions()), - lib::NodeType::STORAGE, index, configId), - _compReg(dynamic_cast<ServiceLayerComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), - _persistenceProvider(), - _executor(vespalib::SequencedTaskExecutor::create(test_executor, 1)), - _host_info() +TestServiceLayerApp::TestServiceLayerApp(const config::ConfigUri& config_uri) + : TestServiceLayerApp(node_index_from_config(config_uri), config_uri) { - lib::NodeState ns(*_nodeStateUpdater.getReportedNodeState()); - _nodeStateUpdater.setReportedNodeState(ns); } TestServiceLayerApp::~TestServiceLayerApp() = default; @@ -147,45 +135,37 @@ TestServiceLayerApp::getPersistenceProvider() } namespace { - template<typename T> - T getConfig(vespalib::stringref configId) { - config::ConfigUri uri(configId); - return *config::ConfigGetter<T>::getConfig(uri.getConfigId(), uri.getContext()); - } + +template<typename T> +[[nodiscard]] T get_config(const config::ConfigUri& uri) { + return *config::ConfigGetter<T>::getConfig(uri.getConfigId(), uri.getContext()); +} + } void -TestDistributorApp::configure(vespalib::stringref id) +TestDistributorApp::configure(const config::ConfigUri& config_uri) { - if (id.empty()) return; - auto dc(getConfig<vespa::config::content::core::StorDistributormanagerConfig>(id)); + auto dc = get_config<vespa::config::content::core::StorDistributormanagerConfig>(config_uri); _compReg.setDistributorConfig(dc); - auto vc(getConfig<vespa::config::content::core::StorVisitordispatcherConfig>(id)); + auto vc = get_config<vespa::config::content::core::StorVisitordispatcherConfig>(config_uri); _compReg.setVisitorConfig(vc); } -TestDistributorApp::TestDistributorApp(vespalib::stringref configId) - : TestStorageApp( - std::make_unique<DistributorComponentRegisterImpl>(), - lib::NodeType::DISTRIBUTOR, getIndexFromConfig(configId), configId), +TestDistributorApp::TestDistributorApp(NodeIndex index, const config::ConfigUri& config_uri) + : TestStorageApp(std::make_unique<DistributorComponentRegisterImpl>(), + lib::NodeType::DISTRIBUTOR, index, config_uri), _compReg(dynamic_cast<DistributorComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), _lastUniqueTimestampRequested(0), _uniqueTimestampCounter(0) { _compReg.setTimeCalculator(*this); - configure(configId); + configure(config_uri); } -TestDistributorApp::TestDistributorApp(NodeIndex index, vespalib::stringref configId) - : TestStorageApp( - std::make_unique<DistributorComponentRegisterImpl>(), - lib::NodeType::DISTRIBUTOR, index, configId), - _compReg(dynamic_cast<DistributorComponentRegisterImpl&>(TestStorageApp::getComponentRegister())), - _lastUniqueTimestampRequested(0), - _uniqueTimestampCounter(0) +TestDistributorApp::TestDistributorApp(const config::ConfigUri& config_uri) + : TestDistributorApp(node_index_from_config(config_uri), config_uri) { - _compReg.setTimeCalculator(*this); - configure(configId); } TestDistributorApp::~TestDistributorApp() = default; diff --git a/storage/src/tests/common/teststorageapp.h b/storage/src/tests/common/teststorageapp.h index fb91145c66a..04fa6996e15 100644 --- a/storage/src/tests/common/teststorageapp.h +++ b/storage/src/tests/common/teststorageapp.h @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. /** - * \class storage::TestServiceLayerApp - * \ingroup common - * - * \brief Helper class for tests involving service layer. + * Helper class for tests involving service layer. * * Some components need some dependencies injected in order to work correctly. * This test class simplifies the process of creating these dependencies. @@ -34,6 +31,8 @@ #include <vespa/vespalib/util/sequencedtaskexecutor.h> #include <atomic> +namespace config { class ConfigUri; } + namespace storage { namespace spi { struct PersistenceProvider; } @@ -66,8 +65,8 @@ public: * from config themselves. */ TestStorageApp(StorageComponentRegisterImpl::UP compReg, - const lib::NodeType&, NodeIndex = NodeIndex(0xffff), - vespalib::stringref configId = ""); + const lib::NodeType&, NodeIndex index, + const config::ConfigUri& config_uri); ~TestStorageApp() override; // Set functions, to be able to modify content while running. @@ -110,8 +109,8 @@ class TestServiceLayerApp : public TestStorageApp HostInfo _host_info; public: - explicit TestServiceLayerApp(vespalib::stringref configId); - explicit TestServiceLayerApp(NodeIndex = NodeIndex(0xffff), vespalib::stringref configId = ""); + TestServiceLayerApp(NodeIndex node_index, const config::ConfigUri& config_uri); + explicit TestServiceLayerApp(const config::ConfigUri& config_uri); ~TestServiceLayerApp() override; void setupDummyPersistence(); @@ -140,11 +139,11 @@ class TestDistributorApp : public TestStorageApp, uint64_t _lastUniqueTimestampRequested; uint32_t _uniqueTimestampCounter; - void configure(vespalib::stringref configId); + void configure(const config::ConfigUri& config_uri); public: - explicit TestDistributorApp(vespalib::stringref configId = ""); - explicit TestDistributorApp(NodeIndex index, vespalib::stringref configId = ""); + TestDistributorApp(NodeIndex index, const config::ConfigUri& config_uri); + explicit TestDistributorApp(const config::ConfigUri& config_uri); ~TestDistributorApp() override; DistributorComponentRegisterImpl& getComponentRegister() override { diff --git a/storage/src/tests/distributor/distributor_stripe_test_util.cpp b/storage/src/tests/distributor/distributor_stripe_test_util.cpp index 923c7d1730b..f0b3db3adf8 100644 --- a/storage/src/tests/distributor/distributor_stripe_test_util.cpp +++ b/storage/src/tests/distributor/distributor_stripe_test_util.cpp @@ -32,7 +32,7 @@ DistributorStripeTestUtil::DistributorStripeTestUtil() _done_initializing(true), _messageSender(_sender, _senderDown) { - _config = getStandardConfig(false); + _config = StorageConfigSet::make_distributor_node_config(); } DistributorStripeTestUtil::~DistributorStripeTestUtil() = default; @@ -40,7 +40,7 @@ DistributorStripeTestUtil::~DistributorStripeTestUtil() = default; void DistributorStripeTestUtil::createLinks() { - _node = std::make_unique<TestDistributorApp>(_config.getConfigId()); + _node = std::make_unique<TestDistributorApp>(_config->config_uri()); _metrics = std::make_shared<DistributorMetricSet>(); _ideal_state_metrics = std::make_shared<IdealStateMetricSet>(); _stripe = std::make_unique<DistributorStripe>(_node->getComponentRegister(), *_metrics, *_ideal_state_metrics, @@ -184,8 +184,8 @@ DistributorStripeTestUtil::close() { _stripe->flush_and_close(); _sender.clear(); - _node.reset(0); - _config = getStandardConfig(false); + _node.reset(); + _config = StorageConfigSet::make_distributor_node_config(); } namespace { diff --git a/storage/src/tests/distributor/distributor_stripe_test_util.h b/storage/src/tests/distributor/distributor_stripe_test_util.h index 801320e2bf8..862d9bfbfba 100644 --- a/storage/src/tests/distributor/distributor_stripe_test_util.h +++ b/storage/src/tests/distributor/distributor_stripe_test_util.h @@ -5,7 +5,9 @@ #include <tests/common/dummystoragelink.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> +#include <tests/common/storage_config_set.h> #include <vespa/storage/common/hostreporter/hostinfo.h> +#include <vespa/storage/config/config-stor-distributormanager.h> #include <vespa/storage/distributor/stripe_host_info_notifier.h> #include <vespa/storage/storageutil/utils.h> @@ -132,8 +134,8 @@ public: const DistributorConfiguration& getConfig(); - vdstestlib::DirConfig& getDirConfig() { - return _config; + vespa::config::content::core::StorDistributormanagerConfigBuilder& backing_config() noexcept { + return _config->distributor_manager_config(); } // TODO explicit notion of bucket spaces for tests @@ -237,7 +239,7 @@ public: void tag_content_node_supports_condition_probing(uint16_t index, bool supported); protected: - vdstestlib::DirConfig _config; + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestDistributorApp> _node; std::shared_ptr<DistributorMetricSet> _metrics; std::shared_ptr<IdealStateMetricSet> _ideal_state_metrics; diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp index 634e4993d53..33da4727017 100644 --- a/storage/src/tests/distributor/externaloperationhandlertest.cpp +++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp @@ -94,7 +94,7 @@ struct ExternalOperationHandlerTest : Test, DistributorStripeTestUtil { TEST_F(ExternalOperationHandlerTest, bucket_split_mask) { { createLinks(); - getDirConfig().getConfig("stor-distributormanager").set("minsplitcount", "16"); + backing_config().minsplitcount = 16; EXPECT_EQ(document::BucketId(16, 0xffff), operation_context().make_split_bit_constrained_bucket_id(document::DocumentId( @@ -115,7 +115,7 @@ TEST_F(ExternalOperationHandlerTest, bucket_split_mask) { close(); } { - getDirConfig().getConfig("stor-distributormanager").set("minsplitcount", "20"); + backing_config().minsplitcount = 20; createLinks(); EXPECT_EQ(document::BucketId(20, 0x11111), operation_context().make_split_bit_constrained_bucket_id(document::DocumentId( diff --git a/storage/src/tests/distributor/idealstatemanagertest.cpp b/storage/src/tests/distributor/idealstatemanagertest.cpp index 0cadaa3fc9f..4639a154e74 100644 --- a/storage/src/tests/distributor/idealstatemanagertest.cpp +++ b/storage/src/tests/distributor/idealstatemanagertest.cpp @@ -74,10 +74,10 @@ TEST_F(IdealStateManagerTest, sibling) { TEST_F(IdealStateManagerTest, status_page) { close(); - getDirConfig().getConfig("stor-distributormanager").set("splitsize", "100"); - getDirConfig().getConfig("stor-distributormanager").set("splitcount", "1000000"); - getDirConfig().getConfig("stor-distributormanager").set("joinsize", "0"); - getDirConfig().getConfig("stor-distributormanager").set("joincount", "0"); + backing_config().splitsize = 100; + backing_config().splitcount = 1000000; + backing_config().joinsize = 0; + backing_config().joincount = 0; createLinks(); setup_stripe(1, 1, "distributor:1 storage:1"); diff --git a/storage/src/tests/distributor/operationtargetresolvertest.cpp b/storage/src/tests/distributor/operationtargetresolvertest.cpp index f0f8a4359fd..171dc5a42c0 100644 --- a/storage/src/tests/distributor/operationtargetresolvertest.cpp +++ b/storage/src/tests/distributor/operationtargetresolvertest.cpp @@ -27,8 +27,7 @@ struct OperationTargetResolverTest : Test, DistributorStripeTestUtil { const document::DocumentType* _html_type; std::unique_ptr<Operation> op; - BucketInstanceList getInstances(const BucketId& bid, - bool stripToRedundancy); + BucketInstanceList getInstances(const BucketId& bid, bool stripToRedundancy, bool symmetry_mode); void SetUp() override { _repo.reset(new document::DocumentTypeRepo( @@ -62,7 +61,7 @@ namespace { TestTargets::createTest(id, *this, *_asserters.back()) struct Asserter { - virtual ~Asserter() {} + virtual ~Asserter() = default; virtual void assertEqualMsg(std::string t1, OperationTargetList t2, OperationTargetList t3) = 0; @@ -73,21 +72,29 @@ struct TestTargets { OperationTargetList _expected; OperationTargetResolverTest& _test; Asserter& _asserter; + bool _symmetry_mode; TestTargets(const BucketId& id, OperationTargetResolverTest& test, Asserter& asserter) - : _id(id), _test(test), _asserter(asserter) {} + : _id(id), _test(test), _asserter(asserter), _symmetry_mode(true) + { + } ~TestTargets() { - BucketInstanceList result(_test.getInstances(_id, true)); - BucketInstanceList all(_test.getInstances(_id, false)); + BucketInstanceList result(_test.getInstances(_id, true, _symmetry_mode)); + BucketInstanceList all(_test.getInstances(_id, false, _symmetry_mode)); _asserter.assertEqualMsg( all.toString(), _expected, result.createTargets(makeBucketSpace())); delete _asserters.back(); _asserters.pop_back(); } + TestTargets& with_symmetric_replica_selection(bool symmetry) noexcept { + _symmetry_mode = symmetry; + return *this; + } + TestTargets& sendsTo(const BucketId& id, uint16_t node) { _expected.push_back(OperationTarget( makeDocumentBucket(id), lib::Node(lib::NodeType::STORAGE, node), false)); @@ -110,7 +117,7 @@ struct TestTargets { } // anonymous BucketInstanceList -OperationTargetResolverTest::getInstances(const BucketId& id, bool stripToRedundancy) +OperationTargetResolverTest::getInstances(const BucketId& id, bool stripToRedundancy, bool symmetry_mode) { auto &bucketSpaceRepo(operation_context().bucket_space_repo()); auto &distributorBucketSpace(bucketSpaceRepo.get(makeBucketSpace())); @@ -118,6 +125,7 @@ OperationTargetResolverTest::getInstances(const BucketId& id, bool stripToRedund distributorBucketSpace, distributorBucketSpace.getBucketDatabase(), 16, distributorBucketSpace.getDistribution().getRedundancy(), makeBucketSpace()); + resolver.use_symmetric_replica_selection(symmetry_mode); if (stripToRedundancy) { return resolver.getInstances(OperationTargetResolver::PUT, id); } else { @@ -143,14 +151,48 @@ TEST_F(OperationTargetResolverTest, choose_ideal_state_when_many_copies) { .sendsTo(BucketId(16, 0), 3); } -TEST_F(OperationTargetResolverTest, trusted_over_ideal_state) { +TEST_F(OperationTargetResolverTest, legacy_prefers_trusted_over_ideal_state) { setup_stripe(2, 4, "storage:4 distributor:1"); addNodesToBucketDB(BucketId(16, 0), "0=0/0/0/t,1=0,2=0/0/0/t,3=0"); // ideal nodes: 1, 3 + MY_ASSERT_THAT(BucketId(32, 0)).with_symmetric_replica_selection(false) + .sendsTo(BucketId(16, 0), 0) + .sendsTo(BucketId(16, 0), 2); +} + +TEST_F(OperationTargetResolverTest, prefer_ready_over_ideal_state_order) { + setup_stripe(2, 4, "storage:4 distributor:1"); + addNodesToBucketDB(BucketId(16, 0), "0=1/2/3/u/i/r,1=1/2/3,2=1/2/3/u/i/r,3=1/2/3"); + // ideal nodes: 1, 3. 0 and 2 are ready. MY_ASSERT_THAT(BucketId(32, 0)).sendsTo(BucketId(16, 0), 0) .sendsTo(BucketId(16, 0), 2); } +TEST_F(OperationTargetResolverTest, prefer_ready_over_ideal_state_order_also_when_retired) { + setup_stripe(2, 4, "storage:4 .0.s:r distributor:1"); + addNodesToBucketDB(BucketId(16, 0), "0=1/2/3/u/i/r,1=1/2/3,2=1/2/3/u/i/r,3=1/2/3"); + // ideal nodes: 1, 3. 0 and 2 are ready. + MY_ASSERT_THAT(BucketId(32, 0)).sendsTo(BucketId(16, 0), 0) + .sendsTo(BucketId(16, 0), 2); +} + +TEST_F(OperationTargetResolverTest, prefer_replicas_with_more_docs_over_replicas_with_fewer_docs) { + setup_stripe(2, 4, "storage:4 distributor:1"); + addNodesToBucketDB(BucketId(16, 0), "0=2/3/4,1=1/2/3,2=3/4/5,3=1/2/3"); + // ideal nodes: 1, 3. 0 and 2 have more docs. + MY_ASSERT_THAT(BucketId(32, 0)).sendsTo(BucketId(16, 0), 2) + .sendsTo(BucketId(16, 0), 0); +} + +TEST_F(OperationTargetResolverTest, fall_back_to_active_state_and_db_index_if_all_other_fields_equal) { + // All replica nodes tagged as retired, which means none are part of the ideal state order + setup_stripe(2, 4, "storage:4 .0.s:r .2.s:r .3.s:r distributor:1"); + addNodesToBucketDB(BucketId(16, 0), "0=2/3/4/u/a,3=2/3/4,2=2/3/4"); + // ideal nodes: 1, 3. 0 is active and 3 is the remaining replica with the lowest DB order. + MY_ASSERT_THAT(BucketId(32, 0)).sendsTo(BucketId(16, 0), 0) + .sendsTo(BucketId(16, 0), 3); +} + TEST_F(OperationTargetResolverTest, choose_highest_split_bucket) { setup_stripe(2, 2, "storage:2 distributor:1"); // 0, 1 are both in ideal state for both buckets. diff --git a/storage/src/tests/distributor/statusreporterdelegatetest.cpp b/storage/src/tests/distributor/statusreporterdelegatetest.cpp index cc23fa7a22e..c70ab533af6 100644 --- a/storage/src/tests/distributor/statusreporterdelegatetest.cpp +++ b/storage/src/tests/distributor/statusreporterdelegatetest.cpp @@ -1,5 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/storage/distributor/statusreporterdelegate.h> @@ -45,8 +46,8 @@ public: } TEST(StatusReporterDelegateTest, delegate_invokes_delegator_on_status_request) { - vdstestlib::DirConfig config(getStandardConfig(false)); - TestDistributorApp app(config.getConfigId()); + auto config = StorageConfigSet::make_distributor_node_config(); + TestDistributorApp app(config->config_uri()); MockDelegator mockDelegator; MockStatusReporter reporter; diff --git a/storage/src/tests/distributor/top_level_distributor_test_util.cpp b/storage/src/tests/distributor/top_level_distributor_test_util.cpp index 94031c6d71e..e1b2bc93f62 100644 --- a/storage/src/tests/distributor/top_level_distributor_test_util.cpp +++ b/storage/src/tests/distributor/top_level_distributor_test_util.cpp @@ -24,7 +24,7 @@ TopLevelDistributorTestUtil::TopLevelDistributorTestUtil() : _message_sender(_sender, _sender_down), _num_distributor_stripes(4) { - _config = getStandardConfig(false); + _config = StorageConfigSet::make_distributor_node_config(); } TopLevelDistributorTestUtil::~TopLevelDistributorTestUtil() = default; @@ -32,7 +32,7 @@ TopLevelDistributorTestUtil::~TopLevelDistributorTestUtil() = default; void TopLevelDistributorTestUtil::create_links() { - _node = std::make_unique<TestDistributorApp>(_config.getConfigId()); + _node = std::make_unique<TestDistributorApp>(_config->config_uri()); _thread_pool = framework::TickingThreadPool::createDefault("distributor", 100ms); _stripe_pool = DistributorStripePool::make_non_threaded_pool_for_testing(); _distributor.reset(new TopLevelDistributor( @@ -123,7 +123,7 @@ TopLevelDistributorTestUtil::close() } _sender.clear(); _node.reset(); - _config = getStandardConfig(false); + _config = StorageConfigSet::make_distributor_node_config(); } void diff --git a/storage/src/tests/distributor/top_level_distributor_test_util.h b/storage/src/tests/distributor/top_level_distributor_test_util.h index 1d4c81a5bfb..51f0739e3e6 100644 --- a/storage/src/tests/distributor/top_level_distributor_test_util.h +++ b/storage/src/tests/distributor/top_level_distributor_test_util.h @@ -3,6 +3,7 @@ #include "distributor_message_sender_stub.h" #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/storage/common/hostreporter/hostinfo.h> @@ -140,7 +141,7 @@ public: static std::vector<document::BucketSpace> bucket_spaces(); protected: - vdstestlib::DirConfig _config; + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestDistributorApp> _node; std::unique_ptr<framework::TickingThreadPool> _thread_pool; std::unique_ptr<DistributorStripePool> _stripe_pool; diff --git a/storage/src/tests/distributor/updateoperationtest.cpp b/storage/src/tests/distributor/updateoperationtest.cpp index 31ebbe19cbb..e00ce249298 100644 --- a/storage/src/tests/distributor/updateoperationtest.cpp +++ b/storage/src/tests/distributor/updateoperationtest.cpp @@ -49,13 +49,13 @@ struct UpdateOperationTest : Test, DistributorStripeTestUtil { const api::ReturnCode& result = api::ReturnCode()); std::shared_ptr<UpdateOperation> - sendUpdate(const std::string& bucketState, bool create_if_missing = false); + sendUpdate(const std::string& bucketState, bool create_if_missing = false, bool cache_create_flag = false); document::BucketId _bId; }; std::shared_ptr<UpdateOperation> -UpdateOperationTest::sendUpdate(const std::string& bucketState, bool create_if_missing) +UpdateOperationTest::sendUpdate(const std::string& bucketState, bool create_if_missing, bool cache_create_flag) { auto update = std::make_shared<document::DocumentUpdate>( *_repo, *_html_type, @@ -67,6 +67,9 @@ UpdateOperationTest::sendUpdate(const std::string& bucketState, bool create_if_m addNodesToBucketDB(_bId, bucketState); auto msg = std::make_shared<api::UpdateCommand>(makeDocumentBucket(document::BucketId(0)), update, 100); + if (cache_create_flag) { + msg->set_cached_create_if_missing(create_if_missing); + } return std::make_shared<UpdateOperation>( node_context(), operation_context(), getDistributorBucketSpace(), msg, std::vector<BucketDatabase::Entry>(), @@ -271,4 +274,20 @@ TEST_F(UpdateOperationTest, cancelled_nodes_are_not_updated_in_db) { dumpBucket(_bId)); } +TEST_F(UpdateOperationTest, cached_create_if_missing_is_propagated_to_fanout_requests) { + setup_stripe(1, 1, "distributor:1 storage:1"); + for (bool cache_flag : {false, true}) { + for (bool create_if_missing : {false, true}) { + std::shared_ptr<UpdateOperation> cb(sendUpdate("0=1/2/3", create_if_missing, cache_flag)); + DistributorMessageSenderStub sender; + cb->start(sender); + + ASSERT_EQ("Update => 0", sender.getCommands(true)); + auto& cmd = dynamic_cast<api::UpdateCommand&>(*sender.command(0)); + EXPECT_EQ(cmd.has_cached_create_if_missing(), cache_flag); + EXPECT_EQ(cmd.create_if_missing(), create_if_missing); + } + } +} + } diff --git a/storage/src/tests/frameworkimpl/status/statustest.cpp b/storage/src/tests/frameworkimpl/status/statustest.cpp index bd28297e108..8592a332f0c 100644 --- a/storage/src/tests/frameworkimpl/status/statustest.cpp +++ b/storage/src/tests/frameworkimpl/status/statustest.cpp @@ -1,10 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/storage_config_set.h> +#include <tests/common/teststorageapp.h> #include <vespa/storageframework/defaultimplementation/component/componentregisterimpl.h> #include <vespa/storage/frameworkimpl/status/statuswebserver.h> #include <vespa/storageframework/generic/status/htmlstatusreporter.h> #include <vespa/storageframework/generic/status/xmlstatusreporter.h> -#include <tests/common/teststorageapp.h> #include <vespa/document/util/stringutil.h> #include <vespa/vespalib/net/crypto_engine.h> #include <vespa/vespalib/net/socket_spec.h> @@ -39,6 +40,7 @@ vespalib::string fetch(int port, const vespalib::string &path) { namespace storage { struct StatusTest : Test { + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; void SetUp() override; @@ -97,7 +99,8 @@ namespace { } void StatusTest::SetUp() { - _node = std::make_unique<TestServiceLayerApp>(); + _config = StorageConfigSet::make_storage_node_config(); + _node = std::make_unique<TestServiceLayerApp>(_config->config_uri()); } namespace { diff --git a/storage/src/tests/persistence/bucketownershipnotifiertest.cpp b/storage/src/tests/persistence/bucketownershipnotifiertest.cpp index 129cac34c68..717a79b030d 100644 --- a/storage/src/tests/persistence/bucketownershipnotifiertest.cpp +++ b/storage/src/tests/persistence/bucketownershipnotifiertest.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/message_sender_stub.h> +#include <tests/common/storage_config_set.h> #include <tests/common/teststorageapp.h> #include <vespa/document/test/make_document_bucket.h> #include <vespa/storage/persistence/bucketownershipnotifier.h> @@ -14,6 +15,7 @@ using namespace ::testing; namespace storage { struct BucketOwnershipNotifierTest : public Test { + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _app; lib::ClusterState _clusterState; @@ -58,7 +60,8 @@ struct BucketOwnershipNotifierTest : public Test { void BucketOwnershipNotifierTest::SetUp() { - _app = std::make_unique<TestServiceLayerApp>(); + _config = StorageConfigSet::make_storage_node_config(); + _app = std::make_unique<TestServiceLayerApp>(_config->config_uri()); _app->setDistribution(Redundancy(1), NodeCount(2)); _app->setClusterState(_clusterState); } diff --git a/storage/src/tests/persistence/common/filestortestfixture.cpp b/storage/src/tests/persistence/common/filestortestfixture.cpp index 6581fb9f7b1..6c7b09c4736 100644 --- a/storage/src/tests/persistence/common/filestortestfixture.cpp +++ b/storage/src/tests/persistence/common/filestortestfixture.cpp @@ -25,14 +25,11 @@ const uint32_t FileStorTestFixture::MSG_WAIT_TIME; void FileStorTestFixture::setupPersistenceThreads(uint32_t threads) { - std::string rootOfRoot = "todo-make-unique-filestorefixture"; - _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, rootOfRoot)); - _config->getConfig("stor-server").set("root_folder", (rootOfRoot + "-vdsroot.2")); - _config->getConfig("stor-devices").set("root_folder", (rootOfRoot + "-vdsroot.2")); - _config->getConfig("stor-server").set("node_index", "1"); - _config->getConfig("stor-filestor").set("num_threads", std::to_string(threads)); - - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(1), _config->getConfigId()); + _config = StorageConfigSet::make_storage_node_config(); + _config->set_node_index(1); + _config->filestor_config().numThreads = threads; + + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(1), _config->config_uri()); _testdoctype1 = _node->getTypeRepo()->getDocumentType("testdoctype1"); } @@ -77,7 +74,7 @@ FileStorTestFixture::TestFileStorComponents::TestFileStorComponents( { injector.inject(top); using StorFilestorConfig = vespa::config::content::internal::InternalStorFilestorType; - auto config = config_from<StorFilestorConfig>(config::ConfigUri(fixture._config->getConfigId())); + auto config = config_from<StorFilestorConfig>(fixture._config->config_uri()); auto fsm = std::make_unique<FileStorManager>(*config, fixture._node->getPersistenceProvider(), fixture._node->getComponentRegister(), *fixture._node, fixture._node->get_host_info()); manager = fsm.get(); diff --git a/storage/src/tests/persistence/common/filestortestfixture.h b/storage/src/tests/persistence/common/filestortestfixture.h index e4776f393ae..d4fb94101cc 100644 --- a/storage/src/tests/persistence/common/filestortestfixture.h +++ b/storage/src/tests/persistence/common/filestortestfixture.h @@ -2,6 +2,7 @@ #pragma once #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/storage/persistence/filestorage/filestorhandlerimpl.h> @@ -14,8 +15,8 @@ namespace storage { class FileStorTestFixture : public ::testing::Test { public: + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; - std::unique_ptr<vdstestlib::DirConfig> _config; const document::DocumentType* _testdoctype1; static const uint32_t MSG_WAIT_TIME = 60 * 1000; diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index f12b85eb2ea..bdc3f17e576 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <tests/persistence/filestorage/forwardingmessagesender.h> @@ -41,7 +42,6 @@ #include <vespa/log/log.h> LOG_SETUP(".filestormanagertest"); -using std::unique_ptr; using document::Document; using document::BucketId; using namespace storage::api; @@ -91,10 +91,8 @@ make_bucket_for_doc(const document::DocumentId& docid) struct FileStorTestBase : Test { enum {LONG_WAITTIME=60}; - unique_ptr<TestServiceLayerApp> _node; - std::unique_ptr<vdstestlib::DirConfig> config; - std::unique_ptr<vdstestlib::DirConfig> config2; - std::unique_ptr<vdstestlib::DirConfig> smallConfig; + std::unique_ptr<StorageConfigSet> _config; + std::unique_ptr<TestServiceLayerApp> _node; const int32_t _waitTime; const document::DocumentType* _testdoctype1; @@ -165,29 +163,10 @@ struct FileStorTestBase : Test { void setupDisks() { std::string rootOfRoot = "filestormanagertest"; - config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, rootOfRoot)); - - config2 = std::make_unique<vdstestlib::DirConfig>(*config); - config2->getConfig("stor-server").set("root_folder", rootOfRoot + "-vdsroot.2"); - config2->getConfig("stor-devices").set("root_folder", rootOfRoot + "-vdsroot.2"); - config2->getConfig("stor-server").set("node_index", "1"); - - smallConfig = std::make_unique<vdstestlib::DirConfig>(*config); - vdstestlib::DirConfig::Config& c(smallConfig->getConfig("stor-filestor", true)); - c.set("initial_index_read", "128"); - c.set("use_direct_io", "false"); - c.set("maximum_gap_to_read_through", "64"); - - assert(system(vespalib::make_string("rm -rf %s", getRootFolder(*config).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("rm -rf %s", getRootFolder(*config2).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("mkdir -p %s/disks/d0", getRootFolder(*config).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("mkdir -p %s/disks/d0", getRootFolder(*config2).c_str()).c_str()) == 0); - try { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), config->getConfigId()); - _node->setupDummyPersistence(); - } catch (config::InvalidConfigException& e) { - fprintf(stderr, "%s\n", e.what()); - } + _config = StorageConfigSet::make_storage_node_config(); + + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); + _node->setupDummyPersistence(); _testdoctype1 = _node->getTypeRepo()->getDocumentType("testdoctype1"); } @@ -227,11 +206,10 @@ struct TestFileStorComponents { DummyStorageLink top; FileStorManager* manager; - explicit TestFileStorComponents(FileStorTestBase& test, bool use_small_config = false) + explicit TestFileStorComponents(FileStorTestBase& test) : manager(nullptr) { - auto config_uri = config::ConfigUri((use_small_config ? test.smallConfig : test.config)->getConfigId()); - auto config = config_from<StorFilestorConfig>(config_uri); + auto config = config_from<StorFilestorConfig>(test._config->config_uri()); auto fsm = std::make_unique<FileStorManager>(*config, test._node->getPersistenceProvider(), test._node->getComponentRegister(), *test._node, test._node->get_host_info()); manager = fsm.get(); @@ -1255,7 +1233,7 @@ createIterator(DummyStorageLink& link, } TEST_F(FileStorManagerTest, visiting) { - TestFileStorComponents c(*this, true); + TestFileStorComponents c(*this); auto& top = c.top; // Adding documents to two buckets which we are going to visit // We want one bucket in one slotfile, and one bucket with a file split @@ -1409,8 +1387,7 @@ TEST_F(FileStorManagerTest, remove_location) { TEST_F(FileStorManagerTest, delete_bucket) { TestFileStorComponents c(*this); - auto config_uri = config::ConfigUri(config->getConfigId()); - StorFilestorConfigBuilder my_config(*config_from<StorFilestorConfig>(config_uri)); + auto my_config = *config_from<StorFilestorConfig>(_config->config_uri()); c.manager->on_configure(my_config); auto& top = c.top; diff --git a/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp b/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp index 710da80972f..38acc4a18b8 100644 --- a/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp @@ -38,7 +38,7 @@ struct BucketCheckerInjector : FileStorTestFixture::StorageLinkInjector {} void inject(DummyStorageLink& link) const override { using vespa::config::content::core::StorServerConfig; - auto cfg = config_from<StorServerConfig>(config::ConfigUri(_fixture._config->getConfigId())); + auto cfg = config_from<StorServerConfig>(_fixture._config->config_uri()); link.push_back(std::make_unique<ModifiedBucketChecker>( _node.getComponentRegister(), _node.getPersistenceProvider(), *cfg)); } diff --git a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp index f96ff9c012e..9fc6ddff268 100644 --- a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp +++ b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/common/exceptions.h> @@ -34,20 +35,20 @@ struct ModifiedBucketCheckerTest : Test { ModifiedBucketChecker* _handler; DummyStorageLink* _bottom; + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; - std::unique_ptr<vdstestlib::DirConfig> _config; }; void ModifiedBucketCheckerTest::SetUp() { - _config.reset(new vdstestlib::DirConfig(getStandardConfig(true))); - _node.reset(new TestServiceLayerApp(NodeIndex(0), _config->getConfigId())); + _config = StorageConfigSet::make_storage_node_config(); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); _node->setupDummyPersistence(); - _top.reset(new DummyStorageLink); + _top = std::make_unique<DummyStorageLink>(); using vespa::config::content::core::StorServerConfig; - auto bootstrap_cfg = config_from<StorServerConfig>(config::ConfigUri(_config->getConfigId())); + auto bootstrap_cfg = config_from<StorServerConfig>(_config->config_uri()); _handler = new ModifiedBucketChecker(_node->getComponentRegister(), _node->getPersistenceProvider(), *bootstrap_cfg); diff --git a/storage/src/tests/persistence/mergehandlertest.cpp b/storage/src/tests/persistence/mergehandlertest.cpp index 4bd0570efa8..524f5bae392 100644 --- a/storage/src/tests/persistence/mergehandlertest.cpp +++ b/storage/src/tests/persistence/mergehandlertest.cpp @@ -220,11 +220,11 @@ void MergeHandlerTest::setUpChain(ChainPos pos) { _nodes.clear(); if (pos != FRONT) { - _nodes.push_back(api::MergeBucketCommand::Node(2, false)); + _nodes.emplace_back(2, false); } - _nodes.push_back(api::MergeBucketCommand::Node(0, false)); + _nodes.emplace_back(0, false); if (pos != BACK) { - _nodes.push_back(api::MergeBucketCommand::Node(1, false)); + _nodes.emplace_back(1, false); } } @@ -1439,4 +1439,52 @@ TEST_F(MergeHandlerTest, partially_filled_apply_bucket_diff_reply) LOG(debug, "got mergebucket reply"); } +TEST_F(MergeHandlerTest, multiple_versions_in_apply_diff_only_writes_newest_version) { + setUpChain(BACK); + + document::TestDocMan doc_mgr; + document::Document::SP doc(doc_mgr.createRandomDocumentAtLocation(_location, 1)); + spi::Timestamp ts_old(10'000); + spi::Timestamp ts_new(20'000); + + PersistenceProviderWrapper provider_wrapper(getPersistenceProvider()); + MergeHandler handler = createHandler(provider_wrapper); + std::vector<api::ApplyBucketDiffCommand::Entry> apply_diff; + // Diff contains two entries for the same document; one old Remove and one newer Put that + // subsumes the Remove operation. We should only schedule the Put to the SPI. + { + api::ApplyBucketDiffCommand::Entry e; + e._entry._timestamp = ts_old; + e._entry._hasMask = 0x1; + e._docName = doc->getId().toString(); + e._entry._flags = MergeHandler::IN_USE | MergeHandler::DELETED; + apply_diff.push_back(e); + } + { + api::ApplyBucketDiffCommand::Entry e; + e._entry._timestamp = ts_new; + e._entry._hasMask = 0x1; + e._entry._flags = MergeHandler::IN_USE; + fill_entry(e, *doc, doc_mgr.getTypeRepo()); + apply_diff.push_back(e); + } + + auto apply_bucket_diff_cmd = std::make_shared<api::ApplyBucketDiffCommand>(_bucket, _nodes); + apply_bucket_diff_cmd->getDiff() = std::move(apply_diff); + + provider_wrapper.clearOperationLog(); + auto tracker = handler.handleApplyBucketDiff(*apply_bucket_diff_cmd, createTracker(apply_bucket_diff_cmd, _bucket)); + ASSERT_FALSE(tracker); + handler.drain_async_writes(); + + // There should be no remove at time=ts_old, only a put at time=ts_new. + // TODO ideally we shouldn't have to know about the other operations... + EXPECT_EQ(provider_wrapper.toString(), + "createIterator(Bucket(0x40000000000004d2), ALL_VERSIONS)\n" + "iterate(1, 18446744073709551615)\n" + "destroyIterator(1)\n" + "put(Bucket(0x40000000000004d2), 20000, id:mail:testdoctype1:n=1234:9380.html)\n" + "getBucketInfo(Bucket(0x40000000000004d2))\n"); +} + } // storage diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index a599b1d380a..b63df53781d 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -25,14 +25,6 @@ namespace storage { namespace { -vdstestlib::DirConfig initialize(const std::string & rootOfRoot) { - vdstestlib::DirConfig config(getStandardConfig(true, rootOfRoot)); - std::string rootFolder = getRootFolder(config); - std::filesystem::remove_all(std::filesystem::path(rootFolder)); - std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str()))); - return config; -} - template<typename T> struct ConfigReader : public T::Subscriber { @@ -49,10 +41,10 @@ constexpr uint32_t MERGE_CHUNK_SIZE = 4_Mi; } -PersistenceTestEnvironment::PersistenceTestEnvironment(const std::string & rootOfRoot) - : _config(initialize(rootOfRoot)), +PersistenceTestEnvironment::PersistenceTestEnvironment() + : _config(StorageConfigSet::make_distributor_node_config()), _messageKeeper(), - _node(NodeIndex(0), _config.getConfigId()), + _node(NodeIndex(0), _config->config_uri()), _component(_node.getComponentRegister(), "persistence test env"), _metrics() { @@ -106,7 +98,7 @@ PersistenceTestUtils::MockBucketLocks::unlock(document::Bucket bucket) } PersistenceTestUtils::PersistenceTestUtils() - : _env(std::make_unique<PersistenceTestEnvironment>("todo-make-unique-persistencetestutils")), + : _env(std::make_unique<PersistenceTestEnvironment>()), _replySender(), _bucketOwnershipNotifier(getEnv()._component, getEnv()._fileStorHandler), _mock_bucket_locks(), diff --git a/storage/src/tests/persistence/persistencetestutils.h b/storage/src/tests/persistence/persistencetestutils.h index d03974855ad..0125bd7aa79 100644 --- a/storage/src/tests/persistence/persistencetestutils.h +++ b/storage/src/tests/persistence/persistencetestutils.h @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include <tests/common/storage_config_set.h> #include <tests/common/teststorageapp.h> #include <tests/common/testhelper.h> #include <vespa/storage/persistence/persistencethread.h> @@ -25,11 +26,11 @@ struct MessageKeeper : public MessageSender { }; struct PersistenceTestEnvironment { - PersistenceTestEnvironment(const std::string & rootOfRoot); + PersistenceTestEnvironment(); ~PersistenceTestEnvironment(); document::TestDocMan _testDocMan; - vdstestlib::DirConfig _config; + std::unique_ptr<StorageConfigSet> _config; MessageKeeper _messageKeeper; TestServiceLayerApp _node; ServiceLayerComponent _component; diff --git a/storage/src/tests/persistence/provider_error_wrapper_test.cpp b/storage/src/tests/persistence/provider_error_wrapper_test.cpp index fc88428f915..d5ce8400b25 100644 --- a/storage/src/tests/persistence/provider_error_wrapper_test.cpp +++ b/storage/src/tests/persistence/provider_error_wrapper_test.cpp @@ -1,5 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/storage_config_set.h> #include <vespa/persistence/spi/test.h> #include <tests/persistence/persistencetestutils.h> #include <tests/persistence/common/persistenceproviderwrapper.h> @@ -33,13 +34,15 @@ struct MockErrorListener : ProviderErrorListener { struct Fixture { // We wrap the wrapper. It's turtles all the way down! PersistenceProviderWrapper providerWrapper; + std::unique_ptr<StorageConfigSet> config; TestServiceLayerApp app; ServiceLayerComponent component; ProviderErrorWrapper errorWrapper; Fixture(spi::PersistenceProvider& provider) : providerWrapper(provider), - app(), + config(StorageConfigSet::make_storage_node_config()), + app(config->config_uri()), component(app.getComponentRegister(), "dummy"), errorWrapper(providerWrapper) { diff --git a/storage/src/tests/storageapi/mbusprot/storageprotocoltest.cpp b/storage/src/tests/storageapi/mbusprot/storageprotocoltest.cpp index 698d8dee573..231f41ffd21 100644 --- a/storage/src/tests/storageapi/mbusprot/storageprotocoltest.cpp +++ b/storage/src/tests/storageapi/mbusprot/storageprotocoltest.cpp @@ -262,7 +262,6 @@ TEST_P(StorageProtocolTest, response_metadata_is_propagated) { TEST_P(StorageProtocolTest, update) { auto update = std::make_shared<document::DocumentUpdate>(_docMan.getTypeRepo(), *_testDoc->getDataType(), _testDoc->getId()); update->addUpdate(FieldUpdate(_testDoc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(17)))); - update->addFieldPathUpdate(std::make_unique<RemoveFieldPathUpdate>("headerval", "testdoctype1.headerval > 0")); auto cmd = std::make_shared<UpdateCommand>(_bucket, update, 14); @@ -284,6 +283,37 @@ TEST_P(StorageProtocolTest, update) { EXPECT_NO_FATAL_FAILURE(assert_bucket_info_reply_fields_propagated(*reply2)); } +TEST_P(StorageProtocolTest, update_request_create_if_missing_flag_is_propagated) { + auto make_update_cmd = [&](bool create_if_missing, bool cached) { + auto update = std::make_shared<document::DocumentUpdate>( + _docMan.getTypeRepo(), *_testDoc->getDataType(), _testDoc->getId()); + update->addUpdate(FieldUpdate(_testDoc->getField("headerval")).addUpdate( + std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(17)))); + update->addFieldPathUpdate(std::make_unique<RemoveFieldPathUpdate>("headerval", "testdoctype1.headerval > 0")); + update->setCreateIfNonExistent(create_if_missing); + auto cmd = std::make_shared<UpdateCommand>(_bucket, update, 14); + if (cached) { + cmd->set_cached_create_if_missing(create_if_missing); + } + return cmd; + }; + + auto check_flag_propagation = [&](bool create_if_missing, bool cached) { + auto cmd = make_update_cmd(create_if_missing, cached); + EXPECT_EQ(cmd->has_cached_create_if_missing(), cached); + EXPECT_EQ(cmd->create_if_missing(), create_if_missing); + + auto cmd2 = copyCommand(cmd); + EXPECT_EQ(cmd2->has_cached_create_if_missing(), cached); + EXPECT_EQ(cmd2->create_if_missing(), create_if_missing); + }; + + check_flag_propagation(false, false); + check_flag_propagation(true, false); + check_flag_propagation(false, true); + check_flag_propagation(true, true); +} + TEST_P(StorageProtocolTest, get) { auto cmd = std::make_shared<GetCommand>(_bucket, _testDocId, "foo,bar,vekterli", 123); auto cmd2 = copyCommand(cmd); @@ -880,7 +910,7 @@ TEST_P(StorageProtocolTest, track_memory_footprint_for_some_messages) { EXPECT_EQ(sizeof(BucketInfoCommand), sizeof(BucketCommand)); EXPECT_EQ(sizeof(TestAndSetCommand), sizeof(BucketInfoCommand) + sizeof(vespalib::string)); EXPECT_EQ(sizeof(PutCommand), sizeof(TestAndSetCommand) + 40); - EXPECT_EQ(sizeof(UpdateCommand), sizeof(TestAndSetCommand) + 32); + EXPECT_EQ(sizeof(UpdateCommand), sizeof(TestAndSetCommand) + 40); EXPECT_EQ(sizeof(RemoveCommand), sizeof(TestAndSetCommand) + 112); EXPECT_EQ(sizeof(GetCommand), sizeof(BucketInfoCommand) + sizeof(TestAndSetCondition) + 184); } diff --git a/storage/src/tests/storageserver/bouncertest.cpp b/storage/src/tests/storageserver/bouncertest.cpp index 296ed6d23bc..11742dd658f 100644 --- a/storage/src/tests/storageserver/bouncertest.cpp +++ b/storage/src/tests/storageserver/bouncertest.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/common/exceptions.h> @@ -26,6 +27,7 @@ using namespace ::testing; namespace storage { struct BouncerTest : public Test { + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestStorageApp> _node; std::unique_ptr<DummyStorageLink> _upper; Bouncer* _manager; @@ -57,15 +59,15 @@ BouncerTest::BouncerTest() } void BouncerTest::setUpAsNode(const lib::NodeType& type) { - vdstestlib::DirConfig config(getStandardConfig(type == lib::NodeType::STORAGE)); + _config = StorageConfigSet::make_node_config(type == lib::NodeType::STORAGE); if (type == lib::NodeType::STORAGE) { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(2), config.getConfigId()); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(2), _config->config_uri()); } else { - _node = std::make_unique<TestDistributorApp>(NodeIndex(2), config.getConfigId()); + _node = std::make_unique<TestDistributorApp>(NodeIndex(2), _config->config_uri()); } _upper = std::make_unique<DummyStorageLink>(); using StorBouncerConfig = vespa::config::content::core::StorBouncerConfig; - auto cfg_uri = config::ConfigUri(config.getConfigId()); + auto& cfg_uri = _config->config_uri(); auto cfg = config::ConfigGetter<StorBouncerConfig>::getConfig(cfg_uri.getConfigId(), cfg_uri.getContext()); _manager = new Bouncer(_node->getComponentRegister(), *cfg); _lower = new DummyStorageLink(); diff --git a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp index 50977b5ec8b..8982b02f2b7 100644 --- a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp +++ b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp @@ -3,6 +3,7 @@ #include <tests/common/teststorageapp.h> #include <tests/common/testhelper.h> #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/base/testdocman.h> #include <vespa/storage/bucketdb/storbucketdb.h> @@ -28,11 +29,12 @@ using namespace ::testing; namespace storage { struct ChangedBucketOwnershipHandlerTest : Test { + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _app; - std::unique_ptr<DummyStorageLink> _top; - ChangedBucketOwnershipHandler* _handler; - DummyStorageLink* _bottom; - document::TestDocMan _testDocRepo; + std::unique_ptr<DummyStorageLink> _top; + ChangedBucketOwnershipHandler* _handler; + DummyStorageLink* _bottom; + document::TestDocMan _testDocRepo; // TODO test: down edge triggered on cluster state with cluster down? @@ -126,11 +128,12 @@ void ChangedBucketOwnershipHandlerTest::SetUp() { using vespa::config::content::PersistenceConfig; - vdstestlib::DirConfig config(getStandardConfig(true)); - _app.reset(new TestServiceLayerApp); - _top.reset(new DummyStorageLink); - _handler = new ChangedBucketOwnershipHandler(*config_from<PersistenceConfig>(config::ConfigUri(config.getConfigId())), + _config = StorageConfigSet::make_storage_node_config(); + _app = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); + _top = std::make_unique<DummyStorageLink>(); + + _handler = new ChangedBucketOwnershipHandler(*config_from<PersistenceConfig>(_config->config_uri()), _app->getComponentRegister()); _top->push_back(std::unique_ptr<StorageLink>(_handler)); _bottom = new DummyStorageLink; diff --git a/storage/src/tests/storageserver/communicationmanagertest.cpp b/storage/src/tests/storageserver/communicationmanagertest.cpp index 04322562d08..b741d79582f 100644 --- a/storage/src/tests/storageserver/communicationmanagertest.cpp +++ b/storage/src/tests/storageserver/communicationmanagertest.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/helper/configgetter.hpp> @@ -65,20 +66,20 @@ wait_for_slobrok_visibility(const CommunicationManager& mgr, TEST_F(CommunicationManagerTest, simple) { mbus::Slobrok slobrok; - vdstestlib::DirConfig distConfig(getStandardConfig(false)); - vdstestlib::DirConfig storConfig(getStandardConfig(true)); - distConfig.getConfig("stor-server").set("node_index", "1"); - storConfig.getConfig("stor-server").set("node_index", "1"); - addSlobrokConfig(distConfig, slobrok); - addSlobrokConfig(storConfig, slobrok); + auto dist_config = StorageConfigSet::make_distributor_node_config(); + auto stor_config = StorageConfigSet::make_storage_node_config(); + dist_config->set_node_index(1); + stor_config->set_node_index(1); + dist_config->set_slobrok_config_port(slobrok.port()); + stor_config->set_slobrok_config_port(slobrok.port()); + + auto& dist_cfg_uri = dist_config->config_uri(); + auto& stor_cfg_uri = stor_config->config_uri(); // Set up a "distributor" and a "storage" node with communication // managers and a dummy storage link below we can use for testing. - TestServiceLayerApp storNode(storConfig.getConfigId()); - TestDistributorApp distNode(distConfig.getConfigId()); - - auto dist_cfg_uri = config::ConfigUri(distConfig.getConfigId()); - auto stor_cfg_uri = config::ConfigUri(storConfig.getConfigId()); + TestServiceLayerApp storNode(stor_cfg_uri); + TestDistributorApp distNode(dist_cfg_uri); CommunicationManager distributor(distNode.getComponentRegister(), dist_cfg_uri, *config_from<CommunicationManagerConfig>(dist_cfg_uri)); @@ -123,23 +124,22 @@ void CommunicationManagerTest::doTestConfigPropagation(bool isContentNode) { mbus::Slobrok slobrok; - vdstestlib::DirConfig config(getStandardConfig(isContentNode)); - config.getConfig("stor-server").set("node_index", "1"); - auto& cfg = config.getConfig("stor-communicationmanager"); - cfg.set("mbus_content_node_max_pending_count", "12345"); - cfg.set("mbus_content_node_max_pending_size", "555666"); - cfg.set("mbus_distributor_node_max_pending_count", "6789"); - cfg.set("mbus_distributor_node_max_pending_size", "777888"); - addSlobrokConfig(config, slobrok); + auto config = StorageConfigSet::make_node_config(isContentNode); + config->set_node_index(1); + config->set_slobrok_config_port(slobrok.port()); + config->communication_manager_config().mbusContentNodeMaxPendingCount = 12345; + config->communication_manager_config().mbusContentNodeMaxPendingSize = 555666; + config->communication_manager_config().mbusDistributorNodeMaxPendingCount = 6789; + config->communication_manager_config().mbusDistributorNodeMaxPendingSize = 777888; + auto& cfg_uri = config->config_uri(); std::unique_ptr<TestStorageApp> node; if (isContentNode) { - node = std::make_unique<TestServiceLayerApp>(config.getConfigId()); + node = std::make_unique<TestServiceLayerApp>(cfg_uri); } else { - node = std::make_unique<TestDistributorApp>(config.getConfigId()); + node = std::make_unique<TestDistributorApp>(cfg_uri); } - auto cfg_uri = config::ConfigUri(config.getConfigId()); CommunicationManager commMgr(node->getComponentRegister(), cfg_uri, *config_from<CommunicationManagerConfig>(cfg_uri)); auto* storageLink = new DummyStorageLink(); @@ -180,12 +180,12 @@ TEST_F(CommunicationManagerTest, stor_pending_limit_configs_are_propagated_to_me TEST_F(CommunicationManagerTest, commands_are_dequeued_in_fifo_order) { mbus::Slobrok slobrok; - vdstestlib::DirConfig storConfig(getStandardConfig(true)); - storConfig.getConfig("stor-server").set("node_index", "1"); - addSlobrokConfig(storConfig, slobrok); - TestServiceLayerApp storNode(storConfig.getConfigId()); + auto config = StorageConfigSet::make_storage_node_config(); + config->set_node_index(1); + config->set_slobrok_config_port(slobrok.port()); + auto& cfg_uri = config->config_uri(); + TestServiceLayerApp storNode(cfg_uri); - auto cfg_uri = config::ConfigUri(storConfig.getConfigId()); CommunicationManager storage(storNode.getComponentRegister(), cfg_uri, *config_from<CommunicationManagerConfig>(cfg_uri)); auto* storageLink = new DummyStorageLink(); @@ -214,12 +214,12 @@ TEST_F(CommunicationManagerTest, commands_are_dequeued_in_fifo_order) { TEST_F(CommunicationManagerTest, replies_are_dequeued_in_fifo_order) { mbus::Slobrok slobrok; - vdstestlib::DirConfig storConfig(getStandardConfig(true)); - storConfig.getConfig("stor-server").set("node_index", "1"); - addSlobrokConfig(storConfig, slobrok); - TestServiceLayerApp storNode(storConfig.getConfigId()); + auto config = StorageConfigSet::make_storage_node_config(); + config->set_node_index(1); + config->set_slobrok_config_port(slobrok.port()); + auto& cfg_uri = config->config_uri(); + TestServiceLayerApp storNode(cfg_uri); - auto cfg_uri = config::ConfigUri(storConfig.getConfigId()); CommunicationManager storage(storNode.getComponentRegister(), cfg_uri, *config_from<CommunicationManagerConfig>(cfg_uri)); auto* storageLink = new DummyStorageLink(); @@ -249,19 +249,21 @@ struct MockMbusReplyHandler : mbus::IReplyHandler { }; struct CommunicationManagerFixture { + std::unique_ptr<StorageConfigSet> config; MockMbusReplyHandler reply_handler; mbus::Slobrok slobrok; std::unique_ptr<TestServiceLayerApp> node; std::unique_ptr<CommunicationManager> comm_mgr; DummyStorageLink* bottom_link; - CommunicationManagerFixture() { - vdstestlib::DirConfig stor_config(getStandardConfig(true)); - stor_config.getConfig("stor-server").set("node_index", "1"); - addSlobrokConfig(stor_config, slobrok); + CommunicationManagerFixture() + : config(StorageConfigSet::make_storage_node_config()) + { + config->set_node_index(1); + config->set_slobrok_config_port(slobrok.port()); + auto& cfg_uri = config->config_uri(); - node = std::make_unique<TestServiceLayerApp>(stor_config.getConfigId()); - auto cfg_uri = config::ConfigUri(stor_config.getConfigId()); + node = std::make_unique<TestServiceLayerApp>(cfg_uri); comm_mgr = std::make_unique<CommunicationManager>(node->getComponentRegister(), cfg_uri, *config_from<CommunicationManagerConfig>(cfg_uri)); bottom_link = new DummyStorageLink(); diff --git a/storage/src/tests/storageserver/documentapiconvertertest.cpp b/storage/src/tests/storageserver/documentapiconvertertest.cpp index eb4789b25d4..1eb6bf5dd9a 100644 --- a/storage/src/tests/storageserver/documentapiconvertertest.cpp +++ b/storage/src/tests/storageserver/documentapiconvertertest.cpp @@ -159,28 +159,46 @@ TEST_F(DocumentApiConverterTest, forwarded_put) { } TEST_F(DocumentApiConverterTest, update) { - auto update = std::make_shared<document::DocumentUpdate>(*_repo, _html_type, defaultDocId); - documentapi::UpdateDocumentMessage updateMsg(update); - updateMsg.setOldTimestamp(1234); - updateMsg.setNewTimestamp(5678); - updateMsg.setCondition(my_condition); - - auto updateCmd = toStorageAPI<api::UpdateCommand>(updateMsg); - EXPECT_EQ(defaultBucket, updateCmd->getBucket()); - ASSERT_EQ(update.get(), updateCmd->getUpdate().get()); - EXPECT_EQ(api::Timestamp(1234), updateCmd->getOldTimestamp()); - EXPECT_EQ(api::Timestamp(5678), updateCmd->getTimestamp()); - EXPECT_EQ(my_condition, updateCmd->getCondition()); - - auto mbusReply = updateMsg.createReply(); - ASSERT_TRUE(mbusReply.get()); - toStorageAPI<api::UpdateReply>(*mbusReply, *updateCmd); - - auto mbusUpdate = toDocumentAPI<documentapi::UpdateDocumentMessage>(*updateCmd); - ASSERT_EQ((&mbusUpdate->getDocumentUpdate()), update.get()); - EXPECT_EQ(api::Timestamp(1234), mbusUpdate->getOldTimestamp()); - EXPECT_EQ(api::Timestamp(5678), mbusUpdate->getNewTimestamp()); - EXPECT_EQ(my_condition, mbusUpdate->getCondition()); + auto do_test_update = [&](bool create_if_missing) { + auto update = std::make_shared<document::DocumentUpdate>(*_repo, _html_type, defaultDocId); + update->setCreateIfNonExistent(create_if_missing); + documentapi::UpdateDocumentMessage updateMsg(update); + updateMsg.setOldTimestamp(1234); + updateMsg.setNewTimestamp(5678); + updateMsg.setCondition(my_condition); + EXPECT_FALSE(updateMsg.has_cached_create_if_missing()); + EXPECT_EQ(updateMsg.create_if_missing(), create_if_missing); + + auto updateCmd = toStorageAPI<api::UpdateCommand>(updateMsg); + EXPECT_EQ(defaultBucket, updateCmd->getBucket()); + ASSERT_EQ(update.get(), updateCmd->getUpdate().get()); + EXPECT_EQ(api::Timestamp(1234), updateCmd->getOldTimestamp()); + EXPECT_EQ(api::Timestamp(5678), updateCmd->getTimestamp()); + EXPECT_EQ(my_condition, updateCmd->getCondition()); + EXPECT_FALSE(updateCmd->has_cached_create_if_missing()); + EXPECT_EQ(updateCmd->create_if_missing(), create_if_missing); + + auto mbusReply = updateMsg.createReply(); + ASSERT_TRUE(mbusReply.get()); + toStorageAPI<api::UpdateReply>(*mbusReply, *updateCmd); + + auto mbusUpdate = toDocumentAPI<documentapi::UpdateDocumentMessage>(*updateCmd); + ASSERT_EQ((&mbusUpdate->getDocumentUpdate()), update.get()); + EXPECT_EQ(api::Timestamp(1234), mbusUpdate->getOldTimestamp()); + EXPECT_EQ(api::Timestamp(5678), mbusUpdate->getNewTimestamp()); + EXPECT_EQ(my_condition, mbusUpdate->getCondition()); + EXPECT_EQ(mbusUpdate->create_if_missing(), create_if_missing); + + // Cached value of create_if_missing should override underlying update's value + updateCmd->set_cached_create_if_missing(!create_if_missing); + EXPECT_TRUE(updateCmd->has_cached_create_if_missing()); + EXPECT_EQ(updateCmd->create_if_missing(), !create_if_missing); + mbusUpdate = toDocumentAPI<documentapi::UpdateDocumentMessage>(*updateCmd); + EXPECT_TRUE(mbusUpdate->has_cached_create_if_missing()); + EXPECT_EQ(mbusUpdate->create_if_missing(), !create_if_missing); + }; + do_test_update(false); + do_test_update(true); } TEST_F(DocumentApiConverterTest, remove) { diff --git a/storage/src/tests/storageserver/mergethrottlertest.cpp b/storage/src/tests/storageserver/mergethrottlertest.cpp index cdf203b8a39..bc1c7f60706 100644 --- a/storage/src/tests/storageserver/mergethrottlertest.cpp +++ b/storage/src/tests/storageserver/mergethrottlertest.cpp @@ -1,10 +1,12 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/test/make_document_bucket.h> #include <vespa/messagebus/dynamicthrottlepolicy.h> +#include <vespa/storage/config/config-stor-server.h> #include <vespa/storage/persistence/messages.h> #include <vespa/storage/storageserver/mergethrottler.h> #include <vespa/storageapi/message/bucket.h> @@ -36,9 +38,8 @@ using StorServerConfigBuilder = vespa::config::content::core::StorServerConfigBu vespalib::string _storage("storage"); std::unique_ptr<StorServerConfig> default_server_config() { - vdstestlib::DirConfig dir_config(getStandardConfig(true)); - auto cfg_uri = ::config::ConfigUri(dir_config.getConfigId()); - return config_from<StorServerConfig>(cfg_uri); + auto config = StorageConfigSet::make_storage_node_config(); + return config_from<StorServerConfig>(config->config_uri()); } struct MergeBuilder { @@ -153,8 +154,9 @@ struct MergeThrottlerTest : Test { static constexpr int _messageWaitTime = 100; // Using n storage node links and dummy servers - std::vector<std::shared_ptr<DummyStorageLink> > _topLinks; - std::vector<std::shared_ptr<TestServiceLayerApp> > _servers; + std::unique_ptr<StorageConfigSet> _config; + std::vector<std::shared_ptr<DummyStorageLink>> _topLinks; + std::vector<std::shared_ptr<TestServiceLayerApp>> _servers; std::vector<MergeThrottler*> _throttlers; std::vector<DummyStorageLink*> _bottomLinks; @@ -198,14 +200,14 @@ MergeThrottlerTest::~MergeThrottlerTest() = default; void MergeThrottlerTest::SetUp() { - auto config = default_server_config(); + _config = StorageConfigSet::make_storage_node_config(); for (int i = 0; i < _storageNodeCount; ++i) { - auto server = std::make_unique<TestServiceLayerApp>(NodeIndex(i)); + auto server = std::make_unique<TestServiceLayerApp>(NodeIndex(i), _config->config_uri()); server->setClusterState(lib::ClusterState("distributor:100 storage:100 version:1")); std::unique_ptr<DummyStorageLink> top; top = std::make_unique<DummyStorageLink>(); - auto* throttler = new MergeThrottler(*config, server->getComponentRegister(), vespalib::HwInfo()); + auto* throttler = new MergeThrottler(_config->server_config(), server->getComponentRegister(), vespalib::HwInfo()); // MergeThrottler will be sandwiched in between two dummy links top->push_back(std::unique_ptr<StorageLink>(throttler)); auto* bottom = new DummyStorageLink; diff --git a/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp b/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp index 6e9485e24d4..c3641b9bc56 100644 --- a/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp +++ b/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp @@ -1,5 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/storage_config_set.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/fnet/frt/rpcrequest.h> #include <vespa/messagebus/testlib/slobrok.h> @@ -11,7 +12,6 @@ #include <vespa/storageapi/message/state.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/vespalib/stllike/asciistream.h> -#include <tests/common/testhelper.h> #include <vespa/vespalib/gtest/gtest.h> #include <vector> @@ -43,7 +43,7 @@ struct DummyReturnHandler : FRT_IReturnHandler { struct FixtureBase { mbus::Slobrok slobrok; - vdstestlib::DirConfig config; + std::unique_ptr<StorageConfigSet> config; MockOperationDispatcher dispatcher; std::unique_ptr<SharedRpcResources> shared_rpc_resources; std::unique_ptr<ClusterControllerApiRpcService> cc_service; @@ -52,12 +52,12 @@ struct FixtureBase { FRT_RPCRequest* bound_request{nullptr}; FixtureBase() - : config(getStandardConfig(true)) + : config(StorageConfigSet::make_storage_node_config()) { - config.getConfig("stor-server").set("node_index", "1"); - addSlobrokConfig(config, slobrok); + config->set_node_index(1); + config->set_slobrok_config_port(slobrok.port()); - shared_rpc_resources = std::make_unique<SharedRpcResources>(config::ConfigUri(config.getConfigId()), 0, 1, 1); + shared_rpc_resources = std::make_unique<SharedRpcResources>(config->config_uri(), 0, 1, 1); cc_service = std::make_unique<ClusterControllerApiRpcService>(dispatcher, *shared_rpc_resources); shared_rpc_resources->start_server_and_register_slobrok("my_cool_rpc_test"); } diff --git a/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp b/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp index 72ddc89f9d3..010f2b441ef 100644 --- a/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp +++ b/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp @@ -1,6 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <tests/common/testhelper.h> +#include <tests/common/storage_config_set.h> #include <vespa/document/base/testdocman.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> @@ -102,7 +102,7 @@ vespalib::string to_slobrok_id(const api::StorageMessageAddress& address) { class RpcNode { protected: - vdstestlib::DirConfig _config; + std::unique_ptr<StorageConfigSet> _config; std::shared_ptr<const document::DocumentTypeRepo> _doc_type_repo; LockingMockOperationDispatcher _messages; std::unique_ptr<MessageCodecProvider> _codec_provider; @@ -111,17 +111,15 @@ protected: vespalib::string _slobrok_id; public: RpcNode(uint16_t node_index, bool is_distributor, const mbus::Slobrok& slobrok) - : _config(getStandardConfig(true)), + : _config(StorageConfigSet::make_node_config(!is_distributor)), _doc_type_repo(document::TestDocRepo().getTypeRepoSp()), _node_address(make_address(node_index, is_distributor)), _slobrok_id(to_slobrok_id(_node_address)) { - auto& cfg = _config.getConfig("stor-server"); - cfg.set("node_index", std::to_string(node_index)); - cfg.set("is_distributor", is_distributor ? "true" : "false"); - addSlobrokConfig(_config, slobrok); + _config->set_node_index(node_index); + _config->set_slobrok_config_port(slobrok.port()); - _shared_rpc_resources = std::make_unique<SharedRpcResources>(config::ConfigUri(_config.getConfigId()), 0, 1, 1); + _shared_rpc_resources = std::make_unique<SharedRpcResources>(_config->config_uri(), 0, 1, 1); // TODO make codec provider into interface so we can test decode-failures more easily? _codec_provider = std::make_unique<MessageCodecProvider>(_doc_type_repo); } diff --git a/storage/src/tests/storageserver/service_layer_error_listener_test.cpp b/storage/src/tests/storageserver/service_layer_error_listener_test.cpp index 63d8eec6dc3..b84f96dd847 100644 --- a/storage/src/tests/storageserver/service_layer_error_listener_test.cpp +++ b/storage/src/tests/storageserver/service_layer_error_listener_test.cpp @@ -1,12 +1,12 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/storage_config_set.h> #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/storage/storageserver/mergethrottler.h> #include <vespa/storage/storageserver/service_layer_error_listener.h> #include <vespa/storageframework/defaultimplementation/component/componentregisterimpl.h> -#include <vespa/vdstestlib/config/dirconfig.h> #include <vespa/vespalib/gtest/gtest.h> using namespace ::testing; @@ -37,10 +37,10 @@ private: struct Fixture { using StorServerConfig = vespa::config::content::core::StorServerConfig; - vdstestlib::DirConfig config{getStandardConfig(true)}; - TestServiceLayerApp app; + std::unique_ptr<StorageConfigSet> config{StorageConfigSet::make_storage_node_config()}; + TestServiceLayerApp app{config->config_uri()}; ServiceLayerComponent component{app.getComponentRegister(), "dummy"}; - MergeThrottler merge_throttler{*config_from<StorServerConfig>(config::ConfigUri(config.getConfigId())), + MergeThrottler merge_throttler{*config_from<StorServerConfig>(config->config_uri()), app.getComponentRegister(), vespalib::HwInfo()}; TestShutdownListener shutdown_listener; ServiceLayerErrorListener error_listener{component, merge_throttler}; diff --git a/storage/src/tests/storageserver/statemanagertest.cpp b/storage/src/tests/storageserver/statemanagertest.cpp index 2a5af397aca..b785bc141b6 100644 --- a/storage/src/tests/storageserver/statemanagertest.cpp +++ b/storage/src/tests/storageserver/statemanagertest.cpp @@ -1,13 +1,14 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> +#include <tests/common/teststorageapp.h> +#include <tests/common/testhelper.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/message/state.h> #include <vespa/vdslib/state/cluster_state_bundle.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/storage/storageserver/statemanager.h> -#include <tests/common/teststorageapp.h> -#include <tests/common/testhelper.h> -#include <tests/common/dummystoragelink.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/gtest/gtest.h> @@ -20,6 +21,7 @@ using namespace ::testing; namespace storage { struct StateManagerTest : Test, NodeStateReporter { + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<DummyStorageLink> _upper; StateManager* _manager; @@ -46,7 +48,8 @@ struct StateManagerTest : Test, NodeStateReporter { }; StateManagerTest::StateManagerTest() - : _node(), + : _config(), + _node(), _upper(), _manager(nullptr), _lower(nullptr) @@ -56,7 +59,8 @@ StateManagerTest::StateManagerTest() void StateManagerTest::SetUp() { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(2)); + _config = StorageConfigSet::make_storage_node_config(); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(2), _config->config_uri()); // Clock will increase 1 sec per call. _node->getClock().setAbsoluteTimeInSeconds(1); _upper = std::make_unique<DummyStorageLink>(); diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp index 43eb37afe15..29d3daf9b86 100644 --- a/storage/src/tests/storageserver/statereportertest.cpp +++ b/storage/src/tests/storageserver/statereportertest.cpp @@ -1,14 +1,15 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> +#include <tests/common/teststorageapp.h> +#include <tests/common/testhelper.h> #include <vespa/storageframework/defaultimplementation/clock/fakeclock.h> #include <vespa/storage/persistence/filestorage/filestormanager.h> #include <vespa/storage/persistence/filestorage/filestormetrics.h> #include <vespa/storage/storageserver/applicationgenerationfetcher.h> #include <vespa/storage/storageserver/statereporter.h> #include <vespa/metrics/metricmanager.h> -#include <tests/common/teststorageapp.h> -#include <tests/common/testhelper.h> -#include <tests/common/dummystoragelink.h> #include <vespa/config/common/exceptions.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/simple_buffer.h> @@ -35,7 +36,7 @@ struct StateReporterTest : Test { std::unique_ptr<DummyStorageLink> _top; DummyApplicationGenerationFether _generationFetcher; std::unique_ptr<StateReporter> _stateReporter; - std::unique_ptr<vdstestlib::DirConfig> _config; + std::unique_ptr<StorageConfigSet> _config; std::unique_ptr<metrics::MetricSet> _topSet; std::unique_ptr<metrics::MetricManager> _metricManager; std::shared_ptr<FileStorMetrics> _filestorMetrics; @@ -68,10 +69,8 @@ StateReporterTest::StateReporterTest() StateReporterTest::~StateReporterTest() = default; void StateReporterTest::SetUp() { - _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "statereportertest")); - assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0); - - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); + _config = StorageConfigSet::make_storage_node_config(); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); _node->setupDummyPersistence(); _clock = &_node->getClock(); _clock->setAbsoluteTimeInSeconds(1000000); @@ -91,7 +90,7 @@ void StateReporterTest::SetUp() { _filestorMetrics->initDiskMetrics(1, 1); _topSet->registerMetric(*_filestorMetrics); - _metricManager->init(config::ConfigUri(_config->getConfigId())); + _metricManager->init(_config->config_uri()); } void StateReporterTest::TearDown() { diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 09457038b70..29f065c0157 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -1,5 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> +#include <tests/common/teststorageapp.h> +#include <tests/common/testhelper.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> @@ -9,9 +13,6 @@ #include <vespa/storage/persistence/filestorage/filestormanager.h> #include <vespa/storage/visiting/visitormanager.h> #include <vespa/storageframework/defaultimplementation/clock/realclock.h> -#include <tests/common/teststorageapp.h> -#include <tests/common/testhelper.h> -#include <tests/common/dummystoragelink.h> #include <vespa/document/test/make_document_bucket.h> #include <vespa/document/test/make_bucket_space.h> #include <tests/storageserver/testvisitormessagesession.h> @@ -45,7 +46,9 @@ api::StorageMessageAddress _address(&_storage, lib::NodeType::STORAGE, 0); struct VisitorManagerTest : Test { protected: static uint32_t docCount; - std::vector<document::Document::SP > _documents; + + std::unique_ptr<StorageConfigSet> _config; + std::vector<document::Document::SP> _documents; std::unique_ptr<TestVisitorMessageSessionFactory> _messageSessionFactory; std::unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<DummyStorageLink> _top; @@ -82,16 +85,16 @@ uint32_t VisitorManagerTest::docCount = 10; void VisitorManagerTest::initializeTest(bool defer_manager_thread_start) { - vdstestlib::DirConfig config(getStandardConfig(true)); - config.getConfig("stor-visitor").set("visitorthreads", "1"); + _config = StorageConfigSet::make_storage_node_config(); + _config->visitor_config().visitorthreads = 1; _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); - _node = std::make_unique<TestServiceLayerApp>(config.getConfigId()); + _node = std::make_unique<TestServiceLayerApp>(_config->config_uri()); _node->setupDummyPersistence(); _node->getStateUpdater().setClusterState(std::make_shared<lib::ClusterState>("storage:1 distributor:1")); _top = std::make_unique<DummyStorageLink>(); using vespa::config::content::core::StorVisitorConfig; - auto bootstrap_cfg = config_from<StorVisitorConfig>(config::ConfigUri(config.getConfigId())); + auto bootstrap_cfg = config_from<StorVisitorConfig>(_config->config_uri()); auto vm = std::make_unique<VisitorManager>(*bootstrap_cfg, _node->getComponentRegister(), *_messageSessionFactory, @@ -100,7 +103,7 @@ VisitorManagerTest::initializeTest(bool defer_manager_thread_start) _manager = vm.get(); _top->push_back(std::move(vm)); using StorFilestorConfig = vespa::config::content::internal::InternalStorFilestorType; - auto filestor_cfg = config_from<StorFilestorConfig>(config::ConfigUri(config.getConfigId())); + auto filestor_cfg = config_from<StorFilestorConfig>(_config->config_uri()); _top->push_back(std::make_unique<FileStorManager>(*filestor_cfg, _node->getPersistenceProvider(), _node->getComponentRegister(), *_node, _node->get_host_info())); _manager->setTimeBetweenTicks(10); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index f83b6c99d64..075ebd13741 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -1,5 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <tests/common/dummystoragelink.h> +#include <tests/common/storage_config_set.h> +#include <tests/common/testhelper.h> +#include <tests/common/teststorageapp.h> #include <vespa/config/common/exceptions.h> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/fieldvalue/intfieldvalue.h> @@ -14,9 +18,6 @@ #include <vespa/storage/visiting/visitormanager.h> #include <vespa/storageapi/message/datagram.h> #include <vespa/storageapi/message/persistence.h> -#include <tests/common/testhelper.h> -#include <tests/common/teststorageapp.h> -#include <tests/common/dummystoragelink.h> #include <tests/storageserver/testvisitormessagesession.h> #include <vespa/persistence/spi/docentry.h> #include <vespa/vespalib/gtest/gtest.h> @@ -59,6 +60,8 @@ struct TestParams { struct VisitorTest : Test { static uint32_t docCount; + + std::unique_ptr<StorageConfigSet> _config; std::vector<Document::SP> _documents; std::unique_ptr<TestVisitorMessageSessionFactory> _messageSessionFactory; std::unique_ptr<TestServiceLayerApp> _node; @@ -146,31 +149,20 @@ VisitorTest::~VisitorTest() = default; void VisitorTest::initializeTest(const TestParams& params) { - vdstestlib::DirConfig config(getStandardConfig(true, "visitortest")); - config.getConfig("stor-visitor").set("visitorthreads", "1"); - config.getConfig("stor-visitor").set( - "defaultparalleliterators", - std::to_string(params._parallelBuckets)); - config.getConfig("stor-visitor").set( - "visitor_memory_usage_limit", - std::to_string(params._maxVisitorMemoryUsage)); - - std::string rootFolder = getRootFolder(config); - - ::chmod(rootFolder.c_str(), 0755); - std::filesystem::remove_all(std::filesystem::path(rootFolder)); - 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()))); + _config = StorageConfigSet::make_storage_node_config(); + _config->visitor_config().visitorthreads = 1; + _config->visitor_config().defaultparalleliterators = params._parallelBuckets; + _config->visitor_config().visitorMemoryUsageLimit = params._maxVisitorMemoryUsage; _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) { _messageSessionFactory->_autoReplyError = params._autoReplyError; _messageSessionFactory->_createAutoReplyVisitorSessions = true; } - _node = std::make_unique<TestServiceLayerApp>(config.getConfigId()); + _node = std::make_unique<TestServiceLayerApp>(_config->config_uri()); _top = std::make_unique<DummyStorageLink>(); using vespa::config::content::core::StorVisitorConfig; - auto bootstrap_cfg = config_from<StorVisitorConfig>(config::ConfigUri(config.getConfigId())); + auto bootstrap_cfg = config_from<StorVisitorConfig>(_config->config_uri()); _top->push_back(std::unique_ptr<StorageLink>(_manager = new VisitorManager(*bootstrap_cfg, _node->getComponentRegister(), *_messageSessionFactory))); _bottom = new DummyStorageLink(); @@ -217,14 +209,12 @@ VisitorTest::initializeTest(const TestParams& params) _documents.clear(); for (uint32_t i=0; i<docCount; ++i) { std::ostringstream uri; - uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" - << i << ".html"; + uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" << i << ".html"; _documents.push_back(Document::SP( _node->getTestDocMan().createDocument(content, uri.str()))); const document::DocumentType& type(_documents.back()->getType()); - _documents.back()->setValue(type.getField("headerval"), - document::IntFieldValue(i % 4)); + _documents.back()->setValue(type.getField("headerval"), document::IntFieldValue(i % 4)); } } |