diff options
author | Tor Brede Vekterli <vekterli@vespa.ai> | 2024-05-14 13:28:20 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@vespa.ai> | 2024-05-14 13:28:20 +0000 |
commit | f8eb4ce5e2eaad0899125c7117446acd578f72b5 (patch) | |
tree | 8e08f94477acf64818631808c43f399b5da519d5 | |
parent | 33166c63cd5c27a4f6c09b9e5b92f64ea36ee7ab (diff) |
Remove legacy storage node root directory IO in unit tests
Once upon a time, VDS roamed the lands. It used real disk IO as
part of tests. Then came the meteor and in-memory dummy persistence
took over. Now it is time for the fossils to be moved into a museum
where they belong.
Also make PID file writing conditional on a config that is set to
`false` during unit testing (but `true` as default).
-rw-r--r-- | storage/src/tests/bucketdb/bucketmanagertest.cpp | 27 | ||||
-rw-r--r-- | storage/src/tests/common/metricstest.cpp | 16 | ||||
-rw-r--r-- | storage/src/tests/common/testhelper.cpp | 1 | ||||
-rw-r--r-- | storage/src/tests/persistence/persistencetestutils.cpp | 6 | ||||
-rw-r--r-- | storage/src/tests/storageserver/statereportertest.cpp | 1 | ||||
-rw-r--r-- | storage/src/tests/visiting/visitortest.cpp | 13 | ||||
-rw-r--r-- | storage/src/vespa/storage/config/stor-server.def | 4 | ||||
-rw-r--r-- | storage/src/vespa/storage/storageserver/storagenode.cpp | 17 | ||||
-rw-r--r-- | storageserver/src/tests/testhelper.cpp | 10 |
9 files changed, 34 insertions, 61 deletions
diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp index 92547a83d25..b5501d38241 100644 --- a/storage/src/tests/bucketdb/bucketmanagertest.cpp +++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp @@ -69,8 +69,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); @@ -131,15 +130,9 @@ std::string getMkDirDisk(const std::string & rootFolder, int 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); auto repo = std::make_shared<const DocumentTypeRepo>( *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec("../config-doctypes.cfg"))); @@ -153,18 +146,10 @@ void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noD auto manager = std::make_unique<BucketManager>(*config_from<StorServerConfig>(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/metricstest.cpp b/storage/src/tests/common/metricstest.cpp index 899c1979e86..7216bef03db 100644 --- a/storage/src/tests/common/metricstest.cpp +++ b/storage/src/tests/common/metricstest.cpp @@ -67,16 +67,12 @@ 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()); - } + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); + _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", {}, "")); { diff --git a/storage/src/tests/common/testhelper.cpp b/storage/src/tests/common/testhelper.cpp index f295d955ab2..91758b894b0 100644 --- a/storage/src/tests/common/testhelper.cpp +++ b/storage/src/tests/common/testhelper.cpp @@ -68,6 +68,7 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & ro rootFolder += (storagenode ? "vdsroot" : "vdsroot.distributor"); config->set("root_folder", rootFolder); config->set("is_distributor", (storagenode ? "false" : "true")); + config->set("write_pid_file_on_startup", "false"); config = &dc.addConfig("stor-devices"); config->set("root_folder", rootFolder); config = &dc.addConfig("stor-status"); diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index a599b1d380a..4a2ee987362 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -26,11 +26,7 @@ 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; + return getStandardConfig(true, rootOfRoot); } template<typename T> diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp index 43eb37afe15..c233c6e9314 100644 --- a/storage/src/tests/storageserver/statereportertest.cpp +++ b/storage/src/tests/storageserver/statereportertest.cpp @@ -69,7 +69,6 @@ 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()); _node->setupDummyPersistence(); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index f83b6c99d64..29a6c1baeb4 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -155,13 +155,6 @@ VisitorTest::initializeTest(const TestParams& params) "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()))); - _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) { _messageSessionFactory->_autoReplyError = params._autoReplyError; @@ -217,14 +210,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/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/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/storageserver/src/tests/testhelper.cpp b/storageserver/src/tests/testhelper.cpp index 40e263d4e68..e3b047ddfd1 100644 --- a/storageserver/src/tests/testhelper.cpp +++ b/storageserver/src/tests/testhelper.cpp @@ -62,13 +62,11 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode) { config->set("enable_dead_lock_detector_warnings", "false"); config->set("max_merges_per_node", "25"); config->set("max_merge_queue_size", "20"); - config->set("root_folder", - (storagenode ? "vdsroot" : "vdsroot.distributor")); - config->set("is_distributor", - (storagenode ? "false" : "true")); + config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor")); + config->set("is_distributor", (storagenode ? "false" : "true")); + config->set("write_pid_file_on_startup", "false"); config = &dc.addConfig("stor-devices"); - config->set("root_folder", - (storagenode ? "vdsroot" : "vdsroot.distributor")); + config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor")); config = &dc.addConfig("stor-status"); config->set("httpport", "0"); config = &dc.addConfig("stor-visitor"); |