diff options
Diffstat (limited to 'storage/src')
65 files changed, 1004 insertions, 614 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)); } } diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp index 6957e541d6b..cfbc4caf82d 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.cpp +++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp @@ -46,6 +46,7 @@ DistributorConfiguration::DistributorConfiguration(StorageComponent& component) _use_weak_internal_read_consistency_for_client_gets(false), _enable_metadata_only_fetch_phase_for_inconsistent_updates(true), _enable_operation_cancellation(false), + _symmetric_put_and_activate_replica_selection(false), _minimumReplicaCountingMode(ReplicaCountingMode::TRUSTED) { } @@ -150,6 +151,7 @@ DistributorConfiguration::configure(const DistributorManagerConfig & config) _max_activation_inhibited_out_of_sync_groups = config.maxActivationInhibitedOutOfSyncGroups; _enable_operation_cancellation = config.enableOperationCancellation; _minimumReplicaCountingMode = deriveReplicaCountingMode(config.minimumReplicaCountingMode); + _symmetric_put_and_activate_replica_selection = config.symmetricPutAndActivateReplicaSelection; if (config.maxClusterClockSkewSec >= 0) { _maxClusterClockSkew = std::chrono::seconds(config.maxClusterClockSkewSec); diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h index 38fac13150c..2b73fdc0fa1 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.h +++ b/storage/src/vespa/storage/config/distributorconfiguration.h @@ -234,8 +234,11 @@ public: [[nodiscard]] bool enable_operation_cancellation() const noexcept { return _enable_operation_cancellation; } + [[nodiscard]] bool symmetric_put_and_activate_replica_selection() const noexcept { + return _symmetric_put_and_activate_replica_selection; + } - bool containsTimeStatement(const std::string& documentSelection) const; + [[nodiscard]] bool containsTimeStatement(const std::string& documentSelection) const; private: StorageComponent& _component; @@ -276,6 +279,7 @@ private: bool _use_weak_internal_read_consistency_for_client_gets; bool _enable_metadata_only_fetch_phase_for_inconsistent_updates; //TODO Rewrite tests and GC bool _enable_operation_cancellation; + bool _symmetric_put_and_activate_replica_selection; ReplicaCountingMode _minimumReplicaCountingMode; }; diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def index 3f6028d7fa1..a4d4461ba68 100644 --- a/storage/src/vespa/storage/config/stor-distributormanager.def +++ b/storage/src/vespa/storage/config/stor-distributormanager.def @@ -159,6 +159,13 @@ num_distributor_stripes int default=0 restart ## requests partially or fully "invalidated" by such a change. enable_operation_cancellation bool default=false +## Iff true there will be an 1-1 symmetry between the replicas chosen as feed targets +## for Put operations and the replica selection logic for bucket activation. In particular, +## the most preferred replica for feed will be the most preferred bucket for activation. +## This helps ensure that new versions of documents are routed to replicas that are most +## likely to reflect these changes as part of visible search results. +symmetric_put_and_activate_replica_selection bool default=false + ## TODO GC very soon, it has no effect. priority_merge_out_of_sync_copies int default=120 diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def index 8cd204bcf9f..44e4b14eafc 100644 --- a/storage/src/vespa/storage/config/stor-server.def +++ b/storage/src/vespa/storage/config/stor-server.def @@ -114,3 +114,7 @@ simulated_bucket_request_latency_msec int default=0 ## a disjoint subset of the node's buckets, in order to reduce locking contention. ## Max value is unspecified, but will be clamped internally. content_node_bucket_db_stripe_bits int default=4 restart + +## Iff set, a special `pidfile` file is written under the node's root directory upon +## startup containing the PID of the running process. +write_pid_file_on_startup bool default=true diff --git a/storage/src/vespa/storage/distributor/activecopy.cpp b/storage/src/vespa/storage/distributor/activecopy.cpp index 35070bcee3b..b823978a0cc 100644 --- a/storage/src/vespa/storage/distributor/activecopy.cpp +++ b/storage/src/vespa/storage/distributor/activecopy.cpp @@ -108,6 +108,7 @@ buildNodeList(const BucketDatabase::Entry& e,vespalib::ConstArrayRef<uint16_t> n struct ActiveStateOrder { bool operator()(const ActiveCopy & e1, const ActiveCopy & e2) noexcept { + // Replica selection order should be kept in sync with OperationTargetResolverImpl's InstanceOrder. if (e1._ready != e2._ready) { return e1._ready; } @@ -120,7 +121,9 @@ struct ActiveStateOrder { if (e1._active != e2._active) { return e1._active; } - return e1.nodeIndex() < e2.nodeIndex(); + // Use _entry_ order instead of node index, as it is in ideal state order (even for retired + // nodes), which avoids unintentional affinities towards lower node indexes. + return e1.entryIndex() < e2.entryIndex(); } }; diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp index 54087850e1b..a92896279b0 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp @@ -180,6 +180,8 @@ void PutOperation::start_direct_put_dispatch(DistributorStripeMessageSender& sen _op_ctx.distributor_config().getMinimalBucketSplit(), _bucket_space.getDistribution().getRedundancy(), _msg->getBucket().getBucketSpace()); + targetResolver.use_symmetric_replica_selection( + _op_ctx.distributor_config().symmetric_put_and_activate_replica_selection()); OperationTargetList targets(targetResolver.getTargets(OperationTargetResolver::PUT, _doc_id_bucket_id)); for (const auto& target : targets) { diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 84e9ab71bcb..849746416d6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -668,7 +668,7 @@ TwoPhaseUpdateOperation::applyUpdateToDocument(document::Document& doc) const bool TwoPhaseUpdateOperation::shouldCreateIfNonExistent() const { - return _updateCmd->getUpdate()->getCreateIfNonExistent(); + return _updateCmd->create_if_missing(); } bool diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp index 7b6833cc299..2b47d53363f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp @@ -29,7 +29,7 @@ UpdateOperation::UpdateOperation(const DistributorNodeContext& node_ctx, _msg(msg), _entries(std::move(entries)), _new_timestamp(_msg->getTimestamp()), - _is_auto_create_update(_msg->getUpdate()->getCreateIfNonExistent()), + _is_auto_create_update(_msg->create_if_missing()), _node_ctx(node_ctx), _op_ctx(op_ctx), _bucketSpace(bucketSpace), @@ -112,6 +112,9 @@ UpdateOperation::onStart(DistributorStripeMessageSender& sender) copyMessageSettings(*_msg, *command); command->setOldTimestamp(_msg->getOldTimestamp()); command->setCondition(_msg->getCondition()); + if (_msg->has_cached_create_if_missing()) { + command->set_cached_create_if_missing(_msg->create_if_missing()); + } messages.emplace_back(std::move(command), node); } diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp index 394c13c2bad..618cfb56359 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp @@ -10,9 +10,12 @@ namespace storage::distributor { BucketInstance::BucketInstance(const document::BucketId& id, const api::BucketInfo& info, lib::Node node, - uint16_t idealLocationPriority, bool trusted, bool exist) noexcept + uint16_t ideal_location_priority, uint16_t db_entry_order, + bool trusted, bool exist) noexcept : _bucket(id), _info(info), _node(node), - _idealLocationPriority(idealLocationPriority), _trusted(trusted), _exist(exist) + _ideal_location_priority(ideal_location_priority), + _db_entry_order(db_entry_order), + _trusted(trusted), _exists(exist) { } @@ -24,8 +27,8 @@ BucketInstance::print(vespalib::asciistream& out, const PrintProperties&) const std::ostringstream ost; ost << std::hex << _bucket.getId(); - out << "(" << ost.str() << ", " << infoString << ", node " << _node.getIndex() << ", ideal " << _idealLocationPriority - << (_trusted ? ", trusted" : "") << (_exist ? "" : ", new copy") << ")"; + out << "(" << ost.str() << ", " << infoString << ", node " << _node.getIndex() << ", ideal " << _ideal_location_priority + << (_trusted ? ", trusted" : "") << (_exists ? "" : ", new copy") << ")"; } bool @@ -42,7 +45,7 @@ BucketInstanceList::add(const BucketDatabase::Entry& e, const IdealServiceLayerN for (uint32_t i = 0; i < e.getBucketInfo().getNodeCount(); ++i) { const BucketCopy& copy(e.getBucketInfo().getNodeRef(i)); lib::Node node(lib::NodeType::STORAGE, copy.getNode()); - _instances.emplace_back(e.getBucketId(), copy.getBucketInfo(), node, idealState.lookup(copy.getNode()), copy.trusted(), true); + _instances.emplace_back(e.getBucketId(), copy.getBucketInfo(), node, idealState.lookup(copy.getNode()), i, copy.trusted(), true); } } @@ -106,7 +109,8 @@ BucketInstanceList::extendToEnoughCopies(const DistributorBucketSpace& distribut for (uint32_t i=0; i<idealNodes.size(); ++i) { lib::Node node(lib::NodeType::STORAGE, idealNodes[i]); if (!contains(node)) { - _instances.emplace_back(newTarget, api::BucketInfo(), node, i, false, false); + // We don't sort `_instances` after extending, so just reuse `i` as dummy DB entry order. + _instances.emplace_back(newTarget, api::BucketInfo(), node, i, i, false, false); } } } @@ -116,7 +120,7 @@ BucketInstanceList::createTargets(document::BucketSpace bucketSpace) { OperationTargetList result; for (const auto& bi : _instances) { - result.emplace_back(document::Bucket(bucketSpace, bi._bucket), bi._node, !bi._exist); + result.emplace_back(document::Bucket(bucketSpace, bi._bucket), bi._node, !bi._exists); } return result; } @@ -129,6 +133,49 @@ BucketInstanceList::print(vespalib::asciistream& out, const PrintProperties& p) namespace { /** + * To maintain a symmetry between which replicas receive Puts and which versions are + * preferred for activation, use an identical ordering predicate for both (for the case + * where replicas are for the same concrete bucket). + * + * Must only be used with BucketInstances that have a distinct _db_entry_order set per instance. + */ +struct ActiveReplicaSymmetricInstanceOrder { + bool operator()(const BucketInstance& a, const BucketInstance& b) noexcept { + if (a._bucket == b._bucket) { + if (a._info.isReady() != b._info.isReady()) { + return a._info.isReady(); + } + if (a._info.getDocumentCount() != b._info.getDocumentCount()) { + return a._info.getDocumentCount() > b._info.getDocumentCount(); + } + if (a._ideal_location_priority != b._ideal_location_priority) { + return a._ideal_location_priority < b._ideal_location_priority; + } + if (a._info.isActive() != b._info.isActive()) { + return a._info.isActive(); + } + // If all else is equal, this implies both A and B are on retired nodes, which is unlikely + // but possible. Fall back to the existing DB _entry order_, which is equal to an ideal + // state order where retired nodes are considered part of the ideal state (which is not the + // case for most ideal state operations). Since the DB entry order is in ideal state order, + // using this instead of node _index_ avoids affinities to lower indexes in such edge cases. + return a._db_entry_order < b._db_entry_order; + } else { + // TODO this inconsistent split case is equal to the legacy logic (aside from the tie-breaking), + // but is considered to be extremely unlikely in practice, so not worth optimizing for. + if ((a._info.getMetaCount() == 0) ^ (b._info.getMetaCount() == 0)) { + return (a._info.getMetaCount() == 0); + } + if (a._bucket.getUsedBits() != b._bucket.getUsedBits()) { + return (a._bucket.getUsedBits() > b._bucket.getUsedBits()); + } + return a._db_entry_order < b._db_entry_order; + } + return false; + } +}; + +/** * - Trusted copies should be preferred over non-trusted copies for the same bucket. * - Buckets in ideal locations should be preferred over non-ideal locations for the * same bucket across several nodes. @@ -137,14 +184,14 @@ namespace { * - Right after split/join, bucket is often not in ideal location, but should be * preferred instead of source anyhow. */ -struct InstanceOrder { - bool operator()(const BucketInstance& a, const BucketInstance& b) { +struct LegacyInstanceOrder { + bool operator()(const BucketInstance& a, const BucketInstance& b) noexcept { if (a._bucket == b._bucket) { - // Trusted only makes sense within same bucket - // Prefer trusted buckets over non-trusted ones. + // Trusted only makes sense within same bucket + // Prefer trusted buckets over non-trusted ones. if (a._trusted != b._trusted) return a._trusted; - if (a._idealLocationPriority != b._idealLocationPriority) { - return a._idealLocationPriority < b._idealLocationPriority; + if (a._ideal_location_priority != b._ideal_location_priority) { + return a._ideal_location_priority < b._ideal_location_priority; } } else { if ((a._info.getMetaCount() == 0) ^ (b._info.getMetaCount() == 0)) { @@ -164,7 +211,11 @@ OperationTargetResolverImpl::getAllInstances(OperationType type, const document: BucketInstanceList instances; if (type == PUT) { instances.populate(id, _distributor_bucket_space, _bucketDatabase); - instances.sort(InstanceOrder()); + if (_symmetric_replica_selection) { + instances.sort(ActiveReplicaSymmetricInstanceOrder()); + } else { + instances.sort(LegacyInstanceOrder()); + } instances.removeNodeDuplicates(); instances.extendToEnoughCopies(_distributor_bucket_space, _bucketDatabase, _bucketDatabase.getAppropriateBucket(_minUsedBucketBits, id), id); diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h index 9f367a89cba..6ab38928200 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h +++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h @@ -15,15 +15,17 @@ struct BucketInstance : public vespalib::AsciiPrintable { document::BucketId _bucket; api::BucketInfo _info; lib::Node _node; - uint16_t _idealLocationPriority; - bool _trusted; - bool _exist; + uint16_t _ideal_location_priority; + uint16_t _db_entry_order; + bool _trusted; // TODO remove + bool _exists; BucketInstance() noexcept - : _idealLocationPriority(0xffff), _trusted(false), _exist(false) {} + : _ideal_location_priority(0xffff), _db_entry_order(0xffff), _trusted(false), _exists(false) + {} BucketInstance(const document::BucketId& id, const api::BucketInfo& info, - lib::Node node, uint16_t idealLocationPriority, bool trusted, - bool exist) noexcept; + lib::Node node, uint16_t ideal_location_priority, + uint16_t db_entry_order, bool trusted, bool exist) noexcept; void print(vespalib::asciistream& out, const PrintProperties&) const override; }; @@ -83,6 +85,7 @@ class OperationTargetResolverImpl : public OperationTargetResolver { uint32_t _minUsedBucketBits; uint16_t _redundancy; document::BucketSpace _bucketSpace; + bool _symmetric_replica_selection; public: OperationTargetResolverImpl(const DistributorBucketSpace& distributor_bucket_space, @@ -94,9 +97,14 @@ public: _bucketDatabase(bucketDatabase), _minUsedBucketBits(minUsedBucketBits), _redundancy(redundancy), - _bucketSpace(bucketSpace) + _bucketSpace(bucketSpace), + _symmetric_replica_selection(true) {} + void use_symmetric_replica_selection(bool symmetry) noexcept { + _symmetric_replica_selection = symmetry; + } + BucketInstanceList getAllInstances(OperationType type, const document::BucketId& id); BucketInstanceList getInstances(OperationType type, const document::BucketId& id) { BucketInstanceList result(getAllInstances(type, id)); diff --git a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp index c3cb38bd7ac..582c69e943f 100644 --- a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp @@ -20,8 +20,8 @@ MergeHandlerMetrics::MergeHandlerMetrics(metrics::MetricSet* owner) "current node.", owner), mergeAverageDataReceivedNeeded("mergeavgdatareceivedneeded", {}, "Amount of data transferred from previous node " "in chain that we needed to apply locally.", owner), - put_latency("put_latency", {}, "Latency of individual puts that are part of merge operations", owner), - remove_latency("remove_latency", {}, "Latency of individual removes that are part of merge operations", owner) + merge_put_latency("merge_put_latency", {}, "Latency of individual puts that are part of merge operations", owner), + merge_remove_latency("merge_remove_latency", {}, "Latency of individual removes that are part of merge operations", owner) {} MergeHandlerMetrics::~MergeHandlerMetrics() = default; diff --git a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h index 44b85570357..a2d68011695 100644 --- a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h +++ b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h @@ -21,8 +21,8 @@ struct MergeHandlerMetrics { metrics::DoubleAverageMetric mergeAverageDataReceivedNeeded; // Individual operation metrics. These capture both count and latency sum, so // no need for explicit count metric on the side. - metrics::DoubleAverageMetric put_latency; - metrics::DoubleAverageMetric remove_latency; + metrics::DoubleAverageMetric merge_put_latency; + metrics::DoubleAverageMetric merge_remove_latency; // Iteration over metadata and document payload data is already covered by // the merge[Meta]Data(Read|Write)Latency metrics, so not repeated here. Can be // explicitly added if deemed required. diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 7ee2d9f37bf..b3207428f5f 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -4,13 +4,14 @@ #include "persistenceutil.h" #include "apply_bucket_diff_entry_complete.h" #include "apply_bucket_diff_state.h" -#include <vespa/storage/persistence/filestorage/mergestatus.h> -#include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> -#include <vespa/vdslib/distribution/distribution.h> #include <vespa/document/fieldset/fieldsets.h> #include <vespa/document/fieldvalue/document.h> +#include <vespa/persistence/spi/docentry.h> +#include <vespa/persistence/spi/persistenceprovider.h> +#include <vespa/storage/persistence/filestorage/mergestatus.h> +#include <vespa/vdslib/distribution/distribution.h> #include <vespa/vespalib/objects/nbostream.h> +#include <vespa/vespalib/stllike/hash_map.hpp> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/isequencedtaskexecutor.h> #include <algorithm> @@ -506,8 +507,18 @@ void MergeHandler::applyDiffEntry(std::shared_ptr<ApplyBucketDiffState> async_results, const spi::Bucket& bucket, const api::ApplyBucketDiffCommand::Entry& e, - const document::DocumentTypeRepo& repo) const + const document::DocumentTypeRepo& repo, + const NewestDocumentVersionMapping& newest_per_doc) const { + if (!e._docName.empty()) { + auto version_iter = newest_per_doc.find(e._docName); + assert(version_iter != newest_per_doc.end()); + if (e._entry._timestamp != version_iter->second) { + LOG(spam, "ApplyBucketDiff(%s): skipping diff entry %s since it is subsumed by a newer timestamp %" PRIu64, + bucket.toString().c_str(), e.toString().c_str(), version_iter->second); + return; + } + } auto throttle_token = _env._fileStorHandler.operation_throttler().blocking_acquire_one(); spi::Timestamp timestamp(e._entry._timestamp); if (!(e._entry._flags & (DELETED | DELETED_IN_PLACE))) { @@ -516,14 +527,14 @@ MergeHandler::applyDiffEntry(std::shared_ptr<ApplyBucketDiffState> async_results document::DocumentId docId = doc->getId(); auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), std::move(docId), std::move(throttle_token), "put", - _clock, _env._metrics.merge_handler_metrics.put_latency); + _clock, _env._metrics.merge_handler_metrics.merge_put_latency); _spi.putAsync(bucket, timestamp, std::move(doc), std::move(complete)); } else { std::vector<spi::IdAndTimestamp> ids; ids.emplace_back(document::DocumentId(e._docName), timestamp); auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), ids[0].id, std::move(throttle_token), "remove", - _clock, _env._metrics.merge_handler_metrics.remove_latency); + _clock, _env._metrics.merge_handler_metrics.merge_remove_latency); _spi.removeAsync(bucket, std::move(ids), std::move(complete)); } } @@ -548,6 +559,7 @@ MergeHandler::applyDiffLocally(const spi::Bucket& bucket, std::vector<api::Apply DocEntryList entries; populateMetaData(bucket, Timestamp::max(), entries, context); + const auto newest_versions = enumerate_newest_document_versions(diff); const document::DocumentTypeRepo & repo = _env.getDocumentTypeRepo(); uint32_t existingCount = entries.size(); @@ -580,7 +592,7 @@ MergeHandler::applyDiffLocally(const spi::Bucket& bucket, std::vector<api::Apply ++i; LOG(spam, "ApplyBucketDiff(%s): Adding slot %s", bucket.toString().c_str(), e.toString().c_str()); - applyDiffEntry(async_results, bucket, e, repo); + applyDiffEntry(async_results, bucket, e, repo, newest_versions); } else { assert(spi::Timestamp(e._entry._timestamp) == existing.getTimestamp()); // Diffing for existing timestamp; should either both be put @@ -591,7 +603,7 @@ MergeHandler::applyDiffLocally(const spi::Bucket& bucket, std::vector<api::Apply if ((e._entry._flags & DELETED) && !existing.isRemove()) { LOG(debug, "Slot in diff is remove for existing timestamp in %s. Diff slot: %s. Existing slot: %s", bucket.toString().c_str(), e.toString().c_str(), existing.toString().c_str()); - applyDiffEntry(async_results, bucket, e, repo); + applyDiffEntry(async_results, bucket, e, repo, newest_versions); } else { // Duplicate put, just ignore it. LOG(debug, "During diff apply, attempting to add slot whose timestamp already exists in %s, " @@ -619,7 +631,7 @@ MergeHandler::applyDiffLocally(const spi::Bucket& bucket, std::vector<api::Apply LOG(spam, "ApplyBucketDiff(%s): Adding slot %s", bucket.toString().c_str(), e.toString().c_str()); - applyDiffEntry(async_results, bucket, e, repo); + applyDiffEntry(async_results, bucket, e, repo, newest_versions); byteCount += e._headerBlob.size() + e._bodyBlob.size(); } if (byteCount + notNeededByteCount != 0) { @@ -631,6 +643,27 @@ MergeHandler::applyDiffLocally(const spi::Bucket& bucket, std::vector<api::Apply bucket.toString().c_str(), addedCount); } +MergeHandler::NewestDocumentVersionMapping +MergeHandler::enumerate_newest_document_versions(const std::vector<api::ApplyBucketDiffCommand::Entry>& diff) +{ + NewestDocumentVersionMapping newest_per_doc; + for (const auto& e : diff) { + // We expect the doc name to always be filled out, both for remove operations and for puts. + // But since the latter is technically redundant (ID is also found within the document), we + // guard on this to be forwards compatible in case this changes (e.g. to populate and use + // the GID field instead). Fallback to the legacy behavior if so. + if (e._docName.empty()) { + continue; + } + auto [existing_iter, inserted] = newest_per_doc.insert(std::make_pair(vespalib::stringref(e._docName), e._entry._timestamp)); + if (!inserted) { + assert(existing_iter != newest_per_doc.end()); + existing_iter->second = std::max(existing_iter->second, e._entry._timestamp); + } + } + return newest_per_doc; +} + void MergeHandler::sync_bucket_info(const spi::Bucket& bucket) const { diff --git a/storage/src/vespa/storage/persistence/mergehandler.h b/storage/src/vespa/storage/persistence/mergehandler.h index f3bef802229..2be45e7bc8b 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.h +++ b/storage/src/vespa/storage/persistence/mergehandler.h @@ -18,6 +18,7 @@ #include <vespa/storageapi/message/bucket.h> #include <vespa/storage/common/cluster_context.h> #include <vespa/storage/common/messagesender.h> +#include <vespa/vespalib/stllike/hash_map.h> #include <vespa/vespalib/util/monitored_refcount.h> #include <vespa/storageframework/generic/clock/time.h> @@ -42,6 +43,8 @@ private: using MessageTrackerUP = std::unique_ptr<MessageTracker>; using Timestamp = framework::MicroSecTime; public: + using NewestDocumentVersionMapping = vespalib::hash_map<vespalib::stringref, api::Timestamp>; + enum StateFlag { IN_USE = 0x01, DELETED = 0x02, @@ -72,6 +75,17 @@ public: void handleApplyBucketDiffReply(api::ApplyBucketDiffReply&, MessageSender&, MessageTrackerUP) const; void drain_async_writes(); + /** + * Returns a mapping that, for each document ID, contains the newest version of that document that + * is present in the diff. + * + * The returned hash_map keys point directly into the `ApplyBucketDiffCommand::Entry::_docName` memory + * owned by `diff`, so this memory must remain unchanged and stable for the duration of the returned + * mapping's lifetime. + */ + static NewestDocumentVersionMapping enumerate_newest_document_versions( + const std::vector<api::ApplyBucketDiffCommand::Entry>& diff); + private: using DocEntryList = std::vector<std::unique_ptr<spi::DocEntry>>; const framework::Clock &_clock; @@ -90,9 +104,13 @@ private: /** * Invoke either put, remove or unrevertable remove on the SPI * depending on the flags in the diff entry. + * + * If `newest_doc_version` indicates that the entry is not the newest version present in the + * diff, the entry is silently ignored and is _not_ invoked on the SPI. */ void applyDiffEntry(std::shared_ptr<ApplyBucketDiffState> async_results, const spi::Bucket&, - const api::ApplyBucketDiffCommand::Entry&, const document::DocumentTypeRepo& repo) const; + const api::ApplyBucketDiffCommand::Entry&, const document::DocumentTypeRepo& repo, + const NewestDocumentVersionMapping& newest_per_doc) const; /** * Fill entries-vector with metadata for bucket up to maxTimestamp, diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index ca46e87285b..5b8052a05f8 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -54,6 +54,9 @@ DocumentApiConverter::toStorageAPI(documentapi::DocumentMessage& fromMsg) auto to = std::make_unique<api::UpdateCommand>(bucket, from.stealDocumentUpdate(), from.getNewTimestamp()); to->setOldTimestamp(from.getOldTimestamp()); to->setCondition(from.getCondition()); + if (from.has_cached_create_if_missing()) { + to->set_cached_create_if_missing(from.create_if_missing()); + } toMsg = std::move(to); break; } @@ -217,6 +220,9 @@ DocumentApiConverter::toDocumentAPI(api::StorageCommand& fromMsg) to->setOldTimestamp(from.getOldTimestamp()); to->setNewTimestamp(from.getTimestamp()); to->setCondition(from.getCondition()); + if (from.has_cached_create_if_missing()) { + to->set_cached_create_if_missing(from.create_if_missing()); + } toMsg = std::move(to); break; } diff --git a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp index 4d74eb1974b..3b53c0c9584 100644 --- a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp @@ -124,7 +124,8 @@ void ClusterControllerApiRpcService::RPC_setSystemState2(FRT_RPCRequest* req) { req->GetParams()->GetValue(0)._string._len); lib::ClusterState systemState(systemStateStr); - auto cmd = std::make_shared<api::SetSystemStateCommand>(lib::ClusterStateBundle(systemState)); + auto bundle = std::make_shared<const lib::ClusterStateBundle>(systemState); + auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(bundle)); cmd->setPriority(api::StorageMessage::VERYHIGH); detach_and_forward_to_enqueuer(std::move(cmd), req); @@ -167,8 +168,7 @@ void ClusterControllerApiRpcService::RPC_setDistributionStates(FRT_RPCRequest* r } LOG(debug, "Got state bundle %s", state_bundle->toString().c_str()); - // TODO add constructor taking in shared_ptr directly instead? - auto cmd = std::make_shared<api::SetSystemStateCommand>(*state_bundle); + auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(state_bundle)); cmd->setPriority(api::StorageMessage::VERYHIGH); detach_and_forward_to_enqueuer(std::move(cmd), req); diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp index f7a426a0527..35b70dd853c 100644 --- a/storage/src/vespa/storage/storageserver/storagenode.cpp +++ b/storage/src/vespa/storage/storageserver/storagenode.cpp @@ -140,7 +140,7 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) // Initializing state manager early, as others use it init time to // update node state according min used bits etc. // Needs node type to be set right away. Needs thread pool, index and - // dead lock detector too, but not before open() + // deadlock detector too, but not before open() _stateManager = std::make_unique<StateManager>( _context.getComponentRegister(), std::move(_hostInfo), @@ -148,10 +148,10 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) _singleThreadedDebugMode); _context.getComponentRegister().setNodeStateUpdater(*_stateManager); - // Create VDS root folder, in case it doesn't already exist. - // Maybe better to rather fail if it doesn't exist, but tests - // might break if we do that. Might alter later. - std::filesystem::create_directories(std::filesystem::path(_rootFolder)); + // Create storage root folder, in case it doesn't already exist. + if (!_rootFolder.empty()) { + std::filesystem::create_directories(std::filesystem::path(_rootFolder)); + } // else: running as part of unit tests initializeNodeSpecific(); @@ -192,13 +192,16 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) initializeStatusWebServer(); + if (server_config().writePidFileOnStartup) { + assert(!_rootFolder.empty()); // Write pid file as the last thing we do. If we fail initialization // due to an exception we won't run shutdown. Thus we won't remove the // pid file if something throws after writing it in initialization. // Initialize _pidfile here, such that we can know that we didn't create // it in shutdown code for shutdown during init. - _pidFile = _rootFolder + "/pidfile"; - writePidFile(_pidFile); + _pidFile = _rootFolder + "/pidfile"; + writePidFile(_pidFile); + } } void diff --git a/storage/src/vespa/storageapi/mbusprot/protobuf/feed.proto b/storage/src/vespa/storageapi/mbusprot/protobuf/feed.proto index 55d516a017b..403752b0c84 100644 --- a/storage/src/vespa/storageapi/mbusprot/protobuf/feed.proto +++ b/storage/src/vespa/storageapi/mbusprot/protobuf/feed.proto @@ -31,11 +31,18 @@ message Update { } message UpdateRequest { - Bucket bucket = 1; - Update update = 2; - uint64 new_timestamp = 3; - uint64 expected_old_timestamp = 4; // If zero; no expectation. - TestAndSetCondition condition = 5; + enum CreateIfMissing { + UNSPECIFIED = 0; // Legacy fallback: must deserialize `update` to find flag value + TRUE = 1; + FALSE = 2; + } + + Bucket bucket = 1; + Update update = 2; + uint64 new_timestamp = 3; + uint64 expected_old_timestamp = 4; // If zero; no expectation. + TestAndSetCondition condition = 5; + CreateIfMissing create_if_missing = 6; } message UpdateResponse { diff --git a/storage/src/vespa/storageapi/mbusprot/protocolserialization7.cpp b/storage/src/vespa/storageapi/mbusprot/protocolserialization7.cpp index 57047be6037..0f4a34cc775 100644 --- a/storage/src/vespa/storageapi/mbusprot/protocolserialization7.cpp +++ b/storage/src/vespa/storageapi/mbusprot/protocolserialization7.cpp @@ -465,6 +465,10 @@ void ProtocolSerialization7::onEncode(GBBuf& buf, const api::UpdateCommand& msg) if (msg.getCondition().isPresent()) { set_tas_condition(*req.mutable_condition(), msg.getCondition()); } + if (msg.has_cached_create_if_missing()) { + req.set_create_if_missing(msg.create_if_missing() ? protobuf::UpdateRequest_CreateIfMissing_TRUE + : protobuf::UpdateRequest_CreateIfMissing_FALSE); + } }); } @@ -482,6 +486,9 @@ api::StorageCommand::UP ProtocolSerialization7::onDecodeUpdateCommand(BBuf& buf) if (req.has_condition()) { cmd->setCondition(get_tas_condition(req.condition())); } + if (req.create_if_missing() != protobuf::UpdateRequest_CreateIfMissing_UNSPECIFIED) { + cmd->set_cached_create_if_missing(req.create_if_missing() == protobuf::UpdateRequest_CreateIfMissing_TRUE); + } return cmd; }); } diff --git a/storage/src/vespa/storageapi/message/persistence.cpp b/storage/src/vespa/storageapi/message/persistence.cpp index 4c24bb74faf..af054855bbe 100644 --- a/storage/src/vespa/storageapi/message/persistence.cpp +++ b/storage/src/vespa/storageapi/message/persistence.cpp @@ -105,13 +105,23 @@ UpdateCommand::UpdateCommand(const document::Bucket &bucket, const document::Doc : TestAndSetCommand(MessageType::UPDATE, bucket), _update(update), _timestamp(time), - _oldTimestamp(0) + _oldTimestamp(0), + _create_if_missing() { if ( ! _update) { throw vespalib::IllegalArgumentException("Cannot update a null update", VESPA_STRLOC); } } +bool +UpdateCommand::create_if_missing() const +{ + if (_create_if_missing.has_value()) { + return *_create_if_missing; + } + return _update->getCreateIfNonExistent(); +} + const document::DocumentType * UpdateCommand::getDocumentType() const { return &_update->getType(); diff --git a/storage/src/vespa/storageapi/message/persistence.h b/storage/src/vespa/storageapi/message/persistence.h index f44ab4e8280..0676e1d0f44 100644 --- a/storage/src/vespa/storageapi/message/persistence.h +++ b/storage/src/vespa/storageapi/message/persistence.h @@ -1,8 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. /** - * @file persistence.h - * - * Persistence related commands, like put, get & remove + * Persistence related commands, like put, get & remove */ #pragma once @@ -10,6 +8,7 @@ #include <vespa/storageapi/defs.h> #include <vespa/document/base/documentid.h> #include <vespa/documentapi/messagebus/messages/testandsetcondition.h> +#include <optional> namespace document { class DocumentUpdate; @@ -117,20 +116,32 @@ class UpdateCommand : public TestAndSetCommand { std::shared_ptr<document::DocumentUpdate> _update; Timestamp _timestamp; Timestamp _oldTimestamp; + std::optional<bool> _create_if_missing; // caches the value held (possibly lazily deserialized) in _update public: UpdateCommand(const document::Bucket &bucket, const std::shared_ptr<document::DocumentUpdate>&, Timestamp); ~UpdateCommand() override; - void setTimestamp(Timestamp ts) { _timestamp = ts; } - void setOldTimestamp(Timestamp ts) { _oldTimestamp = ts; } + void setTimestamp(Timestamp ts) noexcept { _timestamp = ts; } + void setOldTimestamp(Timestamp ts) noexcept { _oldTimestamp = ts; } + + [[nodiscard]] bool has_cached_create_if_missing() const noexcept { + return _create_if_missing.has_value(); + } + // It is the caller's responsibility to ensure this value matches that of _update->getCreateIfNonExisting() + void set_cached_create_if_missing(bool create) noexcept { + _create_if_missing = create; + } const std::shared_ptr<document::DocumentUpdate>& getUpdate() const { return _update; } const document::DocumentId& getDocumentId() const override; Timestamp getTimestamp() const { return _timestamp; } Timestamp getOldTimestamp() const { return _oldTimestamp; } + // May throw iff has_cached_create_if_missing() == false, otherwise noexcept. + [[nodiscard]] bool create_if_missing() const; + const document::DocumentType * getDocumentType() const override; vespalib::string getSummary() const override; diff --git a/storage/src/vespa/storageapi/message/state.cpp b/storage/src/vespa/storageapi/message/state.cpp index 5a50167f584..b4e8655d783 100644 --- a/storage/src/vespa/storageapi/message/state.cpp +++ b/storage/src/vespa/storageapi/message/state.cpp @@ -5,8 +5,7 @@ #include <vespa/vdslib/state/clusterstate.h> #include <ostream> -namespace storage { -namespace api { +namespace storage::api { IMPLEMENT_COMMAND(GetNodeStateCommand, GetNodeStateReply) IMPLEMENT_REPLY(GetNodeStateReply) @@ -45,7 +44,7 @@ GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd) GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd, const lib::NodeState& state) : StorageReply(cmd), - _state(new lib::NodeState(state)) + _state(std::make_unique<lib::NodeState>(state)) { } @@ -64,23 +63,31 @@ GetNodeStateReply::print(std::ostream& out, bool verbose, } } +SetSystemStateCommand::SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state) + : StorageCommand(MessageType::SETSYSTEMSTATE), + _state(std::move(state)) +{ +} + SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterStateBundle& state) : StorageCommand(MessageType::SETSYSTEMSTATE), - _state(state) + _state(std::make_shared<const lib::ClusterStateBundle>(state)) { } SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterState& state) : StorageCommand(MessageType::SETSYSTEMSTATE), - _state(state) + _state(std::make_shared<const lib::ClusterStateBundle>(state)) { } +SetSystemStateCommand::~SetSystemStateCommand() = default; + void SetSystemStateCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "SetSystemStateCommand(" << *_state.getBaselineClusterState() << ")"; + out << "SetSystemStateCommand(" << *_state->getBaselineClusterState() << ")"; if (verbose) { out << " : "; StorageCommand::print(out, verbose, indent); @@ -89,7 +96,7 @@ SetSystemStateCommand::print(std::ostream& out, bool verbose, SetSystemStateReply::SetSystemStateReply(const SetSystemStateCommand& cmd) : StorageReply(cmd), - _state(cmd.getClusterStateBundle()) + _state(cmd.cluster_state_bundle_ptr()) { } @@ -138,5 +145,4 @@ void ActivateClusterStateVersionReply::print(std::ostream& out, bool verbose, } } -} // api -} // storage +} // storage::api diff --git a/storage/src/vespa/storageapi/message/state.h b/storage/src/vespa/storageapi/message/state.h index 900355b12a2..4aa4c8a8f31 100644 --- a/storage/src/vespa/storageapi/message/state.h +++ b/storage/src/vespa/storageapi/message/state.h @@ -61,13 +61,19 @@ public: * put/get/remove etx) */ class SetSystemStateCommand : public StorageCommand { - lib::ClusterStateBundle _state; + std::shared_ptr<const lib::ClusterStateBundle> _state; public: + explicit SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state); explicit SetSystemStateCommand(const lib::ClusterStateBundle &state); explicit SetSystemStateCommand(const lib::ClusterState &state); - const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); } - const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; } + ~SetSystemStateCommand() override; + + [[nodiscard]] const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); } + [[nodiscard]] const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; } + [[nodiscard]] std::shared_ptr<const lib::ClusterStateBundle> cluster_state_bundle_ptr() const noexcept { + return _state; + } void print(std::ostream& out, bool verbose, const std::string& indent) const override; DECLARE_STORAGECOMMAND(SetSystemStateCommand, onSetSystemState) @@ -80,14 +86,14 @@ public: * @brief Reply received after a SetSystemStateCommand. */ class SetSystemStateReply : public StorageReply { - lib::ClusterStateBundle _state; + std::shared_ptr<const lib::ClusterStateBundle> _state; public: explicit SetSystemStateReply(const SetSystemStateCommand& cmd); // Not serialized. Available locally - const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); } - const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; } + const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); } + const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; } void print(std::ostream& out, bool verbose, const std::string& indent) const override; DECLARE_STORAGEREPLY(SetSystemStateReply, onSetSystemStateReply) |