diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-06 15:15:37 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-06 15:15:37 +0100 |
commit | 2778d49e9d4804d08b01407884520ffe00b94a95 (patch) | |
tree | aee2154e43841b3c4f44031c5cd94de4f5cffc4f /searchcore | |
parent | e1977829a79d9c7598fc99627861c6a822d16318 (diff) | |
parent | b1ed9bb59141ad556f376a2cf0c27f982d3b7f79 (diff) |
Merge pull request #15921 from vespa-engine/balder/let-updates-that-does-not-add-2-corpus-through
Allow updates with only remove/clear updates and operations to singleā¦
Diffstat (limited to 'searchcore')
5 files changed, 182 insertions, 10 deletions
diff --git a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp index d06da31b415..d466e5a00d2 100644 --- a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp @@ -5,6 +5,17 @@ #include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/update/documentupdate.h> +#include <vespa/document/update/clearvalueupdate.h> +#include <vespa/document/update/removevalueupdate.h> +#include <vespa/document/update/addvalueupdate.h> +#include <vespa/document/update/tensor_remove_update.h> +#include <vespa/document/update/tensor_modify_update.h> +#include <vespa/document/update/tensor_add_update.h> +#include <vespa/document/update/tensor_partial_update.h> +#include <vespa/document/update/arithmeticvalueupdate.h> +#include <vespa/document/update/mapvalueupdate.h> +#include <vespa/document/update/removefieldpathupdate.h> +#include <vespa/document/fieldvalue/referencefieldvalue.h> #include <vespa/eval/eval/simple_value.h> #include <vespa/eval/eval/tensor_spec.h> #include <vespa/eval/eval/value.h> @@ -18,6 +29,7 @@ #include <vespa/searchcore/proton/feedoperation/updateoperation.h> #include <vespa/searchcore/proton/persistenceengine/i_resource_write_filter.h> #include <vespa/searchcore/proton/server/configstore.h> +#include <vespa/searchcore/proton/server/feed_reject_helper.h> #include <vespa/searchcore/proton/server/ddbstate.h> #include <vespa/searchcore/proton/server/executorthreadingservice.h> #include <vespa/searchcore/proton/server/feedhandler.h> @@ -278,7 +290,7 @@ struct SchemaContext { }; SchemaContext::SchemaContext() - : schema(new Schema()), + : schema(std::make_shared<Schema>()), builder() { schema->addAttributeField(Schema::AttributeField("tensor", DataType::TENSOR, CollectionType::SINGLE, "tensor(x{},y{})")); @@ -320,7 +332,7 @@ struct UpdateContext { DocumentUpdate::SP update; BucketId bucketId; UpdateContext(const vespalib::string &docId, DocBuilder &builder) : - update(new DocumentUpdate(*builder.getDocumentTypeRepo(), builder.getDocumentType(), DocumentId(docId))), + update(std::make_shared<DocumentUpdate>(*builder.getDocumentTypeRepo(), builder.getDocumentType(), DocumentId(docId))), bucketId(BucketFactory::getBucketId(update->getId())) { } @@ -683,6 +695,7 @@ TEST_F("require that update is rejected if resource limit is reached", FeedHandl f.writeFilter._message = "Attribute resource limit reached"; UpdateContext updCtx("id:test:searchdocument::foo", *f.schema.builder); + updCtx.addFieldUpdate("tensor"); auto op = std::make_unique<UpdateOperation>(updCtx.bucketId, Timestamp(10), updCtx.update); FeedTokenContext token; f.handler.performOperation(std::move(token.token), std::move(op)); @@ -806,6 +819,63 @@ TEST_F("require that put with different document type repo is ok", FeedHandlerFi EXPECT_EQUAL(1, f.tls_writer.store_count); } +TEST("require that fixed size field values are detected") { + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::BoolFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::ByteFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::ShortFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::IntFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::LongFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::FloatFieldValue())); + EXPECT_TRUE(FeedRejectHelper::isFixedSizeSingleValue(document::DoubleFieldValue())); + + EXPECT_FALSE(FeedRejectHelper::isFixedSizeSingleValue(document::StringFieldValue())); + EXPECT_FALSE(FeedRejectHelper::isFixedSizeSingleValue(document::RawFieldValue())); + EXPECT_FALSE(FeedRejectHelper::isFixedSizeSingleValue(document::PredicateFieldValue())); + EXPECT_FALSE(FeedRejectHelper::isFixedSizeSingleValue(document::ReferenceFieldValue())); + + document::ArrayDataType intArrayType(*document::DataType::INT); + EXPECT_FALSE(FeedRejectHelper::isFixedSizeSingleValue(document::ArrayFieldValue(intArrayType))); +} + +using namespace document; + +TEST("require that clear, remove, tensor_remove and artithmetic updates ignore feed rejection") { + EXPECT_FALSE(FeedRejectHelper::mustReject(ClearValueUpdate())); + EXPECT_FALSE(FeedRejectHelper::mustReject(RemoveValueUpdate(StringFieldValue()))); + EXPECT_FALSE(FeedRejectHelper::mustReject(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 5.0))); + EXPECT_FALSE(FeedRejectHelper::mustReject(TensorRemoveUpdate(std::make_unique<TensorFieldValue>()))); +} + +TEST("require that add, map, tensor_modify and tensor_add updates will be rejected") { + EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(IntFieldValue()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(IntFieldValue(), ClearValueUpdate()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, + std::make_unique<TensorFieldValue>()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(TensorAddUpdate(std::make_unique<TensorFieldValue>()))); +} +TEST("require that assign updates will be rejected based on their content") { + EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(IntFieldValue()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(StringFieldValue()))); +} + +TEST_F("require that update with a fieldpath update will be rejected", SchemaContext) { + const DocumentType *docType = f.getRepo()->getDocumentType(f.getDocType().getName()); + auto docUpdate = std::make_unique<DocumentUpdate>(*f.getRepo(), *docType, DocumentId("id:ns:" + docType->getName() + "::1")); + docUpdate->addFieldPathUpdate(FieldPathUpdate::CP(std::make_unique<RemoveFieldPathUpdate>())); + EXPECT_TRUE(FeedRejectHelper::mustReject(*docUpdate)); +} + +TEST_F("require that all value updates will be inspected before rejected", SchemaContext) { + const DocumentType *docType = f.getRepo()->getDocumentType(f.getDocType().getName()); + auto docUpdate = std::make_unique<DocumentUpdate>(*f.getRepo(), *docType, DocumentId("id:ns:" + docType->getName() + "::1")); + docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(ClearValueUpdate())); + EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); + docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(ClearValueUpdate())); + EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); + docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(AssignValueUpdate(StringFieldValue()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(*docUpdate)); +} + } // namespace TEST_MAIN() diff --git a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt index 73b7404ce31..85e01f9a8ed 100644 --- a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt @@ -46,6 +46,7 @@ vespa_add_library(searchcore_server STATIC feedhandler.cpp feedstate.cpp feedstates.cpp + feed_reject_helper.cpp fileconfigmanager.cpp flushhandlerproxy.cpp forcecommitcontext.cpp diff --git a/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.cpp b/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.cpp new file mode 100644 index 00000000000..43cc975acc2 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.cpp @@ -0,0 +1,75 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "feed_reject_helper.h" +#include <vespa/searchcore/proton/feedoperation/operations.h> +#include <vespa/document/update/documentupdate.h> +#include <vespa/document/update/assignvalueupdate.h> +#include <vespa/document/fieldvalue/boolfieldvalue.h> +#include <vespa/document/fieldvalue/numericfieldvalue.h> + +using namespace document; + +namespace proton { + +bool +FeedRejectHelper::isFixedSizeSingleValue(const document::FieldValue & fv) { + return fv.inherits(BoolFieldValue::classId) || fv.inherits(NumericFieldValueBase::classId); +} + +bool +FeedRejectHelper::mustReject(const document::ValueUpdate & valueUpdate) { + using namespace document; + switch (valueUpdate.getType()) { + case ValueUpdate::Add: + case ValueUpdate::TensorAddUpdate: + case ValueUpdate::TensorModifyUpdate: + case ValueUpdate::Map: + return true; + case ValueUpdate::Assign: { + const auto & assign = dynamic_cast<const AssignValueUpdate &>(valueUpdate); + if (assign.hasValue()) { + if ( ! isFixedSizeSingleValue(assign.getValue())) { + return true; + } + } + } + default: + break; + } + return false; +} + +bool +FeedRejectHelper::mustReject(const DocumentUpdate & documentUpdate) { + for (const auto & update : documentUpdate.getUpdates()) { + for (const auto & valueUpdate : update.getUpdates()) { + if (mustReject(*valueUpdate)) { + return true; + } + } + } + return ! documentUpdate.getFieldPathUpdates().empty(); +} + +bool +FeedRejectHelper::mustReject(const UpdateOperation & updateOperation) { + using namespace document; + if (updateOperation.getUpdate()) { + return mustReject(*updateOperation.getUpdate()); + } + return false; +} + +bool +FeedRejectHelper::isRejectableFeedOperation(const FeedOperation & op) +{ + FeedOperation::Type type = op.getType(); + if (type == FeedOperation::PUT) { + return true; + } else if (type == FeedOperation::UPDATE_42 || type == FeedOperation::UPDATE) { + return mustReject(dynamic_cast<const UpdateOperation &>(op)); + } + return false; +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.h b/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.h new file mode 100644 index 00000000000..00a8b3d77ad --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/server/feed_reject_helper.h @@ -0,0 +1,30 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +namespace document { + class FieldValue; + class DocumentUpdate; + class ValueUpdate; +} + +namespace proton { + +class FeedOperation; +class UpdateOperation; + +/** + * Tells wether an operation should be blocked when resourcelimits have been reached. + * It looks at the operation type and also the content if it is an 'update' operation. + */ +class FeedRejectHelper { +public: + static bool isRejectableFeedOperation(const FeedOperation & op); + // Public only for testing + static bool isFixedSizeSingleValue(const document::FieldValue & fv); + static bool mustReject(const document::ValueUpdate & valueUpdate); + static bool mustReject(const document::DocumentUpdate & documentUpdate); + static bool mustReject(const UpdateOperation & updateOperation); +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp index 441b1745616..132fdf56b90 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp @@ -6,6 +6,7 @@ #include "i_feed_handler_owner.h" #include "ifeedview.h" #include "configstore.h" +#include "feed_reject_helper.h" #include <vespa/document/base/exceptions.h> #include <vespa/document/datatype/documenttype.h> #include <vespa/document/repo/documenttyperepo.h> @@ -583,14 +584,9 @@ FeedHandler::tlsPrune(SerialNum oldest_to_keep) { namespace { -bool -isRejectableFeedOperation(FeedOperation::Type type) -{ - return type == FeedOperation::PUT || type == FeedOperation::UPDATE_42 || type == FeedOperation::UPDATE; -} - template <typename ResultType> -void feedOperationRejected(FeedToken & token, const vespalib::string &opType, const vespalib::string &docId, +void +feedOperationRejected(FeedToken & token, const vespalib::string &opType, const vespalib::string &docId, const DocTypeName & docTypeName, const vespalib::string &rejectMessage) { if (token) { @@ -621,7 +617,7 @@ notifyFeedOperationRejected(FeedToken & token, const FeedOperation &op, bool FeedHandler::considerWriteOperationForRejection(FeedToken & token, const FeedOperation &op) { - if (!_writeFilter.acceptWriteOperation() && isRejectableFeedOperation(op.getType())) { + if (!_writeFilter.acceptWriteOperation() && FeedRejectHelper::isRejectableFeedOperation(op)) { IResourceWriteFilter::State state = _writeFilter.getAcceptState(); if (!state.acceptWriteOperation()) { notifyFeedOperationRejected(token, op, _docTypeName, state.message()); |