diff options
Diffstat (limited to 'storage')
10 files changed, 14 insertions, 113 deletions
diff --git a/storage/pom.xml b/storage/pom.xml index e0b21d5a8a9..ba47fe84eae 100644 --- a/storage/pom.xml +++ b/storage/pom.xml @@ -7,11 +7,11 @@ <parent> <groupId>com.yahoo.vespa</groupId> <artifactId>parent</artifactId> - <version>6-SNAPSHOT</version> + <version>7-SNAPSHOT</version> <relativePath>../parent/pom.xml</relativePath> </parent> <artifactId>storage</artifactId> - <version>6-SNAPSHOT</version> + <version>7-SNAPSHOT</version> <packaging>jar</packaging> <name>${project.artifactId}</name> <dependencies> diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp index 56f88b7f98f..53f80854bef 100644 --- a/storage/src/tests/distributor/bucketdbupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp @@ -61,7 +61,6 @@ class BucketDBUpdaterTest : public CppUnit::TestFixture, CPPUNIT_TEST(testDistributorChangeWithGrouping); CPPUNIT_TEST(testNormalUsageInitializing); // Check that we send request bucket info when storage node is initializing, and send another when it's up. CPPUNIT_TEST(testFailedRequestBucketInfo); - CPPUNIT_TEST(testEncodeErrorHandling); CPPUNIT_TEST(testBitChange); // Check what happens when distribution bits change CPPUNIT_TEST(testNodeDown); CPPUNIT_TEST(testStorageNodeInMaintenanceClearsBucketsForNode); @@ -123,7 +122,6 @@ protected: void testDistributorChangeWithGrouping(); void testNormalUsageInitializing(); void testFailedRequestBucketInfo(); - void testEncodeErrorHandling(); void testNoResponses(); void testBitChange(); void testInconsistentChecksum(); @@ -811,29 +809,6 @@ BucketDBUpdaterTest::testFailedRequestBucketInfo() } void -BucketDBUpdaterTest::testEncodeErrorHandling() -{ - setSystemState(lib::ClusterState("distributor:1 .0.s:i storage:1")); - - CPPUNIT_ASSERT_EQUAL(_bucketSpaces.size(), _sender.commands.size()); - - // Not yet passing on system state. - CPPUNIT_ASSERT_EQUAL(size_t(0), _senderDown.commands.size()); - for (uint32_t i = 0; i < _bucketSpaces.size(); ++i) { - std::shared_ptr<api::RequestBucketInfoReply> reply = - getFakeBucketReply(lib::ClusterState("distributor:1 .0.s:i storage:1"), - *((RequestBucketInfoCommand*)_sender.commands[i].get()), - 0, - 10); - - reply->setResult(api::ReturnCode::ENCODE_ERROR); - getBucketDBUpdater().onRequestBucketInfoReply(reply); - } - CPPUNIT_ASSERT_EQUAL(std::string("Set system state"), - _senderDown.getCommands()); -} - -void BucketDBUpdaterTest::testDownWhileInit() { setStorageNodes(3); diff --git a/storage/src/tests/storageserver/communicationmanagertest.cpp b/storage/src/tests/storageserver/communicationmanagertest.cpp index 3b72585b076..6af2733f3b9 100644 --- a/storage/src/tests/storageserver/communicationmanagertest.cpp +++ b/storage/src/tests/storageserver/communicationmanagertest.cpp @@ -29,7 +29,7 @@ struct CommunicationManagerTest : public CppUnit::TestFixture { void testRepliesAreDequeuedInFifoOrder(); void bucket_space_config_can_be_updated_live(); void unmapped_bucket_space_documentapi_request_returns_error_reply(); - void unmapped_bucket_space_for_get_documentapi_request_returns_empty_reply(); + void unmapped_bucket_space_for_get_documentapi_request_returns_error_reply(); static constexpr uint32_t MESSAGE_WAIT_TIME_SEC = 60; @@ -54,7 +54,7 @@ struct CommunicationManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST(testRepliesAreDequeuedInFifoOrder); CPPUNIT_TEST(bucket_space_config_can_be_updated_live); CPPUNIT_TEST(unmapped_bucket_space_documentapi_request_returns_error_reply); - CPPUNIT_TEST(unmapped_bucket_space_for_get_documentapi_request_returns_empty_reply); + CPPUNIT_TEST(unmapped_bucket_space_for_get_documentapi_request_returns_error_reply); CPPUNIT_TEST_SUITE_END(); }; @@ -349,11 +349,7 @@ void CommunicationManagerTest::unmapped_bucket_space_documentapi_request_returns CPPUNIT_ASSERT_EQUAL(uint64_t(1), f.comm_mgr->metrics().bucketSpaceMappingFailures.getValue()); } -// Legacy DocumentAPI routing protocols will send Gets to _all_ clusters even -// if they do not contain a particular document type. By sending an empty reply -// we signal a mergeable "not found" to the sender rather than a non-mergeable -// fatal error. -void CommunicationManagerTest::unmapped_bucket_space_for_get_documentapi_request_returns_empty_reply() { +void CommunicationManagerTest::unmapped_bucket_space_for_get_documentapi_request_returns_error_reply() { CommunicationManagerFixture f; BucketspacesConfigBuilder config; @@ -363,9 +359,9 @@ void CommunicationManagerTest::unmapped_bucket_space_for_get_documentapi_request f.comm_mgr->handleMessage(f.documentapi_get_message_for_space("fluff")); CPPUNIT_ASSERT_EQUAL(size_t(1), f.reply_handler.replies.size()); auto& reply = *f.reply_handler.replies[0]; - CPPUNIT_ASSERT(!reply.hasErrors()); - auto& get_reply = dynamic_cast<documentapi::GetDocumentReply&>(reply); - CPPUNIT_ASSERT(!get_reply.hasDocument()); + CPPUNIT_ASSERT(reply.hasErrors()); + CPPUNIT_ASSERT_EQUAL(static_cast<uint32_t>(api::ReturnCode::REJECTED), reply.getError(0).getCode()); + CPPUNIT_ASSERT_EQUAL(uint64_t(1), f.comm_mgr->metrics().bucketSpaceMappingFailures.getValue()); } } // storage diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 11e5e355f0c..46b0f4d830d 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -30,10 +30,6 @@ using msg_ptr_vector = std::vector<api::StorageMessage::SP>; struct TestParams { - TestParams& iteratorsPerBucket(uint32_t n) { - _iteratorsPerBucket = n; - return *this; - } TestParams& maxVisitorMemoryUsage(uint32_t bytes) { _maxVisitorMemoryUsage = bytes; return *this; @@ -47,7 +43,6 @@ struct TestParams return *this; } - uint32_t _iteratorsPerBucket {1}; uint32_t _maxVisitorMemoryUsage {UINT32_MAX}; uint32_t _parallelBuckets {1}; mbus::Error _autoReplyError; @@ -62,7 +57,6 @@ private: CPPUNIT_TEST(testNormalUsage); CPPUNIT_TEST(testFailedCreateIterator); CPPUNIT_TEST(testFailedGetIter); - CPPUNIT_TEST(iterators_per_bucket_config_is_ignored_and_hardcoded_to_1); CPPUNIT_TEST(testDocumentAPIClientError); CPPUNIT_TEST(testNoDocumentAPIResendingForFailedVisitor); CPPUNIT_TEST(testIteratorCreatedForFailedVisitor); @@ -90,7 +84,6 @@ public: void testNormalUsage(); void testFailedCreateIterator(); void testFailedGetIter(); - void iterators_per_bucket_config_is_ignored_and_hardcoded_to_1(); void testDocumentAPIClientError(); void testNoDocumentAPIResendingForFailedVisitor(); void testIteratorCreatedForFailedVisitor(); @@ -184,9 +177,6 @@ VisitorTest::initializeTest(const TestParams& params) vdstestlib::DirConfig config(getStandardConfig(true, "visitortest")); config.getConfig("stor-visitor").set("visitorthreads", "1"); config.getConfig("stor-visitor").set( - "iterators_per_bucket", - std::to_string(params._iteratorsPerBucket)); - config.getConfig("stor-visitor").set( "defaultparalleliterators", std::to_string(params._parallelBuckets)); config.getConfig("stor-visitor").set( @@ -596,33 +586,6 @@ VisitorTest::testFailedGetIter() CPPUNIT_ASSERT(waitUntilNoActiveVisitors()); } -void VisitorTest::iterators_per_bucket_config_is_ignored_and_hardcoded_to_1() { - initializeTest(TestParams().iteratorsPerBucket(20)); - auto cmd = makeCreateVisitor(); - _top->sendDown(cmd); - sendCreateIteratorReply(); - - auto getIterCmd = fetchSingleCommand<GetIterCommand>(*_bottom); - CPPUNIT_ASSERT_EQUAL(spi::IteratorId(1234), - getIterCmd->getIteratorId()); - sendGetIterReply(*getIterCmd); - - CPPUNIT_ASSERT_EQUAL(size_t(0), _bottom->getNumCommands()); - - std::vector<document::Document::SP> docs; - std::vector<document::DocumentId> docIds; - std::vector<std::string> infoMessages; - getMessagesAndReply(_documents.size(), getSession(0), docs, docIds, infoMessages); - CPPUNIT_ASSERT_EQUAL(size_t(0), infoMessages.size()); - CPPUNIT_ASSERT_EQUAL(size_t(0), docIds.size()); - - auto destroyIterCmd = fetchSingleCommand<DestroyIteratorCommand>(*_bottom); - - verifyCreateVisitorReply(api::ReturnCode::OK); - CPPUNIT_ASSERT(waitUntilNoActiveVisitors()); - CPPUNIT_ASSERT_EQUAL(0L, getFailedVisitorDestinationReplyCount()); -} - void VisitorTest::testDocumentAPIClientError() { @@ -709,7 +672,7 @@ VisitorTest::testNoDocumentAPIResendingForFailedVisitor() void VisitorTest::testIteratorCreatedForFailedVisitor() { - initializeTest(TestParams().iteratorsPerBucket(1).parallelBuckets(2)); + initializeTest(TestParams().parallelBuckets(2)); std::shared_ptr<api::CreateVisitorCommand> cmd( makeCreateVisitor()); cmd->addBucketToBeVisited(document::BucketId(16, 4)); @@ -928,8 +891,7 @@ void VisitorTest::testNoMoreIteratorsSentWhileMemoryUsedAboveLimit() { initializeTest(TestParams().maxVisitorMemoryUsage(1) - .parallelBuckets(1) - .iteratorsPerBucket(1)); + .parallelBuckets(1)); std::shared_ptr<api::CreateVisitorCommand> cmd( makeCreateVisitor()); _top->sendDown(cmd); diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def index fbc29e0234b..36ca88dd7de 100644 --- a/storage/src/vespa/storage/config/stor-server.def +++ b/storage/src/vespa/storage/config/stor-server.def @@ -13,11 +13,6 @@ cluster_name string default="storage" restart ## to identify the node, and to decide what data should be on it. node_index int default=0 restart -## The maximum amount of memory to use in the storage node. -## Currently default is 2 GB. -## DEPRECATED! -memorytouse long default=2147483647 restart - ## Set whether this is a distributor or a storage node. This will decide what ## storage links are set up. is_distributor bool restart diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp index 471ce7e2b27..1996ae9d2af 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp @@ -233,12 +233,7 @@ PendingClusterState::onRequestBucketInfoReply(const std::shared_ptr<api::Request const BucketSpaceAndNode bucketSpaceAndNode = iter->second; api::ReturnCode result(reply->getResult()); - if (result == api::ReturnCode::Result::ENCODE_ERROR) { - // Handle failure to encode bucket space due to use of old storage api - // protocol. Pretend that request succeeded with no buckets returned. - // TODO remove this workaround for Vespa 7 - LOG(debug, "Got ENCODE_ERROR, pretending success with no buckets"); - } else if (!result.success()) { + if (!result.success()) { framework::MilliSecTime resendTime(_clock); resendTime += framework::MilliSecTime(100); _delayedRequests.emplace_back(resendTime, bucketSpaceAndNode); diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index e7323d07480..76e04852178 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -34,8 +34,7 @@ FileStorManager(const config::ConfigUri & configUri, const spi::PartitionStateLi _partitions(partitions), _providerCore(provider), _providerErrorWrapper(_providerCore), - _providerMetric(new spi::MetricPersistenceProvider(_providerErrorWrapper)), - _provider(_providerMetric.get()), + _provider(&_providerErrorWrapper), _bucketIdFactory(_component.getBucketIdFactory()), _configUri(configUri), _disks(), @@ -47,7 +46,6 @@ FileStorManager(const config::ConfigUri & configUri, const spi::PartitionStateLi _threadMonitor(), _closed(false) { - _metrics->registerMetric(*_providerMetric), _configFetcher.subscribe(_configUri.getConfigId(), this); _configFetcher.start(); _component.registerMetric(*_metrics); diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h index 5c52e6c6a23..8c7e206237b 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h @@ -14,7 +14,6 @@ #include <vespa/vespalib/util/sync.h> #include <vespa/document/bucket/bucketid.h> #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/metricpersistenceprovider.h> #include <vespa/storage/bucketdb/storbucketdb.h> #include <vespa/storage/common/messagesender.h> #include <vespa/storage/common/servicelayercomponent.h> @@ -56,7 +55,6 @@ class FileStorManager : public StorageLinkQueued, const spi::PartitionStateList& _partitions; spi::PersistenceProvider& _providerCore; ProviderErrorWrapper _providerErrorWrapper; - spi::MetricPersistenceProvider::UP _providerMetric; spi::PersistenceProvider* _provider; const document::BucketIdFactory& _bucketIdFactory; diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index 5a1c2aed113..7fb85ef0ecc 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -259,16 +259,8 @@ void CommunicationManager::fail_with_unresolvable_bucket_space( LOG(debug, "Could not map DocumentAPI message to internal bucket: %s", error_message.c_str()); MBUS_TRACE(msg->getTrace(), 6, "Communication manager: Failing message as its document type has no known bucket space mapping"); std::unique_ptr<mbus::Reply> reply; - if (msg->getType() == documentapi::DocumentProtocol::MESSAGE_GETDOCUMENT) { - // HACK: to avoid breaking legacy routing of GetDocumentMessages to _all_ clusters - // regardless of them having a document type or not, we remap missing bucket spaces - // to explicit Not Found replies (empty document GetDocumentReply). - // TODO remove this workaround for Vespa 7 - reply = std::make_unique<documentapi::GetDocumentReply>(std::shared_ptr<document::Document>()); - } else { - reply = std::make_unique<mbus::EmptyReply>(); - reply->addError(mbus::Error(documentapi::DocumentProtocol::ERROR_REJECTED, error_message)); - } + reply = std::make_unique<mbus::EmptyReply>(); + reply->addError(mbus::Error(documentapi::DocumentProtocol::ERROR_REJECTED, error_message)); msg->swapState(*reply); _metrics.bucketSpaceMappingFailures.inc(); _messageBusSession->reply(std::move(reply)); diff --git a/storage/src/vespa/storage/visiting/stor-visitor.def b/storage/src/vespa/storage/visiting/stor-visitor.def index 6f16bcb60a2..72b3699fe2d 100644 --- a/storage/src/vespa/storage/visiting/stor-visitor.def +++ b/storage/src/vespa/storage/visiting/stor-visitor.def @@ -17,16 +17,6 @@ ignorenonexistingvisitortimelimit int default=300 restart ## 100 buckets, 8 of them will be visited in parallel. defaultparalleliterators int default=8 -## The number of iterators we send for each bucket being visited from visitor -## thread. For streaming search we would likely want two or three. Since -## supporting more than one is a new feature, default is still one. -## (If you visit 8 buckets in parallel and have 2 iterators per bucket, this -## will be 16 requests to persistence layer, but only 8 will be able to execute -## at the same time, since only one operation can be executed at the same time -## for one bucket) -## DEPRECATED: ignored by backend, 1 is always used. -iterators_per_bucket int default=1 - ## Default number of maximum client replies pending. defaultpendingmessages int default=32 |