diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2024-02-01 15:59:56 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2024-02-01 15:59:56 +0000 |
commit | aa4749a2ffb45449e457cbc79dd782526afd2ef6 (patch) | |
tree | de1e6cf5b75d9922a4ef4e0e38f665d1792997fb /storage | |
parent | 4386ce0ea433d534198d90c6c68c9d0a34c4fff0 (diff) |
GC void config from stor-bouncer.def
Diffstat (limited to 'storage')
-rw-r--r-- | storage/src/tests/storageserver/bouncertest.cpp | 76 | ||||
-rw-r--r-- | storage/src/vespa/storage/config/stor-bouncer.def | 20 | ||||
-rw-r--r-- | storage/src/vespa/storage/storageserver/bouncer.cpp | 43 | ||||
-rw-r--r-- | storage/src/vespa/storage/storageserver/bouncer.h | 5 |
4 files changed, 15 insertions, 129 deletions
diff --git a/storage/src/tests/storageserver/bouncertest.cpp b/storage/src/tests/storageserver/bouncertest.cpp index 225b3c94120..296ed6d23bc 100644 --- a/storage/src/tests/storageserver/bouncertest.cpp +++ b/storage/src/tests/storageserver/bouncertest.cpp @@ -4,6 +4,7 @@ #include <tests/common/testhelper.h> #include <tests/common/teststorageapp.h> #include <vespa/config/common/exceptions.h> +#include <memory> #include <vespa/config/helper/configgetter.hpp> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/document/fieldset/fieldsets.h> @@ -41,18 +42,6 @@ struct BouncerTest : public Test { static constexpr int RejectionDisabledConfigValue = -1; - // Note: newThreshold is intentionally int (rather than Priority) in order - // to be able to test out of bounds values. - void configureRejectionThreshold(int newThreshold); - - std::shared_ptr<api::StorageCommand> createDummyFeedMessage( - api::Timestamp timestamp, - Priority priority = 0); - - std::shared_ptr<api::StorageCommand> createDummyFeedMessage( - api::Timestamp timestamp, - document::BucketSpace bucketSpace); - void expectMessageBouncedWithRejection() const; void expect_message_bounced_with_node_down_abort() const; void expect_message_bounced_with_shutdown_abort() const; @@ -70,11 +59,11 @@ BouncerTest::BouncerTest() void BouncerTest::setUpAsNode(const lib::NodeType& type) { vdstestlib::DirConfig config(getStandardConfig(type == lib::NodeType::STORAGE)); if (type == lib::NodeType::STORAGE) { - _node.reset(new TestServiceLayerApp(NodeIndex(2), config.getConfigId())); + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(2), config.getConfigId()); } else { - _node.reset(new TestDistributorApp(NodeIndex(2), config.getConfigId())); + _node = std::make_unique<TestDistributorApp>(NodeIndex(2), config.getConfigId()); } - _upper.reset(new DummyStorageLink()); + _upper = std::make_unique<DummyStorageLink>(); using StorBouncerConfig = vespa::config::content::core::StorBouncerConfig; auto cfg_uri = config::ConfigUri(config.getConfigId()); auto cfg = config::ConfigGetter<StorBouncerConfig>::getConfig(cfg_uri.getConfigId(), cfg_uri.getContext()); @@ -104,8 +93,8 @@ BouncerTest::TearDown() { } std::shared_ptr<api::StorageCommand> -BouncerTest::createDummyFeedMessage(api::Timestamp timestamp, - api::StorageMessage::Priority priority) +createDummyFeedMessage(api::Timestamp timestamp, + api::StorageMessage::Priority priority = 0) { auto cmd = std::make_shared<api::RemoveCommand>( makeDocumentBucket(document::BucketId(0)), @@ -116,14 +105,14 @@ BouncerTest::createDummyFeedMessage(api::Timestamp timestamp, } std::shared_ptr<api::StorageCommand> -BouncerTest::createDummyFeedMessage(api::Timestamp timestamp, - document::BucketSpace bucketSpace) +createDummyFeedMessage(api::Timestamp timestamp, + document::BucketSpace bucketSpace) { auto cmd = std::make_shared<api::RemoveCommand>( document::Bucket(bucketSpace, document::BucketId(0)), document::DocumentId("id:ns:foo::bar"), timestamp); - cmd->setPriority(Priority(0)); + cmd->setPriority(BouncerTest::Priority(0)); return cmd; } @@ -226,58 +215,21 @@ BouncerTest::expectMessageNotBounced() const EXPECT_EQ(size_t(1), _lower->getNumCommands()); } -void -BouncerTest::configureRejectionThreshold(int newThreshold) -{ - using Builder = vespa::config::content::core::StorBouncerConfigBuilder; - Builder config; - config.feedRejectionPriorityThreshold = newThreshold; - _manager->on_configure(config); -} - -TEST_F(BouncerTest, reject_lower_prioritized_feed_messages_when_configured) { - configureRejectionThreshold(Priority(120)); - _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(121))); - expectMessageBouncedWithRejection(); -} - -TEST_F(BouncerTest, do_not_reject_higher_prioritized_feed_messages_than_configured) { - configureRejectionThreshold(Priority(120)); - _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(119))); - expectMessageNotBounced(); -} - -TEST_F(BouncerTest, priority_rejection_threshold_is_exclusive) { - configureRejectionThreshold(Priority(120)); - _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(120))); - expectMessageNotBounced(); -} - -TEST_F(BouncerTest, only_priority_reject_feed_messages_when_configured) { - configureRejectionThreshold(RejectionDisabledConfigValue); - // A message with even the lowest priority should not be rejected. - _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(255))); - expectMessageNotBounced(); -} - TEST_F(BouncerTest, priority_rejection_is_disabled_by_default_in_config) { _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(255))); expectMessageNotBounced(); } -TEST_F(BouncerTest, read_only_operations_are_not_priority_rejected) { - configureRejectionThreshold(Priority(1)); +TEST_F(BouncerTest, read_only_operations_are_not_rejected) { // StatBucket is an external operation, but it's not a mutating operation // and should therefore not be blocked. - auto cmd = std::make_shared<api::StatBucketCommand>( - makeDocumentBucket(document::BucketId(16, 5)), ""); + auto cmd = std::make_shared<api::StatBucketCommand>(makeDocumentBucket(document::BucketId(16, 5)), ""); cmd->setPriority(Priority(2)); _upper->sendDown(cmd); expectMessageNotBounced(); } TEST_F(BouncerTest, internal_operations_are_not_rejected) { - configureRejectionThreshold(Priority(1)); document::BucketId bucket(16, 1234); api::BucketInfo info(0x1, 0x2, 0x3); auto cmd = std::make_shared<api::NotifyBucketChangeCommand>(makeDocumentBucket(bucket), info); @@ -286,12 +238,6 @@ TEST_F(BouncerTest, internal_operations_are_not_rejected) { expectMessageNotBounced(); } -TEST_F(BouncerTest, out_of_bounds_config_values_throw_exception) { - EXPECT_THROW(configureRejectionThreshold(256), config::InvalidConfigException); - EXPECT_THROW(configureRejectionThreshold(-2), config::InvalidConfigException); -} - - namespace { std::shared_ptr<const lib::ClusterStateBundle> diff --git a/storage/src/vespa/storage/config/stor-bouncer.def b/storage/src/vespa/storage/config/stor-bouncer.def index db4471825c9..aa65c31c186 100644 --- a/storage/src/vespa/storage/config/stor-bouncer.def +++ b/storage/src/vespa/storage/config/stor-bouncer.def @@ -1,26 +1,6 @@ # Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. namespace=vespa.config.content.core -## Whether or not the bouncer should stop external load from -## entering node when the cluster state is down. -stop_external_load_when_cluster_down bool default=true - -## Sets what node states the node will allow incoming commands -## in. -stop_all_load_when_nodestate_not_in string default="uri" - ## The maximum clock skew allowed in the system. Any messages received ## that have a timestamp longer in the future than this will be failed. max_clock_skew_seconds int default=5 - -## If this config value is != -1, the node will reject any external feed -## operations with a priority lower than that specified here. Note that since -## we map priorities in such a way that 0 is the _highest_ priority and 255 the -## _lowest_ priority, for two operations A and B, if B has a lower priority -## than A it will have a higher priority _integer_ value. -## -## Only mutating external feed operations will be blocked. Read-only operations -## and internal operations are always let through. -## -## Default is -1 (i.e. rejection is disabled and load is allowed through) -feed_rejection_priority_threshold int default=-1 diff --git a/storage/src/vespa/storage/storageserver/bouncer.cpp b/storage/src/vespa/storage/storageserver/bouncer.cpp index bfc38e0c8ba..0aedee6799d 100644 --- a/storage/src/vespa/storage/storageserver/bouncer.cpp +++ b/storage/src/vespa/storage/storageserver/bouncer.cpp @@ -2,16 +2,13 @@ #include "bouncer.h" #include "bouncer_metrics.h" -#include "config_logging.h" #include <vespa/storageframework/generic/clock/clock.h> #include <vespa/vdslib/state/cluster_state_bundle.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/persistence/spi/bucket_limits.h> #include <vespa/storageapi/message/state.h> #include <vespa/storageapi/message/persistence.h> -#include <vespa/config/subscription/configuri.h> #include <vespa/config/helper/configfetcher.hpp> -#include <vespa/config/common/exceptions.h> #include <vespa/vespalib/util/stringfmt.h> #include <sstream> @@ -62,7 +59,6 @@ Bouncer::onClose() void Bouncer::on_configure(const vespa::config::content::core::StorBouncerConfig& config) { - validateConfig(config); auto new_config = std::make_unique<StorBouncerConfig>(config); std::lock_guard lock(_lock); _config = std::move(new_config); @@ -72,27 +68,6 @@ const BouncerMetrics& Bouncer::metrics() const noexcept { return *_metrics; } -void -Bouncer::validateConfig(const vespa::config::content::core::StorBouncerConfig& newConfig) const -{ - if (newConfig.feedRejectionPriorityThreshold != -1) { - if (newConfig.feedRejectionPriorityThreshold - > std::numeric_limits<api::StorageMessage::Priority>::max()) - { - throw config::InvalidConfigException( - "feed_rejection_priority_threshold config value exceeds " - "maximum allowed value"); - } - if (newConfig.feedRejectionPriorityThreshold - < std::numeric_limits<api::StorageMessage::Priority>::min()) - { - throw config::InvalidConfigException( - "feed_rejection_priority_threshold config value lower than " - "minimum allowed value"); - } - } -} - void Bouncer::append_node_identity(std::ostream& target_stream) const { target_stream << " (on " << _component.getNodeType() << '.' << _component.getIndex() << ")"; } @@ -101,8 +76,7 @@ void Bouncer::abortCommandForUnavailableNode(api::StorageMessage& msg, const lib::State& state) { // If we're not up or retired, fail due to this nodes state. - std::shared_ptr<api::StorageReply> reply( - static_cast<api::StorageCommand&>(msg).makeReply()); + std::shared_ptr<api::StorageReply> reply(static_cast<api::StorageCommand&>(msg).makeReply()); std::ostringstream ost; ost << "We don't allow command of type " << msg.getType() << " when node is in state " << state.toString(true); @@ -235,18 +209,14 @@ Bouncer::onDown(const std::shared_ptr<api::StorageMessage>& msg) const lib::State* state; int maxClockSkewInSeconds; bool isInAvailableState; - bool abortLoadWhenClusterDown; bool closed; const lib::State* cluster_state; - int feedPriorityLowerBound; { std::lock_guard lock(_lock); state = &getDerivedNodeState(msg->getBucket().getBucketSpace()).getState(); maxClockSkewInSeconds = _config->maxClockSkewSeconds; - abortLoadWhenClusterDown = _config->stopExternalLoadWhenClusterDown; cluster_state = _clusterState; - isInAvailableState = state->oneOf(_config->stopAllLoadWhenNodestateNotIn.c_str()); - feedPriorityLowerBound = _config->feedRejectionPriorityThreshold; + isInAvailableState = state->oneOf("uri"); closed = _closed; } const api::MessageType& type = msg->getType(); @@ -292,13 +262,6 @@ Bouncer::onDown(const std::shared_ptr<api::StorageMessage>& msg) if (!externalLoad) { return false; } - if (priorityRejectionIsEnabled(feedPriorityLowerBound) - && isExternalWriteOperation(type) - && (msg->getPriority() > feedPriorityLowerBound)) - { - rejectDueToInsufficientPriority(*msg, feedPriorityLowerBound); - return true; - } uint64_t timestamp = extractMutationTimestampIfAny(*msg); if (timestamp != 0) { @@ -311,7 +274,7 @@ Bouncer::onDown(const std::shared_ptr<api::StorageMessage>& msg) } // If cluster state is not up, fail external load - if (abortLoadWhenClusterDown && !clusterIsUp(*cluster_state)) { + if (!clusterIsUp(*cluster_state)) { abortCommandDueToClusterDown(*msg, *cluster_state); return true; } diff --git a/storage/src/vespa/storage/storageserver/bouncer.h b/storage/src/vespa/storage/storageserver/bouncer.h index 26282625269..51620a49bda 100644 --- a/storage/src/vespa/storage/storageserver/bouncer.h +++ b/storage/src/vespa/storage/storageserver/bouncer.h @@ -49,7 +49,6 @@ public: const BouncerMetrics& metrics() const noexcept; private: - void validateConfig(const vespa::config::content::core::StorBouncerConfig&) const; void onClose() override; void abortCommandForUnavailableNode(api::StorageMessage&, const lib::State&); void rejectCommandWithTooHighClockSkew(api::StorageMessage& msg, int maxClockSkewInSeconds); @@ -61,9 +60,7 @@ private: bool isDistributor() const; static bool isExternalLoad(const api::MessageType&) noexcept; static bool isExternalWriteOperation(const api::MessageType&) noexcept; - static bool priorityRejectionIsEnabled(int configuredPriority) noexcept { - return (configuredPriority != -1); - } + /** * If msg is a command containing a mutating timestamp (put, remove or |