From 7840e780563cafa361e4e3fd31681efe73f8a9f4 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 9 Nov 2020 12:28:43 +0100 Subject: Revert "Revert "Use a hash_set to quickly check if a field is an index field."" Handle structs too. --- .../proton/documentdb/feedview/feedview_test.cpp | 2 +- .../proton/attribute/attribute_writer.cpp | 13 ++-- .../proton/attribute/ifieldupdatecallback.h | 7 +- .../proton/server/searchable_feed_view.cpp | 2 +- .../searchcore/proton/server/storeonlyfeedview.cpp | 83 ++++++++++++++-------- .../searchcore/proton/server/storeonlyfeedview.h | 24 ++----- 6 files changed, 70 insertions(+), 61 deletions(-) diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index b326db47b5c..641e9879c79 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -362,7 +362,7 @@ struct MyAttributeWriter : public IAttributeWriter _updateLid = lid; for (const auto & fieldUpdate : upd.getUpdates()) { search::AttributeVector * attr = getWritableAttribute(fieldUpdate.getField().getName()); - onUpdate.onUpdateField(fieldUpdate.getField().getName(), attr); + onUpdate.onUpdateField(fieldUpdate.getField(), attr); } } void update(SerialNum serialNum, const document::Document &doc, DocumentIdT lid, OnWriteDoneType) override { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index bb25b3da7be..021fc4717af 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -764,9 +764,9 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document for (const auto &fupd : upd.getUpdates()) { LOG(debug, "Retrieving guard for attribute vector '%s'.", fupd.getField().getName().data()); - auto itr = _attrMap.find(fupd.getField().getName()); - AttributeVector * attrp = (itr != _attrMap.end()) ? itr->second.attribute : nullptr; - onUpdate.onUpdateField(fupd.getField().getName(), attrp); + auto found = _attrMap.find(fupd.getField().getName()); + AttributeVector * attrp = (found != _attrMap.end()) ? found->second.attribute : nullptr; + onUpdate.onUpdateField(fupd.getField(), attrp); if (__builtin_expect(attrp == nullptr, false)) { LOG(spam, "Failed to find attribute vector %s", fupd.getField().getName().data()); continue; @@ -776,16 +776,15 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document if (__builtin_expect(attrp->getStatus().getLastSyncToken() >= serialNum, false)) { continue; } - if (itr->second.use_two_phase_put_for_assign_updates && - is_single_assign_update(fupd)) { + if (found->second.use_two_phase_put_for_assign_updates && is_single_assign_update(fupd)) { auto prepare_task = std::make_unique(serialNum, lid, *attrp, get_single_assign_update_field_value(fupd)); auto complete_task = std::make_unique(*prepare_task, onWriteDone); LOG(debug, "About to handle assign update as two phase put for docid %u in attribute vector '%s'", lid, attrp->getName().c_str()); _shared_executor.execute(CpuUsage::wrap(std::move(prepare_task), CpuUsage::Category::WRITE)); - _attributeFieldWriter.executeTask(itr->second.executor_id, std::move(complete_task)); + _attributeFieldWriter.executeTask(found->second.executor_id, std::move(complete_task)); } else { - args[itr->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); + args[found->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); LOG(debug, "About to apply update for docId %u in attribute vector '%s'.", lid, attrp->getName().c_str()); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h index d8872607b44..d3ab970fb39 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h @@ -5,16 +5,17 @@ #include namespace search { class AttributeVector; } +namespace document { class Field; } namespace proton { struct IFieldUpdateCallback { - virtual ~IFieldUpdateCallback() { } - virtual void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) = 0; + virtual ~IFieldUpdateCallback() = default; + virtual void onUpdateField(const document::Field & field, const search::AttributeVector * attr) = 0; }; struct DummyFieldUpdateCallback : IFieldUpdateCallback { - void onUpdateField(vespalib::stringref, const search::AttributeVector *) override {} + void onUpdateField(const document::Field & , const search::AttributeVector *) override {} }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp index 207f1d813d8..7a78b4ba82a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp @@ -34,7 +34,7 @@ SearchableFeedView::SearchableFeedView(StoreOnlyFeedView::Context storeOnlyCtx, const FastAccessFeedView::Context &fastUpdateCtx, Context ctx) : Parent(std::move(storeOnlyCtx), params, fastUpdateCtx), _indexWriter(ctx._indexWriter), - _hasIndexedFields(_schema->getNumIndexFields() > 0) + _hasIndexedFields(getSchema()->getNumIndexFields() > 0) { } SearchableFeedView::~SearchableFeedView() = default; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index a537742b79b..d7c3246cb4b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -7,14 +7,14 @@ #include "putdonecontext.h" #include "removedonecontext.h" #include "updatedonecontext.h" -#include -#include -#include #include #include #include #include #include +#include +#include +#include #include #include #include @@ -88,8 +88,8 @@ SummaryPutDoneContext::SummaryPutDoneContext(FeedToken token, IPendingLidTracker SummaryPutDoneContext::~SummaryPutDoneContext() = default; -std::vector getGidsToRemove(const IDocumentMetaStore &metaStore, - const LidVectorContext::LidVector &lidsToRemove) +std::vector +getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::LidVector &lidsToRemove) { std::vector gids; gids.reserve(lidsToRemove.size()); @@ -102,8 +102,9 @@ std::vector getGidsToRemove(const IDocumentMetaStore &metaSt return gids; } -void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, + const DocumentOperation &op, bool is_removed_doc) { documentmetastore::IStore::Result putRes( meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(), @@ -117,8 +118,9 @@ void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_i assert(op.getLid() == putRes._lid); } -void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, + const DocumentOperation &op, bool is_removed_doc) { assert(meta_store.validLid(op.getPrevLid())); assert(is_removed_doc == op.getPrevMarkedAsRemoved()); @@ -147,6 +149,37 @@ moveMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, c meta_store.move(op.getPrevLid(), op.getLid(), op.get_prepare_serial_num()); } +class UpdateScope final : public IFieldUpdateCallback +{ +private: + const vespalib::hash_set & _indexedFields; + bool _nonAttributeFields; +public: + bool _hasIndexedFields; + + UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd); + bool hasIndexOrNonAttributeFields() const { + return _hasIndexedFields || _nonAttributeFields; + } + void onUpdateField(const document::Field & field, const search::AttributeVector * attr) override; +}; + +UpdateScope::UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd) + : _indexedFields(indexedFields), + _nonAttributeFields(!upd.getFieldPathUpdates().empty()), + _hasIndexedFields(false) +{} + +void +UpdateScope::onUpdateField(const document::Field & field, const search::AttributeVector * attr) { + if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { + _nonAttributeFields = true; + } + if (!_hasIndexedFields && (_indexedFields.find(field.getId()) != _indexedFields.end())) { + _hasIndexedFields = true; + } +} + } // namespace StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms) @@ -160,12 +193,20 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms _pendingLidsForDocStore(), _pendingLidsForCommit(std::move(ctx._pendingLidsForCommit)), _schema(std::move(ctx._schema)), + _indexedFields(), _writeService(ctx._writeService), _params(params), _metaStore(_documentMetaStoreContext->get()), _gidToLidChangeHandler(ctx._gidToLidChangeHandler) { _docType = _repo->getDocumentType(_params._docTypeName.getName()); + if (_schema && _docType) { + for (const auto &indexField : _schema->getIndexFields()) { + document::FieldPath fieldPath; + _docType->buildFieldPath(fieldPath, indexField.getName()); + _indexedFields.insert(fieldPath.back().getFieldRef().getId()); + } + } } StoreOnlyFeedView::~StoreOnlyFeedView() = default; @@ -207,7 +248,7 @@ void StoreOnlyFeedView::putAttributes(SerialNum, Lid, const Document &, OnPutDoneType) {} void -StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const Document::SP &, OnOperationDoneType) {} +StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const std::shared_ptr &, OnOperationDoneType) {} void StoreOnlyFeedView::preparePut(PutOperation &putOp) @@ -285,7 +326,7 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, const DocumentUpdate & upd, OnOperationDoneType, IFieldUpdateCallback & onUpdate) { for (const auto & fieldUpdate : upd.getUpdates()) { - onUpdate.onUpdateField(fieldUpdate.getField().getName(), nullptr); + onUpdate.onUpdateField(fieldUpdate.getField(), nullptr); } } @@ -387,22 +428,6 @@ StoreOnlyFeedView::heartBeatSummary(SerialNum serialNum, DoneCallback onDone) { })); } -StoreOnlyFeedView::UpdateScope::UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd) - : _schema(&schema), - _indexedFields(false), - _nonAttributeFields(!upd.getFieldPathUpdates().empty()) -{} - -void -StoreOnlyFeedView::UpdateScope::onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) { - if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { - _nonAttributeFields = true; - } - if (!_indexedFields && _schema->isIndexField(fieldName)) { - _indexedFields = true; - } -} - void StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) { if ( ! updOp.getUpdate()) { @@ -432,7 +457,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) } auto onWriteDone = createUpdateDoneContext(std::move(token), get_pending_lid_token(updOp), updOp.getUpdate()); - UpdateScope updateScope(*_schema, upd); + UpdateScope updateScope(_indexedFields, upd); updateAttributes(serialNum, lid, upd, onWriteDone, updateScope); if (updateScope.hasIndexOrNonAttributeFields()) { @@ -440,7 +465,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) FutureDoc futureDoc = promisedDoc.get_future().share(); onWriteDone->setDocument(futureDoc); _pendingLidsForDocStore.waitComplete(lid); - if (updateScope._indexedFields) { + if (updateScope._hasIndexedFields) { updateIndexedFields(serialNum, lid, futureDoc, onWriteDone); } PromisedStream promisedStream; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index 2822aa70525..25a98da7ce7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -9,7 +9,6 @@ #include "searchcontext.h" #include #include -#include #include #include #include @@ -18,9 +17,10 @@ #include #include #include +#include #include +#include #include -#include namespace vespalib { class IDestructorCallback; } @@ -118,22 +118,6 @@ public: {} }; -protected: - class UpdateScope : public IFieldUpdateCallback - { - private: - const search::index::Schema *_schema; - public: - bool _indexedFields; - bool _nonAttributeFields; - - UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd); - bool hasIndexOrNonAttributeFields() const { - return _indexedFields || _nonAttributeFields; - } - void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) override; - }; - private: const ISummaryAdapter::SP _summaryAdapter; const IDocumentMetaStoreContext::SP _documentMetaStoreContext; @@ -142,9 +126,9 @@ private: LidReuseDelayer _lidReuseDelayer; PendingLidTracker _pendingLidsForDocStore; std::shared_ptr _pendingLidsForCommit; - + const search::index::Schema::SP _schema; + vespalib::hash_set _indexedFields; protected: - const search::index::Schema::SP _schema; searchcorespi::index::IThreadingService &_writeService; PersistentParams _params; IDocumentMetaStore &_metaStore; -- cgit v1.2.3 From c68ea1b2a5d2e5e34302b1b9ca0616c1e826a33c Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Sun, 17 Jul 2022 21:33:34 +0000 Subject: Fetch list of buckets and iterate over bucket ids instead of using iterator directly. --- .../src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp | 7 ++++--- searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h | 1 - searchcore/src/vespa/searchcore/proton/server/documentdb.cpp | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp index b68892be60f..9d28d88ba4a 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp @@ -33,10 +33,11 @@ checksumToString(storage::spi::BucketChecksum checksum) void convertBucketsToSlime(const BucketDB &bucketDb, Cursor &array) { - for (auto itr = bucketDb.begin(); itr != bucketDb.end(); ++itr) { + BucketId::List buckets = bucketDb.getBuckets(); + for (BucketId bucketId : buckets) { Cursor &object = array.addObject(); - object.setString("id", bucketIdToString(itr->first)); - const bucketdb::BucketState &state = itr->second; + object.setString("id", bucketIdToString(bucketId)); + bucketdb::BucketState state = bucketDb.get(bucketId); object.setString("checksum", checksumToString(state.getChecksum())); object.setLong("readyCount", state.getReadyCount()); object.setLong("notReadyCount", state.getNotReadyCount()); diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h index da475f2969a..955b6c8062c 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h @@ -64,7 +64,6 @@ public: BucketId::List getActiveBuckets() const; BucketId::List populateActiveBuckets(BucketId::List buckets); - ConstMapIterator begin() const { return _map.begin(); } ConstMapIterator end() const { return _map.end(); } ConstMapIterator lowerBound(const BucketId &bucket) const { return _map.lower_bound(bucket); } size_t size() const { return _map.size(); } diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 3fdf82ea61d..783282b71c1 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -1025,11 +1025,12 @@ notifyBucketsChanged(const documentmetastore::IBucketHandler &metaStore, IBucketModifiedHandler &handler, const vespalib::string &name) { - bucketdb::Guard buckets = metaStore.getBucketDB().takeGuard(); - for (const auto &kv : *buckets) { - handler.notifyBucketModified(kv.first); + bucketdb::Guard guard = metaStore.getBucketDB().takeGuard(); + document::BucketId::List buckets = guard->getBuckets(); + for (document::BucketId bucketId : buckets) { + handler.notifyBucketModified(bucketId); } - LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets->size()); + LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets.size()); } } -- cgit v1.2.3 From 8f74172202904bac49d7e2aba57529ccc32273e6 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 18 Jul 2022 12:37:02 +0000 Subject: Do not expose iterate api. --- .../documentdb/documentbucketmover/CMakeLists.txt | 12 -- .../documentbucketmover/scaniterator_test.cpp | 185 --------------------- .../searchcore/proton/bucketdb/CMakeLists.txt | 1 - .../vespa/searchcore/proton/bucketdb/bucketdb.cpp | 36 ++-- .../vespa/searchcore/proton/bucketdb/bucketdb.h | 38 ++--- .../proton/bucketdb/bucketscaniterator.cpp | 15 -- .../proton/bucketdb/bucketscaniterator.h | 38 ----- .../searchcore/proton/server/bucketmovejob.cpp | 17 +- .../vespa/searchcore/proton/server/bucketmovejob.h | 16 +- 9 files changed, 57 insertions(+), 301 deletions(-) delete mode 100644 searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp delete mode 100644 searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp delete mode 100644 searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt index 6ca815cd216..5435e9eb93e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt @@ -17,18 +17,6 @@ vespa_add_executable(searchcore_documentbucketmover_test_app TEST ) vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app) -vespa_add_executable(searchcore_scaniterator_test_app TEST - SOURCES - scaniterator_test.cpp - DEPENDS - searchcore_bucketmover_test - searchcore_server - searchcore_test - searchcore_feedoperation - GTest::GTest -) -vespa_add_test(NAME searchcore_scaniterator_test_app COMMAND searchcore_scaniterator_test_app) - vespa_add_executable(searchcore_documentmover_test_app TEST SOURCES documentmover_test.cpp diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp deleted file mode 100644 index 4ec38ff7170..00000000000 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp +++ /dev/null @@ -1,185 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "bucketmover_common.h" -#include -#include - -#include -LOG_SETUP("document_bucket_mover_test"); - -using namespace proton; -using namespace proton::move::test; -using document::BucketId; - -using ScanItr = bucketdb::ScanIterator; - -struct ScanTestBase : public ::testing::Test -{ - test::UserDocumentsBuilder _builder; - std::shared_ptr _bucketDB; - MySubDb _ready; - MySubDb _notReady; - ScanTestBase(); - ~ScanTestBase(); - - ScanItr getItr() { - return getItr(BucketId()); - } - - ScanItr getItr(BucketId bucket) { - return ScanItr(_bucketDB->takeGuard(), bucket); - } -}; - -ScanTestBase::ScanTestBase() - : _builder(), - _bucketDB(std::make_shared()), - _ready(_builder.getRepo(), _bucketDB, 1, SubDbType::READY), - _notReady(_builder.getRepo(), _bucketDB, 2, SubDbType::NOTREADY) -{} -ScanTestBase::~ScanTestBase() = default; - -struct ScanTest : public ScanTestBase -{ - ScanTest() : ScanTestBase() - { - _builder.createDocs(6, 1, 2); - _builder.createDocs(8, 2, 3); - _ready.insertDocs(_builder.getDocs()); - _builder.clearDocs(); - _builder.createDocs(2, 1, 2); - _builder.createDocs(4, 2, 3); - _notReady.insertDocs(_builder.getDocs()); - _builder.clearDocs(); - } -}; - -struct OnlyNotReadyScanTest : public ScanTestBase -{ - OnlyNotReadyScanTest() : ScanTestBase() - { - _builder.createDocs(2, 1, 2); - _builder.createDocs(4, 2, 3); - _notReady.insertDocs(_builder.getDocs()); - } -}; - -struct OnlyReadyScanTest : public ScanTestBase -{ - OnlyReadyScanTest() : ScanTestBase() - { - _builder.createDocs(6, 1, 2); - _builder.createDocs(8, 2, 3); - _ready.insertDocs(_builder.getDocs()); - } -}; - -struct BucketVector : public BucketId::List -{ - BucketVector() : BucketId::List() {} - BucketVector &add(const BucketId &bucket) { - push_back(bucket); - return *this; - } -}; - -void -advanceToFirstBucketWithDocs(ScanItr &itr, SubDbType subDbType) -{ - while (itr.valid()) { - if (subDbType == SubDbType::READY) { - if (itr.hasReadyBucketDocs()) - return; - } else { - if (itr.hasNotReadyBucketDocs()) - return; - } - ++itr; - } -} - -void -assertEquals(const BucketVector &exp, ScanItr &itr, SubDbType subDbType) -{ - for (size_t i = 0; i < exp.size(); ++i) { - advanceToFirstBucketWithDocs(itr, subDbType); - EXPECT_TRUE(itr.valid()); - EXPECT_EQ(exp[i], itr.getBucket()); - ++itr; - } - advanceToFirstBucketWithDocs(itr, subDbType); - EXPECT_FALSE(itr.valid()); -} - -TEST_F(ScanTest, require_that_we_can_iterate_all_buckets_from_start_to_end) -{ - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_not_ready_buckets) -{ - BucketId bucket = _notReady.bucket(4); - { - ScanItr itr = getItr(bucket); - assertEquals(BucketVector(). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_ready_buckets) -{ - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - BucketId bucket = _ready.bucket(6); - ScanItr itr = getItr(bucket); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(OnlyNotReadyScanTest, require_that_we_can_iterate_only_not_ready_buckets) -{ - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); -} - -TEST_F(OnlyReadyScanTest, require_that_we_can_iterate_only_ready_buckets) -{ - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); -} - -TEST_F(ScanTestBase, require_that_we_can_iterate_zero_buckets) -{ - ScanItr itr = getItr(); - EXPECT_FALSE(itr.valid()); -} - -GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt index 810fe53236d..a542f44f27e 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt @@ -6,7 +6,6 @@ vespa_add_library(searchcore_bucketdb STATIC bucket_db_owner.cpp bucketdb.cpp bucketdbhandler.cpp - bucketscaniterator.cpp bucketsessionbase.cpp bucketstate.cpp checksumaggregators.cpp diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp index ecdc679e4bc..5ee6fb3973f 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp @@ -27,12 +27,12 @@ BucketDB::~BucketDB() } void -BucketDB::add(const BucketId &bucketId, const BucketState & state) { +BucketDB::add(BucketId bucketId, const BucketState & state) { _map[bucketId] += state; } bucketdb::BucketState * -BucketDB::getBucketStatePtr(const BucketId &bucket) +BucketDB::getBucketStatePtr(BucketId bucket) { auto it(_map.find(bucket)); if (it != _map.end()) { @@ -42,7 +42,7 @@ BucketDB::getBucketStatePtr(const BucketId &bucket) } void -BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta) +BucketDB::unloadBucket(BucketId bucket, const BucketState &delta) { BucketState *state = getBucketStatePtr(bucket); assert(state); @@ -51,7 +51,7 @@ BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta) const bucketdb::BucketState & BucketDB::add(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType) { BucketState &state = _map[bucketId]; @@ -61,7 +61,7 @@ BucketDB::add(const GlobalId &gid, void BucketDB::remove(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType) { BucketState &state = _map[bucketId]; @@ -84,8 +84,8 @@ BucketDB::remove_batch(const std::vector &removed, SubDbType s void BucketDB::modify(const GlobalId &gid, - const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize, - const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize, + BucketId oldBucketId, Timestamp oldTimestamp, uint32_t oldDocSize, + BucketId newBucketId, Timestamp newTimestamp, uint32_t newDocSize, SubDbType subDbType) { if (oldBucketId == newBucketId) { @@ -99,7 +99,7 @@ BucketDB::modify(const GlobalId &gid, bucketdb::BucketState -BucketDB::get(const BucketId &bucketId) const +BucketDB::get(BucketId bucketId) const { auto itr = _map.find(bucketId); if (itr != _map.end()) { @@ -109,7 +109,7 @@ BucketDB::get(const BucketId &bucketId) const } void -BucketDB::cacheBucket(const BucketId &bucketId) +BucketDB::cacheBucket(BucketId bucketId) { _cachedBucketId = bucketId; _cachedBucketState = get(bucketId); @@ -123,13 +123,13 @@ BucketDB::uncacheBucket() } bool -BucketDB::isCachedBucket(const BucketId &bucketId) const +BucketDB::isCachedBucket(BucketId bucketId) const { return _cachedBucketId == bucketId; } bucketdb::BucketState -BucketDB::cachedGet(const BucketId &bucketId) const +BucketDB::cachedGet(BucketId bucketId) const { if (isCachedBucket(bucketId)) { return _cachedBucketState; @@ -138,7 +138,7 @@ BucketDB::cachedGet(const BucketId &bucketId) const } storage::spi::BucketInfo -BucketDB::cachedGetBucketInfo(const BucketId &bucketId) const +BucketDB::cachedGetBucketInfo(BucketId bucketId) const { if (isCachedBucket(bucketId)) { return _cachedBucketState; @@ -151,14 +151,14 @@ BucketDB::cachedGetBucketInfo(const BucketId &bucketId) const } bool -BucketDB::hasBucket(const BucketId &bucketId) const +BucketDB::hasBucket(BucketId bucketId) const { return (_map.find(bucketId) != _map.end()); } bool -BucketDB::isActiveBucket(const BucketId &bucketId) const +BucketDB::isActiveBucket(BucketId bucketId) const { auto itr = _map.find(bucketId); return (itr != _map.end()) && itr->second.isActive(); @@ -199,7 +199,7 @@ BucketDB::checkEmpty() const void -BucketDB::setBucketState(const BucketId &bucketId, bool active) +BucketDB::setBucketState(BucketId bucketId, bool active) { BucketState &state = _map[bucketId]; state.setActive(active); @@ -207,7 +207,7 @@ BucketDB::setBucketState(const BucketId &bucketId, bool active) void -BucketDB::createBucket(const BucketId &bucketId) +BucketDB::createBucket(BucketId bucketId) { BucketState &state = _map[bucketId]; (void) state; @@ -215,7 +215,7 @@ BucketDB::createBucket(const BucketId &bucketId) void -BucketDB::deleteEmptyBucket(const BucketId &bucketId) +BucketDB::deleteEmptyBucket(BucketId bucketId) { auto itr = _map.find(bucketId); if (itr == _map.end()) { @@ -262,7 +262,7 @@ BucketDB::populateActiveBuckets(BucketId::List buckets) } BucketState activeState; activeState.setActive(true); - for (const BucketId & bucketId : toAdd) { + for (BucketId bucketId : toAdd) { auto [itr, inserted] = _map.emplace(bucketId, activeState); assert(inserted); } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h index 955b6c8062c..3dbb3ad8c04 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h @@ -20,7 +20,6 @@ public: typedef storage::spi::BucketChecksum BucketChecksum; typedef bucketdb::BucketState BucketState; typedef std::map Map; - typedef Map::const_iterator ConstMapIterator; private: Map _map; @@ -34,42 +33,39 @@ public: ~BucketDB(); const BucketState & add(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType); - void add(const BucketId &bucketId, const BucketState & state); + void add(BucketId bucketId, const BucketState & state); void remove(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType); void remove_batch(const std::vector &removed, SubDbType sub_db_type); void modify(const GlobalId &gid, - const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize, - const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize, + BucketId oldBucketId, Timestamp oldTimestamp, uint32_t oldDocSize, + BucketId newBucketId, Timestamp newTimestamp, uint32_t newDocSize, SubDbType subDbType); - BucketState get(const BucketId &bucketId) const; - void cacheBucket(const BucketId &bucketId); + BucketState get(BucketId bucketId) const; + void cacheBucket(BucketId bucketId); void uncacheBucket(); - bool isCachedBucket(const BucketId &bucketId) const; - storage::spi::BucketInfo cachedGetBucketInfo(const BucketId &bucketId) const; - BucketState cachedGet(const BucketId &bucketId) const; - bool hasBucket(const BucketId &bucketId) const; + bool isCachedBucket(BucketId bucketId) const; + storage::spi::BucketInfo cachedGetBucketInfo(BucketId bucketId) const; + BucketState cachedGet(BucketId bucketId) const; + bool hasBucket(BucketId bucketId) const; BucketId::List getBuckets() const; bool empty() const; - void setBucketState(const BucketId &bucketId, bool active); - void createBucket(const BucketId &bucketId); - void deleteEmptyBucket(const BucketId &bucketId); + void setBucketState(BucketId bucketId, bool active); + void createBucket(BucketId bucketId); + void deleteEmptyBucket(BucketId bucketId); BucketId::List getActiveBuckets() const; BucketId::List populateActiveBuckets(BucketId::List buckets); - - ConstMapIterator end() const { return _map.end(); } - ConstMapIterator lowerBound(const BucketId &bucket) const { return _map.lower_bound(bucket); } size_t size() const { return _map.size(); } - bool isActiveBucket(const BucketId &bucketId) const; - BucketState *getBucketStatePtr(const BucketId &bucket); - void unloadBucket(const BucketId &bucket, const BucketState &delta); + bool isActiveBucket(BucketId bucketId) const; + BucketState *getBucketStatePtr(BucketId bucket); + void unloadBucket(BucketId bucket, const BucketState &delta); }; } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp deleted file mode 100644 index c691e786add..00000000000 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "bucketscaniterator.h" - -using document::BucketId; - -namespace proton::bucketdb { - -ScanIterator::ScanIterator(const Guard & db, BucketId bucket) - : _db(std::move(db)), - _itr(_db->lowerBound(bucket)), - _end(_db->end()) -{ } - -} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h deleted file mode 100644 index 1b68cfc1e59..00000000000 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include "bucket_db_owner.h" -#include "bucketdb.h" - -namespace proton::bucketdb { - - -class ScanIterator { -private: - using BucketId = document::BucketId; - using BucketIterator = BucketDB::ConstMapIterator; - const Guard &_db; - BucketIterator _itr; - BucketIterator _end; - -public: - ScanIterator(const Guard & db, BucketId bucket); - ScanIterator(const ScanIterator &) = delete; - ScanIterator(ScanIterator &&rhs) = delete; - ScanIterator &operator=(const ScanIterator &) = delete; - ScanIterator &operator=(ScanIterator &&rhs) = delete; - - bool valid() const { return _itr != _end; } - bool isActive() const { return _itr->second.isActive(); } - BucketId getBucket() const { return _itr->first; } - bool hasReadyBucketDocs() const { return _itr->second.getReadyCount() != 0; } - bool hasNotReadyBucketDocs() const { return _itr->second.getNotReadyCount() != 0; } - - ScanIterator & operator++() { - ++_itr; - return *this; - } -}; - -} diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index 45cac1d02e9..415000c3f9b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -9,6 +9,7 @@ #include "document_db_maintenance_config.h" #include #include +#include #include #include #include @@ -141,7 +142,7 @@ BucketMoveJob::create(const std::shared_ptr &calc, } BucketMoveJob::NeedResult -BucketMoveJob::needMove(const ScanIterator &itr) const { +BucketMoveJob::needMove(BucketId bucketId, const BucketStateWrapper &itr) const { NeedResult noMove(false, false); const bool hasReadyDocs = itr.hasReadyBucketDocs(); const bool hasNotReadyDocs = itr.hasNotReadyBucketDocs(); @@ -154,13 +155,13 @@ BucketMoveJob::needMove(const ScanIterator &itr) const { if (!_calc || (_calc->nodeRetired() && !isActive)) { return noMove; } - const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, itr.getBucket())); + const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, bucketId)); if (shouldBeReady == Trinary::Undefined) { return noMove; } const bool wantReady = (shouldBeReady == Trinary::True) || isActive; LOG(spam, "needMove(): bucket(%s), shouldBeReady(%s), active(%s)", - itr.getBucket().toString().c_str(), toStr(shouldBeReady), toStr(isActive)); + bucketId.toString().c_str(), toStr(shouldBeReady), toStr(isActive)); if (wantReady) { if (!hasNotReadyDocs) { return noMove; // No notready bucket to make ready @@ -301,8 +302,7 @@ BucketMoveJob::considerBucket(const bucketdb::Guard & guard, BucketId bucket) { void BucketMoveJob::reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket) { assert( ! _bucketsInFlight.contains(bucket)); - ScanIterator itr(guard, bucket); - auto [mustMove, wantReady] = needMove(itr); + auto [mustMove, wantReady] = needMove(bucket, guard->get(bucket)); if (mustMove) { _buckets2Move[bucket] = wantReady; } else { @@ -322,10 +322,11 @@ BucketMoveJob::BucketMoveSet BucketMoveJob::computeBuckets2Move(const bucketdb::Guard & guard) { BucketMoveJob::BucketMoveSet toMove; - for (ScanIterator itr(guard, BucketId()); itr.valid(); ++itr) { - auto [mustMove, wantReady] = needMove(itr); + BucketId::List buckets = guard->getBuckets(); + for (BucketId bucketId : buckets) { + auto [mustMove, wantReady] = needMove(bucketId, guard->get(bucketId)); if (mustMove) { - toMove[itr.getBucket()] = wantReady; + toMove[bucketId] = wantReady; } } return toMove; diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h index d4438fcd411..9885b581a24 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h @@ -8,9 +8,9 @@ #include "ibucketstatechangedhandler.h" #include "iclusterstatechangedhandler.h" #include "maintenancedocumentsubdb.h" -#include #include #include +#include namespace storage::spi { struct BucketExecutor; } @@ -49,7 +49,6 @@ private: using IDestructorCallbackSP = std::shared_ptr; using IThreadService = searchcorespi::index::IThreadService; using BucketId = document::BucketId; - using ScanIterator = bucketdb::ScanIterator; using BucketMoveSet = std::map; using NeedResult = std::pair; using ActiveState = storage::spi::BucketInfo::ActiveState; @@ -79,6 +78,17 @@ private: IBucketStateChangedNotifier &_bucketStateChangedNotifier; IDiskMemUsageNotifier &_diskMemUsageNotifier; + class BucketStateWrapper { + private: + const bucketdb::BucketState & _state; + + public: + BucketStateWrapper(const bucketdb::BucketState & state) noexcept : _state(state) {} + + bool isActive() const noexcept { return _state.isActive(); } + bool hasReadyBucketDocs() const noexcept { return _state.getReadyCount() != 0; } + bool hasNotReadyBucketDocs() const noexcept { return _state.getNotReadyCount() != 0; } + }; BucketMoveJob(const std::shared_ptr &calc, vespalib::RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, @@ -103,7 +113,7 @@ private: void reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket); void updatePending(); void cancelBucket(BucketId bucket); // True if something to cancel - NeedResult needMove(const ScanIterator &itr) const; + NeedResult needMove(BucketId bucketId, const BucketStateWrapper &itr) const; BucketMoveSet computeBuckets2Move(const bucketdb::Guard & guard); BucketMoverSP createMover(BucketId bucket, bool wantReady); BucketMoverSP greedyCreateMover(); -- cgit v1.2.3 From 5aabc4e97f06809a02e208c00b52ebdbac0172ca Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 18 Jul 2022 18:49:24 +0000 Subject: Use a hash map for faster lookup, and use explicit sorting when handing out bucket lists. --- .../vespa/searchcore/proton/bucketdb/bucketdb.cpp | 31 +++++++++------------- .../vespa/searchcore/proton/bucketdb/bucketdb.h | 4 +-- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp index 5ee6fb3973f..a99d5cbd573 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp @@ -2,6 +2,7 @@ #include "bucketdb.h" #include "remove_batch_entry.h" +#include #include #include #include @@ -35,10 +36,7 @@ bucketdb::BucketState * BucketDB::getBucketStatePtr(BucketId bucket) { auto it(_map.find(bucket)); - if (it != _map.end()) { - return &it->second; - } - return nullptr; + return (it != _map.end()) ? &it->second : nullptr; } void @@ -102,10 +100,7 @@ bucketdb::BucketState BucketDB::get(BucketId bucketId) const { auto itr = _map.find(bucketId); - if (itr != _map.end()) { - return itr->second; - } - return BucketState(); + return (itr != _map.end()) ? itr->second : BucketState(); } void @@ -143,11 +138,7 @@ BucketDB::cachedGetBucketInfo(BucketId bucketId) const if (isCachedBucket(bucketId)) { return _cachedBucketState; } - auto itr = _map.find(bucketId); - if (itr != _map.end()) { - return itr->second; - } - return BucketState(); + return get(bucketId); } bool @@ -172,6 +163,7 @@ BucketDB::getBuckets() const for (const auto & entry : _map) { buckets.push_back(entry.first); } + std::sort(buckets.begin(), buckets.end()); return buckets; } @@ -231,11 +223,13 @@ document::BucketId::List BucketDB::getActiveBuckets() const { BucketId::List buckets; + buckets.reserve(_map.size()); for (const auto & entry : _map) { if (entry.second.isActive()) { buckets.push_back(entry.first); } } + std::sort(buckets.begin(), buckets.end()); return buckets; } @@ -247,11 +241,12 @@ BucketDB::populateActiveBuckets(BucketId::List buckets) std::sort(buckets.begin(), buckets.end()); auto si = buckets.begin(); auto se = buckets.end(); - for (const auto & entry : _map) { - for (; si != se && !(entry.first < *si); ++si) { - if (*si < entry.first) { + BucketId::List currentBuckets = getBuckets(); + for (BucketId bucketId : currentBuckets) { + for (; si != se && !(bucketId < *si); ++si) { + if (*si < bucketId) { toAdd.push_back(*si); - } else if (!entry.second.isActive()) { + } else if (!isActiveBucket(bucketId)) { fixupBuckets.push_back(*si); setBucketState(*si, true); } @@ -263,7 +258,7 @@ BucketDB::populateActiveBuckets(BucketId::List buckets) BucketState activeState; activeState.setActive(true); for (BucketId bucketId : toAdd) { - auto [itr, inserted] = _map.emplace(bucketId, activeState); + auto [itr, inserted] = _map.insert(std::make_pair(bucketId, activeState)); assert(inserted); } return fixupBuckets; diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h index 3dbb3ad8c04..bd6aa16504b 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h @@ -5,7 +5,7 @@ #include "bucketstate.h" #include #include -#include +#include namespace proton::bucketdb { class RemoveBatchEntry; } @@ -19,7 +19,7 @@ public: typedef storage::spi::Timestamp Timestamp; typedef storage::spi::BucketChecksum BucketChecksum; typedef bucketdb::BucketState BucketState; - typedef std::map Map; + typedef vespalib::hash_map Map; private: Map _map; -- cgit v1.2.3 From 4e682e5e032f7a1390717662080c26efd63a4483 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 3 Jan 2019 17:55:46 +0000 Subject: run unpack too --- searchlib/src/vespa/searchlib/test/initrange.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/searchlib/src/vespa/searchlib/test/initrange.cpp b/searchlib/src/vespa/searchlib/test/initrange.cpp index 5154a8abb08..d082bfd4e78 100644 --- a/searchlib/src/vespa/searchlib/test/initrange.cpp +++ b/searchlib/src/vespa/searchlib/test/initrange.cpp @@ -72,7 +72,7 @@ InitRangeVerifier::InitRangeVerifier() : } } -InitRangeVerifier::~InitRangeVerifier() { } +InitRangeVerifier::~InitRangeVerifier() = default; InitRangeVerifier::DocIds InitRangeVerifier::invert(const DocIds & docIds, uint32_t docIdlimit) @@ -173,6 +173,7 @@ InitRangeVerifier::searchRelaxed(SearchIterator & it, Range range) for (uint32_t docid = range.first; docid < range.second; ++docid) { if (it.seek(docid)) { result.emplace_back(docid); + it.unpack(docid); } } return result; @@ -185,6 +186,7 @@ InitRangeVerifier::searchStrict(SearchIterator & it, Range range) it.initRange(range.first, range.second); for (uint32_t docId = it.seekFirst(range.first); docId < range.second; docId = it.seekNext(docId + 1)) { result.push_back(docId); + it.unpack(docId); } return result; } -- cgit v1.2.3 From 6dcfc6aaebc0d965c09e405069722631b785c9d1 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 4 Jan 2019 17:31:44 +0000 Subject: Allow test to work with unpack too. --- searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp index 77cc7920ea8..d0951cbe2d3 100644 --- a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp +++ b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp @@ -264,7 +264,7 @@ TEST_F("test Eager Matching Children", MockFixture(5)) { class IteratorChildrenVerifier : public search::test::IteratorChildrenVerifier { private: SearchIterator::UP create(const std::vector &children) const override { - std::vector no_child_match; + std::vector no_child_match(children.size(), &_tfmd); MatchData::UP no_match_data; return DotProductSearch::create(children, _tfmd, false, no_child_match, _weights, std::move(no_match_data)); } @@ -273,7 +273,7 @@ private: class WeightIteratorChildrenVerifier : public search::test::DwaIteratorChildrenVerifier { private: SearchIterator::UP create(std::vector && children) const override { - return SearchIterator::UP(DotProductSearch::create(_tfmd, false, _weights, std::move(children))); + return DotProductSearch::create(_tfmd, false, _weights, std::move(children)); } }; -- cgit v1.2.3 From 272549560656e507a31b2194aa768ebfc91b91e3 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Aug 2022 18:04:11 +0000 Subject: Just create a new value every time we deserialize, then there is no need to to extra cloning if it must be kept. --- .../src/vespa/searchvisitor/searchvisitor.cpp | 1 - .../src/vespa/vsm/common/storagedocument.cpp | 29 ++++++++-------------- .../src/vespa/vsm/common/storagedocument.h | 1 - 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index ddd2457ec49..c2bbf4d09da 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -936,7 +936,6 @@ SearchVisitor::handleDocument(StorageDocument & document) group(document.docDoc(), rp.getRankScore(), false); if (amongTheBest) { - document.saveCachedFields(); needToKeepDocument = true; } diff --git a/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp b/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp index a0d666268f5..b73b6be2862 100644 --- a/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp +++ b/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp @@ -17,28 +17,31 @@ StorageDocument::StorageDocument(document::Document::UP doc, const SharedFieldPa _fieldMap(fim), _cachedFields(getFieldCount()), _backedFields() -{ } +{ + _backedFields.reserve(getFieldCount()); +} -StorageDocument::~StorageDocument() { } +StorageDocument::~StorageDocument() = default; namespace { FieldPath _emptyFieldPath; - StorageDocument::SubDocument _empySubDocument(NULL, _emptyFieldPath.getFullRange()); + StorageDocument::SubDocument _empySubDocument(nullptr, _emptyFieldPath.getFullRange()); } const StorageDocument::SubDocument & StorageDocument::getComplexField(FieldIdT fId) const { - if (_cachedFields[fId].getFieldValue() == NULL) { + if (_cachedFields[fId].getFieldValue() == nullptr) { const FieldPath & fp = (*_fieldMap)[fId]; if ( ! fp.empty() ) { const document::StructuredFieldValue * sfv = _doc.get(); NestedIterator nested = fp.getFullRange(); const document::FieldPathEntry& fvInfo = nested.cur(); - bool ok = sfv->getValue(fvInfo.getFieldRef(), fvInfo.getFieldValueToSet()); - if (ok) { - SubDocument tmp(&fvInfo.getFieldValueToSet(), nested.next()); + document::FieldValue::UP fv = sfv->getValue(fvInfo.getFieldRef()); + if (fv) { + SubDocument tmp(fv.get(), nested.next()); _cachedFields[fId].swap(tmp); + _backedFields.push_back(std::move(fv)); } } else { LOG(debug, "Failed getting field fId %d.", fId); @@ -48,18 +51,6 @@ StorageDocument::getComplexField(FieldIdT fId) const return _cachedFields[fId]; } -void StorageDocument::saveCachedFields() const -{ - size_t m(_cachedFields.size()); - _backedFields.reserve(m); - for (size_t i(0); i < m; i++) { - if (_cachedFields[i].getFieldValue() != 0) { - _backedFields.emplace_back(document::FieldValue::UP(_cachedFields[i].getFieldValue()->clone())); - _cachedFields[i].setFieldValue(_backedFields.back().get()); - } - } -} - const document::FieldValue * StorageDocument::getField(FieldIdT fId) const { diff --git a/streamingvisitors/src/vespa/vsm/common/storagedocument.h b/streamingvisitors/src/vespa/vsm/common/storagedocument.h index a7f21cb052f..0ac94157462 100644 --- a/streamingvisitors/src/vespa/vsm/common/storagedocument.h +++ b/streamingvisitors/src/vespa/vsm/common/storagedocument.h @@ -47,7 +47,6 @@ public: const SubDocument &getComplexField(FieldIdT fId) const; const document::FieldValue *getField(FieldIdT fId) const override; bool setField(FieldIdT fId, document::FieldValue::UP fv) override ; - void saveCachedFields() const; private: document::Document::UP _doc; SharedFieldPathMap _fieldMap; -- cgit v1.2.3 From 524aa1541cadc3ca9a8d4e19bfe6ddc608b0ade1 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Sat, 13 Aug 2022 19:54:17 +0200 Subject: Simplify --- .../clustercontroller/core/FleetController.java | 12 ++---------- .../core/status/LegacyIndexPageRequestHandler.java | 21 ++++++++++----------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index 4097810b633..ac33398120f 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -22,7 +22,6 @@ import com.yahoo.vespa.clustercontroller.core.status.ClusterStateRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.LegacyIndexPageRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.LegacyNodePageRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.NodeHealthRequestHandler; -import com.yahoo.vespa.clustercontroller.core.status.RunDataExtractor; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageResponse; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageServer; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageServerInterface; @@ -101,13 +100,6 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta // deriving is done. private Set configuredBucketSpaces = Collections.emptySet(); - private final RunDataExtractor dataExtractor = new RunDataExtractor() { - @Override - public FleetControllerOptions getOptions() { return options; } - @Override - public ContentCluster getCluster() { return cluster; } - }; - public FleetController(FleetControllerContext context, Timer timer, EventLog eventLog, @@ -155,8 +147,7 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta new ClusterStateRequestHandler(stateVersionTracker)); this.statusRequestRouter.addHandler( "^/$", - new LegacyIndexPageRequestHandler(timer, cluster, masterElectionHandler, stateVersionTracker, eventLog, - timer.getCurrentTimeInMillis(), dataExtractor)); + new LegacyIndexPageRequestHandler(timer, cluster, masterElectionHandler, stateVersionTracker, eventLog, options)); propagateOptions(); } @@ -1240,4 +1231,5 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta public EventLog getEventLog() { return eventLog; } + } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java index 96dc114c734..4f20b3d0cdc 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java @@ -36,22 +36,21 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa private final StateVersionTracker stateVersionTracker; private final EventLog eventLog; private final long startedTime; - private final RunDataExtractor data; + private final FleetControllerOptions options; public LegacyIndexPageRequestHandler(Timer timer, ContentCluster cluster, MasterElectionHandler masterElectionHandler, StateVersionTracker stateVersionTracker, EventLog eventLog, - long startedTime, - RunDataExtractor data) { + FleetControllerOptions options) { this.timer = timer; this.cluster = cluster; this.masterElectionHandler = masterElectionHandler; this.stateVersionTracker = stateVersionTracker; this.eventLog = eventLog; - this.startedTime = startedTime; - this.data = data; + this.startedTime = timer.getCurrentTimeInMillis(); + this.options = options; } @Override @@ -63,7 +62,7 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa response.setContentType("text/html"); StringBuilder content = new StringBuilder(); content.append("\n"); - response.writeHtmlHeader(content, cluster.getName() + " Cluster Controller " + data.getOptions().fleetControllerIndex + " Status Page"); + response.writeHtmlHeader(content, cluster.getName() + " Cluster Controller " + options.fleetControllerIndex + " Status Page"); content.append("

") .append(" [ Current config") .append(" | Cluster states") @@ -72,19 +71,19 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa content.append(""); //content.append(""); content.append("
UTC time when creating this page:").append(RealTimer.printDateNoMilliSeconds(currentTime, tz)).append("
Fleetcontroller version:" + Vtag.V_TAG_PKG + "
Cluster controller uptime:" + RealTimer.printDuration(currentTime - startedTime) + "
"); - if (masterElectionHandler.isAmongNthFirst(data.getOptions().stateGatherCount)) { + if (masterElectionHandler.isAmongNthFirst(options.stateGatherCount)) { // Table overview of all the nodes - writeHtmlState(cluster, content, timer, stateVersionTracker, data.getOptions(), eventLog); + writeHtmlState(cluster, content, timer, stateVersionTracker, options, eventLog); // Current cluster state and cluster state history writeHtmlState(stateVersionTracker, content); } else { // Overview of current config - data.getOptions().writeHtmlState(content); + options.writeHtmlState(content); } // State of master election - masterElectionHandler.writeHtmlState(content, data.getOptions().stateGatherCount); + masterElectionHandler.writeHtmlState(content, options.stateGatherCount); // Overview of current config - data.getOptions().writeHtmlState(content); + options.writeHtmlState(content); // Event log eventLog.writeHtmlState(content, null); response.writeHtmlFooter(content, ""); -- cgit v1.2.3 From 11296c9e4b4e8ca2749ff9713f31fc17ea7b7462 Mon Sep 17 00:00:00 2001 From: jonmv Date: Fri, 12 Aug 2022 11:58:04 +0200 Subject: Remove dated version check TODO --- .../vespa/hosted/controller/deployment/JobController.java | 2 +- .../hosted/controller/deployment/InternalStepRunnerTest.java | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 881107fa0f9..3d4f4833757 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -194,7 +194,7 @@ public class JobController { Map.of("from", Long.toString(from.toEpochMilli()))), from); - if (run.hasStep(installTester) && run.versions().targetPlatform().isAfter(new Version("7.590"))) { // todo jonmv: remove + if (run.hasStep(installTester)) { deployedAt = run.stepInfo(installTester).flatMap(StepInfo::startTime).orElseThrow(); from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); List testerLog = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 493c0945ecc..784ea284de5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -470,28 +470,24 @@ public class InternalStepRunnerTest { new LogEntry(lastId + 4, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - /* new LogEntry(lastId + 5, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), new LogEntry(lastId + 6, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - */ - new LogEntry(lastId + 5, tester.clock().instant().minusSeconds(3), info, + new LogEntry(lastId + 7, tester.clock().instant().minusSeconds(3), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - new LogEntry(lastId + 6, tester.clock().instant().minusSeconds(3), warning, + new LogEntry(lastId + 8, tester.clock().instant().minusSeconds(3), warning, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstderr\n" + - "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)")); - /* + "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)"), new LogEntry(lastId + 9, tester.clock().instant().minusSeconds(3), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), new LogEntry(lastId + 10, tester.clock().instant().minusSeconds(3), warning, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstderr\n" + "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)")); - */ } @Test -- cgit v1.2.3 From dcb841cb1f1acb24dddab8c8745dc760b59849c1 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 15 Aug 2022 12:36:36 +0200 Subject: Remove unused argument and method --- .../yahoo/vespa/clustercontroller/core/FleetController.java | 2 -- .../clustercontroller/core/database/DatabaseHandler.java | 11 ++++------- .../vespa/clustercontroller/core/DatabaseHandlerTest.java | 7 ------- .../clustercontroller/core/SystemStateBroadcasterTest.java | 7 ------- 4 files changed, 4 insertions(+), 23 deletions(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index ac33398120f..29887666a1b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -1144,8 +1144,6 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta @Override public FleetController getFleetController() { return FleetController.this; } @Override - public SlobrokListener getNodeAddedOrRemovedListener() { return FleetController.this; } - @Override public NodeListener getNodeStateUpdateListener() { return FleetController.this; } }; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java index 01b8ed48c80..408c10e81b1 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java @@ -6,14 +6,12 @@ import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.ClusterStateBundle; import com.yahoo.vespa.clustercontroller.core.ContentCluster; -import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; import com.yahoo.vespa.clustercontroller.core.FleetController; +import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; import com.yahoo.vespa.clustercontroller.core.NodeInfo; import com.yahoo.vespa.clustercontroller.core.Timer; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import org.apache.zookeeper.KeeperException; - import java.io.PrintWriter; import java.io.StringWriter; import java.util.Map; @@ -32,7 +30,6 @@ public class DatabaseHandler { public interface DatabaseContext { ContentCluster getCluster(); FleetController getFleetController(); - SlobrokListener getNodeAddedOrRemovedListener(); NodeListener getNodeStateUpdateListener(); } @@ -179,7 +176,7 @@ public class DatabaseHandler { private boolean usingZooKeeper() { return (zooKeeperAddress != null); } - private void connect(ContentCluster cluster, long currentTime) { + private void connect(long currentTime) { try { lastZooKeeperConnectionAttempt = currentTime; synchronized (databaseMonitor) { @@ -251,7 +248,7 @@ public class DatabaseHandler { return false; // Not time to attempt connection yet. } didWork = true; - connect(databaseContext.getCluster(), currentTime); + connect(currentTime); } try { synchronized (databaseMonitor) { @@ -344,7 +341,7 @@ public class DatabaseHandler { return didWork; } - public void setMasterVote(DatabaseContext databaseContext, int wantedMasterCandidate) throws InterruptedException { + public void setMasterVote(DatabaseContext databaseContext, int wantedMasterCandidate) { fleetControllerContext.log(logger, Level.FINE, () -> "Checking if master vote has been updated and need to be stored."); // Schedule a write if one of the following is true: // - There is already a pending vote to be written, that may have been written already without our knowledge diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java index 4d8c7799c58..c07a29b3c1e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java @@ -9,14 +9,12 @@ import com.yahoo.vespa.clustercontroller.core.database.Database; import com.yahoo.vespa.clustercontroller.core.database.DatabaseFactory; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.MockitoAnnotations; - import java.util.Map; import java.util.TreeMap; @@ -70,11 +68,6 @@ public class DatabaseHandlerTest { return mockController; } - @Override - public SlobrokListener getNodeAddedOrRemovedListener() { - return null; - } - @Override public NodeListener getNodeStateUpdateListener() { return null; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java index 0a193766df3..6bc9f434ee6 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java @@ -6,11 +6,9 @@ import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; - import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -68,11 +66,6 @@ public class SystemStateBroadcasterTest { return null; // We assume the broadcaster doesn't use this for our test purposes } - @Override - public SlobrokListener getNodeAddedOrRemovedListener() { - return null; - } - @Override public NodeListener getNodeStateUpdateListener() { return null; -- cgit v1.2.3 From 7b4a447a035234db7f6e655b138fc8fdf4547724 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 15 Aug 2022 09:29:25 +0200 Subject: Move class --- .../vespa/clustercontroller/core/FleetControllerTest.java | 13 ++----------- .../yahoo/vespa/clustercontroller/core/RpcServerTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index 371a57e08c0..01720644fa8 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -7,7 +7,6 @@ import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; -import com.yahoo.jrt.slobrok.api.BackOffPolicy; import com.yahoo.jrt.slobrok.server.Slobrok; import com.yahoo.log.LogSetup; import com.yahoo.vdslib.distribution.ConfiguredNode; @@ -83,14 +82,6 @@ public abstract class FleetControllerTest implements Waiter { LogSetup.initVespaLogging("fleetcontroller"); } - static class BackOff implements BackOffPolicy { - private int counter = 0; - public void reset() { counter = 0; } - public double get() { ++counter; return 0.01; } - public boolean shouldWarn(double v) { return ((counter % 1000) == 10); } - public boolean shouldInform(double v) { return false; } - } - public static class CleanupZookeeperLogsOnSuccess implements TestWatcher { public CleanupZookeeperLogsOnSuccess() {} @@ -335,7 +326,7 @@ public abstract class FleetControllerTest implements Waiter { } private static class ExpectLine { - Pattern regex; + final Pattern regex; int matchedCount = 0; int minCount = 1; int maxCount = 1; @@ -439,7 +430,7 @@ public abstract class FleetControllerTest implements Waiter { eventsGotten.append(eventLine).append("\n"); } errors.append("\nExpected events matching:\n" + exp + "\n"); - errors.append("but got the following events:\n" + eventsGotten.toString()); + errors.append("but got the following events:\n" + eventsGotten); fail(errors.toString()); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java index 641f15467a6..6a7c75c629e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java @@ -9,6 +9,7 @@ import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; +import com.yahoo.jrt.slobrok.api.BackOffPolicy; import com.yahoo.jrt.slobrok.server.Slobrok; import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.distribution.Distribution; @@ -595,4 +596,12 @@ public class RpcServerTest extends FleetControllerTest { return req; } + private static class BackOff implements BackOffPolicy { + private int counter = 0; + public void reset() { counter = 0; } + public double get() { ++counter; return 0.01; } + public boolean shouldWarn(double v) { return ((counter % 1000) == 10); } + public boolean shouldInform(double v) { return false; } + } + } -- cgit v1.2.3 From 4fe51e8e0401543532483eaf6e2f619055388a2e Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 15 Aug 2022 09:37:01 +0200 Subject: Remove unused method and class --- .../core/FleetControllerTest.java | 111 --------------------- .../clustercontroller/core/StateChangeTest.java | 2 +- 2 files changed, 1 insertion(+), 112 deletions(-) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index 01720644fa8..c39a5c52836 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -41,7 +41,6 @@ import java.util.TreeMap; import java.util.TreeSet; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -325,116 +324,6 @@ public abstract class FleetControllerTest implements Waiter { fleetController.waitForCompleteCycle(timeout); } - private static class ExpectLine { - final Pattern regex; - int matchedCount = 0; - int minCount = 1; - int maxCount = 1; - boolean repeatable() { return (maxCount == 0 || maxCount > matchedCount); } - boolean optional() { return (matchedCount >= minCount); } - - boolean matches(String event) { - if (event == null) return false; - boolean m = regex.matcher(event).matches(); - if (m) ++matchedCount; - return m; - } - - ExpectLine(String pattern) { - if (pattern.charAt(0) == '?') { - pattern = pattern.substring(1); - minCount = 0; - } else if (pattern.charAt(0) == '*') { - pattern = pattern.substring(1); - minCount = 0; - maxCount = 0; - } else if (pattern.charAt(0) == '+') { - pattern = pattern.substring(1); - maxCount = 0; - } - regex = Pattern.compile(pattern); - } - - public String toString() { - return "{"+minCount+","+maxCount+"}("+matchedCount+") " + regex; - } - } - - /** - * Verifies that node event list is equal to some expected value. - * The format of the expected values is as follows: - *

    - *
  • Each line in the exp string specifies a pattern to match one or more events. - *
  • A line starting with ? * or + means that the line can match 0 or 1, 0 or more or 1 or more respectively. - *
  • The rest of the line is a regular expression. - *
- */ - protected void verifyNodeEvents(Node n, String exp) { - List events = fleetController.getNodeEvents(n); - String[] expectLines = exp.split("\n"); - List expected = new ArrayList<>(); - for (String line : expectLines) { - expected.add(new ExpectLine(line)); - } - - boolean mismatch = false; - StringBuilder errors = new StringBuilder(); - - int gotno = 0; - int expno = 0; - - while (gotno < events.size() || expno < expected.size()) { - String eventLine = null; - if (gotno < events.size()) { - NodeEvent e = events.get(gotno); - eventLine = e.toString(); - } - - ExpectLine pattern = null; - if (expno < expected.size()) { - pattern = expected.get(expno); - } - - if (pattern == null) { - errors.append("Exhausted expected list before matching event " + gotno - + ": '" + eventLine + "'."); - mismatch = true; - break; - } - - if (pattern.matches(eventLine)) { - if (! pattern.repeatable()) { - ++expno; - } - ++gotno; - } else { - if (pattern.optional()) { - ++expno; - } else { - errors.append("Event " + gotno + ": '" + eventLine - + "' did not match regex " + expno + ": " + pattern); - mismatch = true; - break; - } - } - } - if (!mismatch && expno < expected.size()) { - errors.append("Too few entries in event log (only matched " - + expno + " of " + expected.size() + ")"); - mismatch = true; - } - if (mismatch) { - StringBuilder eventsGotten = new StringBuilder(); - for (Event e : events) { - String eventLine = e.toString(); - eventsGotten.append(eventLine).append("\n"); - } - errors.append("\nExpected events matching:\n" + exp + "\n"); - errors.append("but got the following events:\n" + eventsGotten); - fail(errors.toString()); - } - } - public static Set toNodes(Integer ... indexes) { return Arrays.stream(indexes) .map(i -> new ConfiguredNode(i, false)) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java index 6ad0430b5c3..426ac42fe9e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java @@ -82,7 +82,7 @@ public class StateChangeTest extends FleetControllerTest { super.tearDown(); } - public void verifyNodeEvents(Node n, String correct) { + private void verifyNodeEvents(Node n, String correct) { String actual = ""; for (NodeEvent e : eventLog.getNodeEvents(n)) { actual += e.toString() + "\n"; -- cgit v1.2.3 From 02c03d29a12123c68302d66a7e47f0c97f0f2aa5 Mon Sep 17 00:00:00 2001 From: jonmv Date: Mon, 15 Aug 2022 14:11:23 +0200 Subject: Store Vespa logs in S3 for non-prod-deployment jobs --- .../controller/api/integration/RunDataStore.java | 7 +++ .../api/integration/stubs/MockRunDataStore.java | 16 +++++ .../controller/deployment/JobController.java | 68 +++++++++++++++++++--- .../hosted/controller/deployment/JobProfile.java | 1 + .../restapi/application/ApplicationApiHandler.java | 1 + .../application/JobControllerApiHandlerHelper.java | 27 +++++++-- .../JobControllerApiHandlerHelperTest.java | 21 +++++-- .../responses/deployment-overview-2.json | 8 +++ .../responses/dev-us-east-1-log-first-part.json | 3 +- .../responses/dev-us-east-1-log-second-part.json | 3 +- .../application/responses/staging-test-log.json | 4 +- .../application/responses/system-test-details.json | 4 +- .../application/responses/system-test-log.json | 4 +- .../restapi/application/responses/vespa.log | 1 + 14 files changed, 147 insertions(+), 21 deletions(-) create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java index 9cf508d37c9..3558d18f721 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.integration; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import java.io.InputStream; import java.util.Optional; /** @@ -29,4 +30,10 @@ public interface RunDataStore { /** Deletes all data associated with the given application. */ void delete(ApplicationId id); + /** Stores Vespa logs for the run. */ + void putLogs(RunId id, boolean tester, InputStream logs); + + /** Fetches Vespa logs for the run. */ + InputStream getLogs(RunId id, boolean tester); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java index 32591cfe4e2..cc1e084de8e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java @@ -5,10 +5,14 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * @author jonmv */ @@ -16,6 +20,8 @@ public class MockRunDataStore implements RunDataStore { private final Map logs = new ConcurrentHashMap<>(); private final Map reports = new ConcurrentHashMap<>(); + private final Map vespaLogs = new ConcurrentHashMap<>(); + private final Map testerLogs = new ConcurrentHashMap<>(); @Override public Optional get(RunId id) { @@ -49,4 +55,14 @@ public class MockRunDataStore implements RunDataStore { reports.keySet().removeIf(runId -> runId.application().equals(id)); } + @Override + public void putLogs(RunId id, boolean tester, InputStream logs) { + (tester ? testerLogs : vespaLogs).put(id, uncheck(logs::readAllBytes)); + } + + @Override + public InputStream getLogs(RunId id, boolean tester) { + return new ByteArrayInputStream((tester ? testerLogs : vespaLogs).get(id)); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 3d4f4833757..069aa816af5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -38,6 +38,9 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; @@ -181,20 +184,30 @@ public class JobController { if ( ! run.hasStep(copyVespaLogs)) return run; + storeVespaLogs(id); + + // TODO jonmv: remove all the below around start of 2023. ZoneId zone = id.type().zone(); Optional deployment = Optional.ofNullable(controller.applications().requireInstance(id.application()) .deployments().get(zone)); if (deployment.isEmpty() || deployment.get().at().isBefore(run.start())) return run; - Instant deployedAt = run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal)).flatMap(StepInfo::startTime).orElseThrow(); - Instant from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); - List log = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() - .getLogs(new DeploymentId(id.application(), zone), - Map.of("from", Long.toString(from.toEpochMilli()))), - from); + List log; + Instant deployedAt; + Instant from; + if ( ! run.id().type().isProduction()) { + deployedAt = run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal)).flatMap(StepInfo::startTime).orElseThrow(); + from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); + log = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() + .getLogs(new DeploymentId(id.application(), zone), + Map.of("from", Long.toString(from.toEpochMilli()))), + from); + } + else + log = List.of(); - if (run.hasStep(installTester)) { + if (id.type().isTest()) { deployedAt = run.stepInfo(installTester).flatMap(StepInfo::startTime).orElseThrow(); from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); List testerLog = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() @@ -216,6 +229,47 @@ public class JobController { }); } + public InputStream getVespaLogs(RunId id, long fromMillis, boolean tester) { + Run run = run(id); + return run.stepStatus(copyVespaLogs).map(succeeded::equals).orElse(false) + ? controller.serviceRegistry().runDataStore().getLogs(id, tester) + : getVespaLogsFromLogserver(run, fromMillis, tester); + } + + public static Optional deploymentCompletedAt(Run run, boolean tester) { + return (tester ? run.stepInfo(installTester) + : run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal))) + .flatMap(StepInfo::startTime); + } + + public void storeVespaLogs(RunId id) { + Run run = run(id); + if ( ! id.type().isProduction()) { + try (InputStream logs = getVespaLogsFromLogserver(run, 0, false)) { + controller.serviceRegistry().runDataStore().putLogs(id, false, logs); + } + catch (IOException e) { + throw new UncheckedTimeoutException(e); + } + } + if (id.type().isTest()) { + try (InputStream logs = getVespaLogsFromLogserver(run, 0, true)) { + controller.serviceRegistry().runDataStore().putLogs(id, true, logs); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } + + private InputStream getVespaLogsFromLogserver(Run run, long fromMillis, boolean tester) { + long deploymentCompletedAtMillis = deploymentCompletedAt(run, tester).orElse(Instant.EPOCH).toEpochMilli(); + return controller.serviceRegistry().configServer().getLogs(new DeploymentId(tester ? run.id().tester().id() : run.id().application(), + run.id().type().zone()), + Map.of("from", Long.toString(Math.max(fromMillis, deploymentCompletedAtMillis)), + "to", Long.toString(run.end().orElse(controller.clock().instant()).toEpochMilli()))); + } + /** Fetches any new test log entries, and records the id of the last of these, for continuation. */ public void updateTestLog(RunId id) { locked(id, run -> { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java index 5f9207953ca..f0ec39b8d1c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java @@ -63,6 +63,7 @@ public enum JobProfile { installTester, startTests, endTests, + copyVespaLogs, deactivateTester, report)), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 8d2fac84bc0..7a843b59c87 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -272,6 +272,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/diff/{number}")) return devApplicationPackageDiff(runIdFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/test-config")) return testConfig(appIdFromPath(path), jobTypeFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}")) return JobControllerApiHandlerHelper.runDetailsResponse(controller.jobController(), runIdFromPath(path), request.getProperty("after")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}/logs")) return JobControllerApiHandlerHelper.vespaLogsResponse(controller.jobController(), runIdFromPath(path), asLong(request.getProperty("from"), 0), request.getBooleanProperty("tester")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return getReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 25953c16bf0..60f65070557 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -8,7 +8,6 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.SlimeJsonResponse; -import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; @@ -16,16 +15,15 @@ import com.yahoo.text.Text; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.NotExistsException; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ConvergenceSummary; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.JobController; @@ -39,11 +37,13 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.URI; import java.time.Instant; import java.time.format.TextStyle; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Locale; @@ -53,6 +53,7 @@ import java.util.stream.Stream; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.canary; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; +import static com.yahoo.vespa.hosted.controller.deployment.Step.copyVespaLogs; import static com.yahoo.vespa.hosted.controller.deployment.Step.installInitialReal; import static com.yahoo.vespa.hosted.controller.deployment.Step.installReal; import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.broken; @@ -154,9 +155,27 @@ class JobControllerApiHandlerHelper { .map(Slime::get) .ifPresent(reportArrayCursor -> SlimeUtils.copyArray(reportArrayCursor, detailsObject.setArray("testReports"))); + boolean logsStored = run.stepStatus(copyVespaLogs).map(succeeded::equals).orElse(false); + if (run.hasStep(copyVespaLogs) && ! runId.type().isProduction() && JobController.deploymentCompletedAt(run, false).isPresent()) + detailsObject.setBool("vespaLogsActive", ! logsStored); + + if (runId.type().isTest() && JobController.deploymentCompletedAt(run, true).isPresent()) + detailsObject.setBool("testerLogsActive", ! logsStored); + return new SlimeJsonResponse(slime); } + /** Proxies a Vespa log request for a run to S3 once logs have been copied, or to logserver before this. */ + static HttpResponse vespaLogsResponse(JobController jobController, RunId runId, long fromMillis, boolean tester) { + return new HttpResponse(200) { + @Override public void render(OutputStream out) throws IOException { + try (InputStream logs = jobController.getVespaLogs(runId, fromMillis, tester)) { + logs.transferTo(out); + } + } + }; + } + private static void toSlime(Cursor summaryObject, ConvergenceSummary summary) { summaryObject.setLong("nodes", summary.nodes()); summaryObject.setLong("down", summary.down()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index 233b460333e..168b9b374f3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -7,6 +7,7 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TestReport; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; @@ -144,6 +145,10 @@ public class JobControllerApiHandlerHelperTest { assertResponse(JobControllerApiHandlerHelper.runResponse(app.application(), tester.jobs().runs(userApp.instanceId(), devAwsUsEast2a), Optional.empty(), URI.create("https://some.url:43/root")), "dev-aws-us-east-2a-runs.json"); assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), userApp.instanceId(), URI.create("https://some.url:43/root/")), "overview-user-instance.json"); assertResponse(JobControllerApiHandlerHelper.overviewResponse(tester.controller(), app.application().id(), URI.create("https://some.url:43/root/")), "deployment-overview-2.json"); + + tester.configServer().setLogStream(() -> "no more logs"); + assertResponse(JobControllerApiHandlerHelper.vespaLogsResponse(tester.jobs(), new RunId(app.instanceId(), stagingTest, 1), 0, false), "vespa.log"); + assertResponse(JobControllerApiHandlerHelper.vespaLogsResponse(tester.jobs(), new RunId(app.instanceId(), stagingTest, 1), 0, true), "vespa.log"); } @Test @@ -197,11 +202,17 @@ public class JobControllerApiHandlerHelperTest { Path path = Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/").resolve(fileName); ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - byte[] actualJson = SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(baos.toByteArray()).get(), false); - // Files.write(path, actualJson); - byte[] expected = Files.readAllBytes(path); - assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(expected).get(), false), UTF_8), - new String(actualJson, UTF_8)); + if (fileName.endsWith(".json")) { + byte[] actualJson = SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(baos.toByteArray()).get(), false); + // Files.write(path, actualJson); + byte[] expected = Files.readAllBytes(path); + assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(expected).get(), false), UTF_8), + new String(actualJson, UTF_8)); + } + else { + assertEquals(Files.readString(path), + baos.toString(UTF_8)); + } } catch (Exception e) { throw new RuntimeException(e); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index a7e2d1913cf..a02fb1fb375 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -966,6 +966,10 @@ "name": "endTests", "status": "succeeded" }, + { + "name": "copyVespaLogs", + "status": "succeeded" + }, { "name": "deactivateTester", "status": "succeeded" @@ -1015,6 +1019,10 @@ "name": "endTests", "status": "succeeded" }, + { + "name": "copyVespaLogs", + "status": "succeeded" + }, { "name": "deactivateTester", "status": "succeeded" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json index 9b391196d55..3190dee06fa 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json @@ -82,5 +82,6 @@ "copyVespaLogs": { "status": "unfinished" } - } + }, + "vespaLogsActive": true } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json index 4ffac2bf738..cee7e0f4c92 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json @@ -39,5 +39,6 @@ "status": "succeeded", "startMillis": 0 } - } + }, + "vespaLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json index a2f62621f5b..0b8937c31a4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json @@ -232,5 +232,7 @@ "status": "succeeded", "startMillis": 14503000 } - } + }, + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json index a691762c40b..638a7e24ee7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json @@ -401,5 +401,7 @@ "status": "succeeded", "startMillis": 1600000000000 } - } + }, + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json index 4e8737d5f67..a8b38079b6f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json @@ -404,5 +404,7 @@ "failed": 0 } } - ] + ], + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log new file mode 100644 index 00000000000..25916d9e6df --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log @@ -0,0 +1 @@ +INFO - All good \ No newline at end of file -- cgit v1.2.3 From 9aa66cbdfdca390d9d70fb9fc3f537dea64b7dbd Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Mon, 15 Aug 2022 14:35:41 +0200 Subject: Bump feature flag expiry dates --- flags/src/main/java/com/yahoo/vespa/flags/Flags.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 1b1a5493cc5..e349165b855 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -119,7 +119,7 @@ public class Flags { public static final UnboundBooleanFlag USE_THREE_PHASE_UPDATES = defineFeatureFlag( "use-three-phase-updates", false, - List.of("vekterli"), "2020-12-02", "2022-08-15", + List.of("vekterli"), "2020-12-02", "2022-10-01", "Whether to enable the use of three-phase updates when bucket replicas are out of sync.", "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); @@ -245,7 +245,7 @@ public class Flags { public static final UnboundIntFlag MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS = defineIntFlag( "max-activation-inhibited-out-of-sync-groups", 0, - List.of("vekterli"), "2021-02-19", "2022-08-15", + List.of("vekterli"), "2021-02-19", "2022-10-01", "Allows replicas in up to N content groups to not be activated " + "for query visibility if they are out of sync with a majority of other replicas", "Takes effect at redeployment", @@ -253,14 +253,14 @@ public class Flags { public static final UnboundIntFlag MAX_CONCURRENT_MERGES_PER_NODE = defineIntFlag( "max-concurrent-merges-per-node", 16, - List.of("balder", "vekterli"), "2021-06-06", "2022-08-15", + List.of("balder", "vekterli"), "2021-06-06", "2022-10-01", "Specifies max concurrent merges per content node.", "Takes effect at redeploy", ZONE_ID, APPLICATION_ID); public static final UnboundIntFlag MAX_MERGE_QUEUE_SIZE = defineIntFlag( "max-merge-queue-size", 100, - List.of("balder", "vekterli"), "2021-06-06", "2022-08-15", + List.of("balder", "vekterli"), "2021-06-06", "2022-10-01", "Specifies max size of merge queue.", "Takes effect at redeploy", ZONE_ID, APPLICATION_ID); @@ -339,7 +339,7 @@ public class Flags { public static final UnboundStringFlag MERGE_THROTTLING_POLICY = defineStringFlag( "merge-throttling-policy", "STATIC", - List.of("vekterli"), "2022-01-25", "2022-08-15", + List.of("vekterli"), "2022-01-25", "2022-10-01", "Sets the policy used for merge throttling on the content nodes. " + "Valid values: STATIC, DYNAMIC", "Takes effect at redeployment", @@ -347,7 +347,7 @@ public class Flags { public static final UnboundDoubleFlag PERSISTENCE_THROTTLING_WS_DECREMENT_FACTOR = defineDoubleFlag( "persistence-throttling-ws-decrement-factor", 1.2, - List.of("vekterli"), "2022-01-27", "2022-08-15", + List.of("vekterli"), "2022-01-27", "2022-10-01", "Sets the dynamic throttle policy window size decrement factor for persistence " + "async throttling. Only applies if DYNAMIC policy is used.", "Takes effect on redeployment", @@ -355,7 +355,7 @@ public class Flags { public static final UnboundDoubleFlag PERSISTENCE_THROTTLING_WS_BACKOFF = defineDoubleFlag( "persistence-throttling-ws-backoff", 0.95, - List.of("vekterli"), "2022-01-27", "2022-08-15", + List.of("vekterli"), "2022-01-27", "2022-10-01", "Sets the dynamic throttle policy window size backoff for persistence " + "async throttling. Only applies if DYNAMIC policy is used. Valid range [0, 1]", "Takes effect on redeployment", -- cgit v1.2.3 From 2831803a3af7e8802fd34df881d74e39e6ad1e10 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 15 Aug 2022 12:50:33 +0000 Subject: Add clarifying comment. --- searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp | 8 ++++++-- searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp index d0951cbe2d3..0fef721a6fa 100644 --- a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp +++ b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp @@ -263,7 +263,10 @@ TEST_F("test Eager Matching Children", MockFixture(5)) { class IteratorChildrenVerifier : public search::test::IteratorChildrenVerifier { private: - SearchIterator::UP create(const std::vector &children) const override { + SearchIterator::UP + create(const std::vector &children) const override { + // This is a pragmatic and dirty workaround to make IteratorVerifier test + // not fail on unpack when accessing child match weights std::vector no_child_match(children.size(), &_tfmd); MatchData::UP no_match_data; return DotProductSearch::create(children, _tfmd, false, no_child_match, _weights, std::move(no_match_data)); @@ -272,7 +275,8 @@ private: class WeightIteratorChildrenVerifier : public search::test::DwaIteratorChildrenVerifier { private: - SearchIterator::UP create(std::vector && children) const override { + SearchIterator::UP + create(std::vector && children) const override { return DotProductSearch::create(_tfmd, false, _weights, std::move(children)); } }; diff --git a/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp b/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp index 393140784b0..34cc343b16d 100644 --- a/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp @@ -169,9 +169,9 @@ DotProductSearch::create(const std::vector &children, *childMatch[0], weights[0], std::move(md)); } if (childMatch.size() < 128) { - return SearchIterator::UP(new ArrayHeapImpl(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md)))); + return std::make_unique(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md))); } - return SearchIterator::UP(new HeapImpl(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md)))); + return std::make_unique(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md))); } //----------------------------------------------------------------------------- @@ -186,9 +186,9 @@ DotProductSearch::create(TermFieldMatchData &tmd, typedef DotProductSearchImpl HeapImpl; if (iterators.size() < 128) { - return SearchIterator::UP(new ArrayHeapImpl(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators)))); + return std::make_unique(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators))); } - return SearchIterator::UP(new HeapImpl(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators)))); + return std::make_unique(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators))); } //----------------------------------------------------------------------------- -- cgit v1.2.3 From 697884fc1015e3469610fd7b12c64e02352d91ff Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 15 Aug 2022 15:17:18 +0200 Subject: Avoid off-by-one if version can be triggered the same day --- .../yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java index ddcfef23d86..2d0424e9f03 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java @@ -119,6 +119,7 @@ public class OsUpgradeScheduler extends ControllerMaintainer { Objects.requireNonNull(artifactRepository); } + @Override public Optional change(Version currentVersion, Instant instant) { OsRelease release = artifactRepository.osRelease(currentVersion.getMajor(), tag()); if (!release.version().isAfter(currentVersion)) return Optional.empty(); @@ -162,8 +163,8 @@ public class OsUpgradeScheduler extends ControllerMaintainer { public Optional change(Version currentVersion, Instant instant) { Version wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); while (!wantedVersion.isAfter(currentVersion)) { - wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); instant = instant.plus(Duration.ofDays(1)); + wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); } return Optional.of(new Change(wantedVersion, upgradeBudget(), schedulingInstant(instant, system))); } -- cgit v1.2.3 From 019ab5615dd299e46e7b3e708682d34825a6b7db Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 15 Aug 2022 15:30:22 +0200 Subject: Add Felix Logging to container-test --- application/pom.xml | 4 ++++ cloud-tenant-base-dependencies-enforcer/pom.xml | 3 +++ 2 files changed, 7 insertions(+) diff --git a/application/pom.xml b/application/pom.xml index 34bf1b56c0d..e2176d4deac 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -85,6 +85,10 @@ org.apache.felix org.apache.felix.framework + + org.apache.felix + org.apache.felix.log + org.apache.opennlp opennlp-tools diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 1a83afd1be5..a30cbb86299 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -189,6 +189,7 @@ org.apache.commons:commons-compress:1.21:jar:test org.apache.commons:commons-math3:3.6.1:jar:test org.apache.felix:org.apache.felix.framework:[${felix.version}]:jar:test + org.apache.felix:org.apache.felix.log:1.0.1:jar:test org.apache.httpcomponents.client5:httpclient5:${httpclient5.version}:jar:test org.apache.httpcomponents.core5:httpcore5:${httpclient5.version}:jar:test org.apache.httpcomponents.core5:httpcore5-h2:${httpclient5.version}:jar:test @@ -227,6 +228,8 @@ org.junit.vintage:junit-vintage-engine:[${junit5.version}]:jar:test org.lz4:lz4-java:[${org.lz4.version}]:jar:test org.opentest4j:opentest4j:1.2.0:jar:test + org.osgi:org.osgi.compendium:4.1.0:jar:test + org.osgi:org.osgi.core:4.1.0:jar:test xerces:xercesImpl:2.12.1:jar:test -- cgit v1.2.3 From a96ebf411cc18c968ddd0368bd639d0d654f8276 Mon Sep 17 00:00:00 2001 From: Ola Aunrønning Date: Mon, 15 Aug 2022 15:46:57 +0200 Subject: Add Vespa consumer to host life metric --- .../src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java | 3 +++ .../test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java index e18ea374fe1..148f48136f4 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java @@ -4,6 +4,7 @@ package ai.vespa.metricsproxy.node; import ai.vespa.metricsproxy.core.MetricsManager; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; +import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.ServiceId; @@ -18,6 +19,7 @@ import com.yahoo.container.jdisc.state.HostLifeGatherer; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static ai.vespa.metricsproxy.node.ServiceHealthGatherer.gatherServiceHealthMetrics; @@ -78,6 +80,7 @@ public class NodeMetricGatherer { builder.putMetric(MetricId.toMetricId(key), metrics.get(key).asLong()); } } + builder.addConsumers(Set.of(ConsumerId.toConsumerId("Vespa"))); builders.add(builder); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java index c693e9690e8..0de3526b40d 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.metricsproxy.node; +import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import com.fasterxml.jackson.databind.JsonNode; @@ -10,6 +11,7 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; +import java.util.Set; import static org.junit.Assert.assertEquals; @@ -33,6 +35,7 @@ public class NodeMetricGathererTest { assertEquals(123, packet.timestamp); assertEquals(12l, packet.metrics().get(MetricId.toMetricId("uptime"))); assertEquals(1l, packet.metrics().get(MetricId.toMetricId("alive"))); + assertEquals(Set.of(ConsumerId.toConsumerId("Vespa")), packet.consumers()); } private JsonNode generateHostLifePacket() { -- cgit v1.2.3 From 618d5cc8c9c0f5d8ff105e174849a18f87464437 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 15 Aug 2022 14:16:55 +0000 Subject: Unify on using directive. Add final to implementations classes. --- .../proton/matching/match_loop_communicator.h | 2 +- .../searchcore/proton/matching/match_master.cpp | 2 +- .../searchcore/proton/matching/match_tools.cpp | 18 ++-- .../vespa/searchcore/proton/matching/match_tools.h | 98 ++++++++++++---------- 4 files changed, 65 insertions(+), 55 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h b/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h index 217fa2db1f5..3f50bf9faec 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h @@ -8,7 +8,7 @@ namespace proton::matching { -class MatchLoopCommunicator : public IMatchLoopCommunicator +class MatchLoopCommunicator final : public IMatchLoopCommunicator { private: using IDiversifier = search::queryeval::IDiversifier; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index 985176bcef5..84ff0672fa0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -24,7 +24,7 @@ using vespalib::Issue; namespace { -struct TimedMatchLoopCommunicator : IMatchLoopCommunicator { +struct TimedMatchLoopCommunicator final : IMatchLoopCommunicator { IMatchLoopCommunicator &communicator; vespalib::Timer timer; vespalib::duration elapsed; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index e0449236af0..eecbf116e9d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -14,15 +14,12 @@ #include #include -using search::attribute::IAttributeContext; -using search::queryeval::IRequestContext; using search::queryeval::IDiversifier; using search::attribute::diversity::DiversityFilter; using search::attribute::BasicType; using search::attribute::AttributeBlueprintParams; using vespalib::Issue; -using namespace search::fef; using namespace search::fef::indexproperties::matchphase; using namespace search::fef::indexproperties::matching; using namespace search::fef::indexproperties; @@ -33,6 +30,9 @@ namespace proton::matching { namespace { +using search::fef::Properties; +using search::fef::RankSetup; + bool contains_all(const HandleRecorder::HandleMap &old_map, const HandleRecorder::HandleMap &new_map) { @@ -70,7 +70,7 @@ extractDiversityParams(const RankSetup &rankSetup, const Properties &rankPropert } // namespace proton::matching:: void -MatchTools::setup(search::fef::RankProgram::UP rank_program, double termwise_limit) +MatchTools::setup(std::unique_ptr rank_program, double termwise_limit) { if (_search) { _match_data->soft_reset(); @@ -240,12 +240,12 @@ std::unique_ptr MatchToolsFactory::createDiversifier(uint32_t heapSize) const { if ( !_diversityParams.enabled() ) { - return std::unique_ptr(); + return {}; } auto attr = _requestContext.getAttribute(_diversityParams.attribute); if ( !attr) { Issue::report("Skipping diversity due to no %s attribute.", _diversityParams.attribute.c_str()); - return std::unique_ptr(); + return {}; } size_t max_per_group = heapSize/_diversityParams.min_groups; return DiversityFilter::create(*attr, heapSize, max_per_group, _diversityParams.min_groups, @@ -302,10 +302,8 @@ MatchToolsFactory::get_feature_rename_map() const } AttributeBlueprintParams -MatchToolsFactory::extract_global_filter_params(const search::fef::RankSetup& rank_setup, - const search::fef::Properties& rank_properties, - uint32_t active_docids, - uint32_t docid_limit) +MatchToolsFactory::extract_global_filter_params(const RankSetup& rank_setup, const Properties& rank_properties, + uint32_t active_docids, uint32_t docid_limit) { double lower_limit = GlobalFilterLowerLimit::lookup(rank_properties, rank_setup.get_global_filter_lower_limit()); double upper_limit = GlobalFilterUpperLimit::lookup(rank_properties, rank_setup.get_global_filter_upper_limit()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h index 11e943957ef..d7254d4b958 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h @@ -30,19 +30,25 @@ class MatchTools { private: using IRequestContext = search::queryeval::IRequestContext; - QueryLimiter &_queryLimiter; - const vespalib::Doom &_doom; - const Query &_query; - MaybeMatchPhaseLimiter &_match_limiter; - const QueryEnvironment &_queryEnv; - const search::fef::RankSetup &_rankSetup; - const search::fef::Properties &_featureOverrides; - std::unique_ptr _match_data; - std::unique_ptr _rank_program; - search::queryeval::SearchIterator::UP _search; - HandleRecorder::HandleMap _used_handles; - bool _search_has_changed; - void setup(std::unique_ptr, double termwise_limit = 1.0); + using SearchIterator = search::queryeval::SearchIterator; + using MatchData = search::fef::MatchData; + using MatchDataLayout = search::fef::MatchDataLayout; + using Properties = search::fef::Properties; + using RankProgram = search::fef::RankProgram; + using RankSetup = search::fef::RankSetup; + QueryLimiter &_queryLimiter; + const vespalib::Doom &_doom; + const Query &_query; + MaybeMatchPhaseLimiter &_match_limiter; + const QueryEnvironment &_queryEnv; + const RankSetup &_rankSetup; + const Properties &_featureOverrides; + std::unique_ptr _match_data; + std::unique_ptr _rank_program; + std::unique_ptr _search; + HandleRecorder::HandleMap _used_handles; + bool _search_has_changed; + void setup(std::unique_ptr, double termwise_limit = 1.0); public: typedef std::unique_ptr UP; MatchTools(const MatchTools &) = delete; @@ -52,19 +58,19 @@ public: const Query &query, MaybeMatchPhaseLimiter &match_limiter_in, const QueryEnvironment &queryEnv, - const search::fef::MatchDataLayout &mdl, - const search::fef::RankSetup &rankSetup, - const search::fef::Properties &featureOverrides); + const MatchDataLayout &mdl, + const RankSetup &rankSetup, + const Properties &featureOverrides); ~MatchTools(); const vespalib::Doom &getDoom() const { return _doom; } QueryLimiter & getQueryLimiter() { return _queryLimiter; } MaybeMatchPhaseLimiter &match_limiter() { return _match_limiter; } bool has_second_phase_rank() const; - const search::fef::MatchData &match_data() const { return *_match_data; } - search::fef::RankProgram &rank_program() { return *_rank_program; } - search::queryeval::SearchIterator &search() { return *_search; } - search::queryeval::SearchIterator::UP borrow_search() { return std::move(_search); } - void give_back_search(search::queryeval::SearchIterator::UP search_in) { _search = std::move(search_in); } + const MatchData &match_data() const { return *_match_data; } + RankProgram &rank_program() { return *_rank_program; } + SearchIterator &search() { return *_search; } + std::unique_ptr borrow_search() { return std::move(_search); } + void give_back_search(std::unique_ptr search_in) { _search = std::move(search_in); } void tag_search_as_changed() { _search_has_changed = true; } void setup_first_phase(); void setup_second_phase(); @@ -92,17 +98,25 @@ class MatchToolsFactory { private: using IAttributeFunctor = search::attribute::IAttributeFunctor; - QueryLimiter & _queryLimiter; - search::attribute::AttributeBlueprintParams _global_filter_params; - Query _query; - MaybeMatchPhaseLimiter::UP _match_limiter; - QueryEnvironment _queryEnv; - RequestContext _requestContext; - search::fef::MatchDataLayout _mdl; - const search::fef::RankSetup & _rankSetup; - const search::fef::Properties & _featureOverrides; - DiversityParams _diversityParams; - bool _valid; + using IAttributeContext = search::attribute::IAttributeContext; + using AttributeBlueprintParams = search::attribute::AttributeBlueprintParams; + using MatchDataLayout = search::fef::MatchDataLayout; + using Properties = search::fef::Properties; + using RankProgram = search::fef::RankProgram; + using RankSetup = search::fef::RankSetup; + using IIndexEnvironment = search::fef::IIndexEnvironment; + using IDiversifier = search::queryeval::IDiversifier; + QueryLimiter & _queryLimiter; + AttributeBlueprintParams _global_filter_params; + Query _query; + MaybeMatchPhaseLimiter::UP _match_limiter; + QueryEnvironment _queryEnv; + RequestContext _requestContext; + MatchDataLayout _mdl; + const RankSetup & _rankSetup; + const Properties & _featureOverrides; + DiversityParams _diversityParams; + bool _valid; std::unique_ptr createTask(vespalib::stringref attribute, vespalib::stringref operation) const; @@ -114,23 +128,23 @@ public: MatchToolsFactory(QueryLimiter & queryLimiter, const vespalib::Doom & softDoom, ISearchContext &searchContext, - search::attribute::IAttributeContext &attributeContext, + IAttributeContext &attributeContext, search::engine::Trace & root_trace, vespalib::stringref queryStack, const vespalib::string &location, const ViewResolver &viewResolver, const search::IDocumentMetaStore &metaStore, - const search::fef::IIndexEnvironment &indexEnv, - const search::fef::RankSetup &rankSetup, - const search::fef::Properties &rankProperties, - const search::fef::Properties &featureOverrides, + const IIndexEnvironment &indexEnv, + const RankSetup &rankSetup, + const Properties &rankProperties, + const Properties &featureOverrides, bool is_search); ~MatchToolsFactory(); bool valid() const { return _valid; } const MaybeMatchPhaseLimiter &match_limiter() const { return *_match_limiter; } MatchTools::UP createMatchTools() const; bool should_diversify() const { return _diversityParams.enabled(); } - std::unique_ptr createDiversifier(uint32_t heapSize) const; + std::unique_ptr createDiversifier(uint32_t heapSize) const; search::queryeval::Blueprint::HitEstimate estimate() const { return _query.estimate(); } bool has_first_phase_rank() const; bool has_match_features() const; @@ -151,11 +165,9 @@ public: * When searchable-copies > 1, we must scale the parameters to match the effective range of the estimated hit ratio. * This is done by multiplying with the active hit ratio (active docids / docid limit). */ - static search::attribute::AttributeBlueprintParams - extract_global_filter_params(const search::fef::RankSetup& rank_setup, - const search::fef::Properties& rank_properties, - uint32_t active_docids, - uint32_t docid_limit); + static AttributeBlueprintParams + extract_global_filter_params(const RankSetup& rank_setup, const Properties& rank_properties, + uint32_t active_docids, uint32_t docid_limit); }; } -- cgit v1.2.3 From 480295e5bbabb06ff61130eac6f4b6eeefc8a14e Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 15 Aug 2022 17:42:02 +0200 Subject: Revert "Added defaultIndex processing for userInput items parsing" --- .../yahoo/prelude/query/parser/AbstractParser.java | 6 ++-- .../yahoo/prelude/query/parser/AdvancedParser.java | 2 +- .../com/yahoo/prelude/query/parser/AllParser.java | 8 +++--- .../com/yahoo/prelude/query/parser/AnyParser.java | 4 +-- .../yahoo/prelude/query/parser/PhraseParser.java | 2 +- .../yahoo/prelude/query/parser/SimpleParser.java | 12 +++----- .../prelude/query/parser/StructuredParser.java | 8 ------ .../yahoo/prelude/query/parser/TokenizeParser.java | 2 +- .../com/yahoo/prelude/query/parser/WebParser.java | 4 +-- .../com/yahoo/search/yql/UserInputTestCase.java | 33 +--------------------- 10 files changed, 19 insertions(+), 62 deletions(-) diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 55001ae5915..586d1d32d57 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -143,7 +143,7 @@ public abstract class AbstractParser implements CustomParser { } setState(parsingLanguage, indexFacts); - Item root = parseItems(defaultIndexName); + Item root = parseItems(); if (filterToParse != null) { AnyParser filterParser = new AnyParser(environment); if (root == null) { @@ -222,8 +222,8 @@ public abstract class AbstractParser implements CustomParser { if (tokenOrNull == null) return false; return kind.equals(tokenOrNull.kind); } - - protected abstract Item parseItems(String defaultIndexName); + + protected abstract Item parseItems(); /** * Assigns the default index to query terms having no default index. The diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java index 3358075d670..690fc67af7e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java @@ -20,7 +20,7 @@ public class AdvancedParser extends StructuredParser { super(environment); } - protected Item parseItems(String defaultIndexName) { + protected Item parseItems() { return advancedItems(true); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java index 09caa72ca59..545bb8e777f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java @@ -41,16 +41,16 @@ public class AllParser extends SimpleParser { } @Override - protected Item parseItems(String defaultIndexName) { + protected Item parseItems() { int position = tokens.getPosition(); try { - return parseItemsBody(defaultIndexName); + return parseItemsBody(); } finally { tokens.setPosition(position); } } - protected Item parseItemsBody(String defaultIndexName) { + protected Item parseItemsBody() { // Algorithm: Collect positive, negative, and and'ed items, then combine. CompositeItem and = null; NotItem not = null; // Store negatives here as we go @@ -65,7 +65,7 @@ public class AllParser extends SimpleParser { current = positiveItem(); if (current == null) - current = indexableItem(defaultIndexName); + current = indexableItem(); if (current == null) current = compositeItem(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java index f4ff769ad05..e22043c6b8f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java @@ -31,8 +31,8 @@ public class AnyParser extends SimpleParser { super(environment); } - protected Item parseItems(String defaultIndexName) { - return anyItems(true, defaultIndexName); + protected Item parseItems() { + return anyItems(true); } Item parseFilter(String filter, Language queryLanguage, IndexFacts.Session indexFacts) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java index 72eb56dd0fb..75edf9fbf5c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java @@ -16,7 +16,7 @@ public class PhraseParser extends AbstractParser { super(environment); } - protected Item parseItems(String defaultIndex) { + protected Item parseItems() { return forcedPhrase(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index fafbf55a522..27bce6bd027 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -33,12 +33,12 @@ abstract class SimpleParser extends StructuredParser { * If there's a explicit composite and some other terms, * a rank terms combines them */ - protected Item anyItems(boolean topLevel, String defaultIndexName) { + protected Item anyItems(boolean topLevel) { int position = tokens.getPosition(); Item item = null; try { - item = anyItemsBody(topLevel, defaultIndexName); + item = anyItemsBody(topLevel); return item; } finally { if (item == null) { @@ -47,11 +47,7 @@ abstract class SimpleParser extends StructuredParser { } } - protected Item anyItems(boolean topLevel) { - return anyItems(topLevel, null); - } - - private Item anyItemsBody(boolean topLevel, String defaultIndexName) { + private Item anyItemsBody(boolean topLevel) { Item topLevelItem = null; NotItem not = null; Item item = null; @@ -92,7 +88,7 @@ abstract class SimpleParser extends StructuredParser { } if (item == null) { - item = indexableItem(defaultIndexName); + item = indexableItem(); if (item != null) { if (topLevelItem == null) { topLevelItem = item; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index c668cf66447..f993c7a9e02 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -7,7 +7,6 @@ import com.yahoo.search.query.parser.ParserEnvironment; import java.util.ArrayList; import java.util.List; -import java.util.Objects; import static com.yahoo.prelude.query.parser.Token.Kind.*; @@ -53,18 +52,11 @@ abstract class StructuredParser extends AbstractParser { } protected Item indexableItem() { - return indexableItem(null); - } - - protected Item indexableItem(String defaultIndexName) { int position = tokens.getPosition(); Item item = null; try { String indexName = indexPrefix(); - if (Objects.isNull(indexName)) { - indexName = defaultIndexName; - } setSubmodeFromIndex(indexName, indexFacts); item = number(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java index eefbe5fa0d0..dbbc321d057 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java @@ -22,7 +22,7 @@ public final class TokenizeParser extends AbstractParser { } @Override - protected Item parseItems(String defaultIndex) { + protected Item parseItems() { WeakAndItem weakAnd = new WeakAndItem(); Token token; while (null != (token = tokens.next())) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java index 40497d94a6d..d7c7dec4798 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java @@ -28,7 +28,7 @@ public class WebParser extends AllParser { } @Override - protected Item parseItemsBody(String defaultIndexName) { + protected Item parseItemsBody() { // Algorithm: Collect positive, negative, and'ed and or'ed elements, then combine. CompositeItem and = null; OrItem or = null; @@ -45,7 +45,7 @@ public class WebParser extends AllParser { current = positiveItem(); if (current == null) - current = indexableItem(defaultIndexName); + current = indexableItem(); if (current != null) { if (and != null && (current instanceof WordItem) && "OR".equals(((WordItem)current).getRawWord())) { diff --git a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java index 1e3b52c23af..8fe451dd095 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java @@ -3,10 +3,7 @@ package com.yahoo.search.yql; import static org.junit.jupiter.api.Assertions.*; -import com.yahoo.prelude.Index; -import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.IndexModel; -import com.yahoo.prelude.SearchDefinition; +import com.yahoo.search.query.QueryTree; import org.apache.http.client.utils.URIBuilder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -370,32 +367,4 @@ public class UserInputTestCase { assertEquals("select * from sources * where text_field contains \"boom\"", query.yqlRepresentation()); } - @Test - void testUserInputWithPhraseSegmentingIndex() { - execution = new Execution(searchChain, Execution.Context.createContextStub(createIndexFacts(true))); - URIBuilder builder = searchUri(); - builder.setParameter("wql", "foo&bar"); - builder.setParameter("yql", "select * from sources * where ([{\"defaultIndex\": \"text_field\",\"grammar\": \"any\"}]userInput(@wql))"); - Query query = searchAndAssertNoErrors(builder); - assertEquals("select * from sources * where text_field contains phrase(\"foo\", \"bar\")", query.yqlRepresentation()); - } - - @Test - void testUserInputWithNonPhraseSegmentingIndex() { - execution = new Execution(searchChain, Execution.Context.createContextStub(createIndexFacts(false))); - URIBuilder builder = searchUri(); - builder.setParameter("wql", "foo&bar"); - builder.setParameter("yql", "select * from sources * where ([{\"defaultIndex\": \"text_field\",\"grammar\": \"any\"}]userInput(@wql))"); - Query query = searchAndAssertNoErrors(builder); - assertEquals("select * from sources * where (text_field contains \"foo\" AND text_field contains \"bar\")", query.yqlRepresentation()); - } - - private IndexFacts createIndexFacts(boolean phraseSegmenting) { - SearchDefinition sd = new SearchDefinition("sources"); - Index test = new Index("text_field"); - test.setPhraseSegmenting(phraseSegmenting); - sd.addIndex(test); - return new IndexFacts(new IndexModel(sd)); - } - } -- cgit v1.2.3 From ff05a0ce9953882f37aa5b63f8c11777d12f2186 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 15 Aug 2022 18:59:17 +0200 Subject: Revert "Revert "Revert "Use a hash_set to quickly check if a field i…" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../proton/documentdb/feedview/feedview_test.cpp | 2 +- .../proton/attribute/attribute_writer.cpp | 13 ++-- .../proton/attribute/ifieldupdatecallback.h | 7 +- .../proton/server/searchable_feed_view.cpp | 2 +- .../searchcore/proton/server/storeonlyfeedview.cpp | 83 ++++++++-------------- .../searchcore/proton/server/storeonlyfeedview.h | 24 +++++-- 6 files changed, 61 insertions(+), 70 deletions(-) diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index 641e9879c79..b326db47b5c 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -362,7 +362,7 @@ struct MyAttributeWriter : public IAttributeWriter _updateLid = lid; for (const auto & fieldUpdate : upd.getUpdates()) { search::AttributeVector * attr = getWritableAttribute(fieldUpdate.getField().getName()); - onUpdate.onUpdateField(fieldUpdate.getField(), attr); + onUpdate.onUpdateField(fieldUpdate.getField().getName(), attr); } } void update(SerialNum serialNum, const document::Document &doc, DocumentIdT lid, OnWriteDoneType) override { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index 021fc4717af..bb25b3da7be 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -764,9 +764,9 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document for (const auto &fupd : upd.getUpdates()) { LOG(debug, "Retrieving guard for attribute vector '%s'.", fupd.getField().getName().data()); - auto found = _attrMap.find(fupd.getField().getName()); - AttributeVector * attrp = (found != _attrMap.end()) ? found->second.attribute : nullptr; - onUpdate.onUpdateField(fupd.getField(), attrp); + auto itr = _attrMap.find(fupd.getField().getName()); + AttributeVector * attrp = (itr != _attrMap.end()) ? itr->second.attribute : nullptr; + onUpdate.onUpdateField(fupd.getField().getName(), attrp); if (__builtin_expect(attrp == nullptr, false)) { LOG(spam, "Failed to find attribute vector %s", fupd.getField().getName().data()); continue; @@ -776,15 +776,16 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document if (__builtin_expect(attrp->getStatus().getLastSyncToken() >= serialNum, false)) { continue; } - if (found->second.use_two_phase_put_for_assign_updates && is_single_assign_update(fupd)) { + if (itr->second.use_two_phase_put_for_assign_updates && + is_single_assign_update(fupd)) { auto prepare_task = std::make_unique(serialNum, lid, *attrp, get_single_assign_update_field_value(fupd)); auto complete_task = std::make_unique(*prepare_task, onWriteDone); LOG(debug, "About to handle assign update as two phase put for docid %u in attribute vector '%s'", lid, attrp->getName().c_str()); _shared_executor.execute(CpuUsage::wrap(std::move(prepare_task), CpuUsage::Category::WRITE)); - _attributeFieldWriter.executeTask(found->second.executor_id, std::move(complete_task)); + _attributeFieldWriter.executeTask(itr->second.executor_id, std::move(complete_task)); } else { - args[found->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); + args[itr->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); LOG(debug, "About to apply update for docId %u in attribute vector '%s'.", lid, attrp->getName().c_str()); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h index d3ab970fb39..d8872607b44 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h @@ -5,17 +5,16 @@ #include namespace search { class AttributeVector; } -namespace document { class Field; } namespace proton { struct IFieldUpdateCallback { - virtual ~IFieldUpdateCallback() = default; - virtual void onUpdateField(const document::Field & field, const search::AttributeVector * attr) = 0; + virtual ~IFieldUpdateCallback() { } + virtual void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) = 0; }; struct DummyFieldUpdateCallback : IFieldUpdateCallback { - void onUpdateField(const document::Field & , const search::AttributeVector *) override {} + void onUpdateField(vespalib::stringref, const search::AttributeVector *) override {} }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp index 7a78b4ba82a..207f1d813d8 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp @@ -34,7 +34,7 @@ SearchableFeedView::SearchableFeedView(StoreOnlyFeedView::Context storeOnlyCtx, const FastAccessFeedView::Context &fastUpdateCtx, Context ctx) : Parent(std::move(storeOnlyCtx), params, fastUpdateCtx), _indexWriter(ctx._indexWriter), - _hasIndexedFields(getSchema()->getNumIndexFields() > 0) + _hasIndexedFields(_schema->getNumIndexFields() > 0) { } SearchableFeedView::~SearchableFeedView() = default; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index d7c3246cb4b..a537742b79b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -7,14 +7,14 @@ #include "putdonecontext.h" #include "removedonecontext.h" #include "updatedonecontext.h" +#include +#include +#include #include #include #include #include #include -#include -#include -#include #include #include #include @@ -88,8 +88,8 @@ SummaryPutDoneContext::SummaryPutDoneContext(FeedToken token, IPendingLidTracker SummaryPutDoneContext::~SummaryPutDoneContext() = default; -std::vector -getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::LidVector &lidsToRemove) +std::vector getGidsToRemove(const IDocumentMetaStore &metaStore, + const LidVectorContext::LidVector &lidsToRemove) { std::vector gids; gids.reserve(lidsToRemove.size()); @@ -102,9 +102,8 @@ getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::Lid return gids; } -void -putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, - const DocumentOperation &op, bool is_removed_doc) +void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, + const DocumentOperation &op, bool is_removed_doc) { documentmetastore::IStore::Result putRes( meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(), @@ -118,9 +117,8 @@ putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, assert(op.getLid() == putRes._lid); } -void -removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, - const DocumentOperation &op, bool is_removed_doc) +void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, + const DocumentOperation &op, bool is_removed_doc) { assert(meta_store.validLid(op.getPrevLid())); assert(is_removed_doc == op.getPrevMarkedAsRemoved()); @@ -149,37 +147,6 @@ moveMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, c meta_store.move(op.getPrevLid(), op.getLid(), op.get_prepare_serial_num()); } -class UpdateScope final : public IFieldUpdateCallback -{ -private: - const vespalib::hash_set & _indexedFields; - bool _nonAttributeFields; -public: - bool _hasIndexedFields; - - UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd); - bool hasIndexOrNonAttributeFields() const { - return _hasIndexedFields || _nonAttributeFields; - } - void onUpdateField(const document::Field & field, const search::AttributeVector * attr) override; -}; - -UpdateScope::UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd) - : _indexedFields(indexedFields), - _nonAttributeFields(!upd.getFieldPathUpdates().empty()), - _hasIndexedFields(false) -{} - -void -UpdateScope::onUpdateField(const document::Field & field, const search::AttributeVector * attr) { - if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { - _nonAttributeFields = true; - } - if (!_hasIndexedFields && (_indexedFields.find(field.getId()) != _indexedFields.end())) { - _hasIndexedFields = true; - } -} - } // namespace StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms) @@ -193,20 +160,12 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms _pendingLidsForDocStore(), _pendingLidsForCommit(std::move(ctx._pendingLidsForCommit)), _schema(std::move(ctx._schema)), - _indexedFields(), _writeService(ctx._writeService), _params(params), _metaStore(_documentMetaStoreContext->get()), _gidToLidChangeHandler(ctx._gidToLidChangeHandler) { _docType = _repo->getDocumentType(_params._docTypeName.getName()); - if (_schema && _docType) { - for (const auto &indexField : _schema->getIndexFields()) { - document::FieldPath fieldPath; - _docType->buildFieldPath(fieldPath, indexField.getName()); - _indexedFields.insert(fieldPath.back().getFieldRef().getId()); - } - } } StoreOnlyFeedView::~StoreOnlyFeedView() = default; @@ -248,7 +207,7 @@ void StoreOnlyFeedView::putAttributes(SerialNum, Lid, const Document &, OnPutDoneType) {} void -StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const std::shared_ptr &, OnOperationDoneType) {} +StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const Document::SP &, OnOperationDoneType) {} void StoreOnlyFeedView::preparePut(PutOperation &putOp) @@ -326,7 +285,7 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, const DocumentUpdate & upd, OnOperationDoneType, IFieldUpdateCallback & onUpdate) { for (const auto & fieldUpdate : upd.getUpdates()) { - onUpdate.onUpdateField(fieldUpdate.getField(), nullptr); + onUpdate.onUpdateField(fieldUpdate.getField().getName(), nullptr); } } @@ -428,6 +387,22 @@ StoreOnlyFeedView::heartBeatSummary(SerialNum serialNum, DoneCallback onDone) { })); } +StoreOnlyFeedView::UpdateScope::UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd) + : _schema(&schema), + _indexedFields(false), + _nonAttributeFields(!upd.getFieldPathUpdates().empty()) +{} + +void +StoreOnlyFeedView::UpdateScope::onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) { + if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { + _nonAttributeFields = true; + } + if (!_indexedFields && _schema->isIndexField(fieldName)) { + _indexedFields = true; + } +} + void StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) { if ( ! updOp.getUpdate()) { @@ -457,7 +432,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) } auto onWriteDone = createUpdateDoneContext(std::move(token), get_pending_lid_token(updOp), updOp.getUpdate()); - UpdateScope updateScope(_indexedFields, upd); + UpdateScope updateScope(*_schema, upd); updateAttributes(serialNum, lid, upd, onWriteDone, updateScope); if (updateScope.hasIndexOrNonAttributeFields()) { @@ -465,7 +440,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) FutureDoc futureDoc = promisedDoc.get_future().share(); onWriteDone->setDocument(futureDoc); _pendingLidsForDocStore.waitComplete(lid); - if (updateScope._hasIndexedFields) { + if (updateScope._indexedFields) { updateIndexedFields(serialNum, lid, futureDoc, onWriteDone); } PromisedStream promisedStream; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index 25a98da7ce7..2822aa70525 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -9,6 +9,7 @@ #include "searchcontext.h" #include #include +#include #include #include #include @@ -17,10 +18,9 @@ #include #include #include -#include #include -#include #include +#include namespace vespalib { class IDestructorCallback; } @@ -118,6 +118,22 @@ public: {} }; +protected: + class UpdateScope : public IFieldUpdateCallback + { + private: + const search::index::Schema *_schema; + public: + bool _indexedFields; + bool _nonAttributeFields; + + UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd); + bool hasIndexOrNonAttributeFields() const { + return _indexedFields || _nonAttributeFields; + } + void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) override; + }; + private: const ISummaryAdapter::SP _summaryAdapter; const IDocumentMetaStoreContext::SP _documentMetaStoreContext; @@ -126,9 +142,9 @@ private: LidReuseDelayer _lidReuseDelayer; PendingLidTracker _pendingLidsForDocStore; std::shared_ptr _pendingLidsForCommit; - const search::index::Schema::SP _schema; - vespalib::hash_set _indexedFields; + protected: + const search::index::Schema::SP _schema; searchcorespi::index::IThreadingService &_writeService; PersistentParams _params; IDocumentMetaStore &_metaStore; -- cgit v1.2.3 From 00b69dd91f1073a54f1f307c97693111a9ab63cb Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 16 Aug 2022 06:51:27 +0000 Subject: Use std::make_shared and std::make_unique. --- document/src/vespa/document/datatype/datatype.cpp | 2 +- .../proton/documentdb/feedview/feedview_test.cpp | 35 ++++++++++++---------- .../searchcore/proton/test/dummy_document_store.h | 2 +- .../searchcore/proton/test/dummy_summary_manager.h | 2 +- searchlib/src/vespa/searchlib/index/uri_field.cpp | 3 +- 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/document/src/vespa/document/datatype/datatype.cpp b/document/src/vespa/document/datatype/datatype.cpp index 3cc7034e336..7d53679bc28 100644 --- a/document/src/vespa/document/datatype/datatype.cpp +++ b/document/src/vespa/document/datatype/datatype.cpp @@ -155,7 +155,7 @@ DataType::buildFieldPath(FieldPath & path, vespalib::stringref remainFieldName) { if ( !remainFieldName.empty() ) { path.reserve(4); // Optimize for short paths - onBuildFieldPath(path,remainFieldName); + onBuildFieldPath(path, remainFieldName); } } diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index b326db47b5c..2957a2a015d 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -143,7 +143,7 @@ struct MyIndexWriter : public test::MockIndexWriter uint32_t _wantedLidLimit; MyTracer &_tracer; MyIndexWriter(MyTracer &tracer) - : test::MockIndexWriter(IIndexManager::SP(new test::MockIndexManager())), + : test::MockIndexWriter(std::make_shared()), _removes(), _heartBeatCount(0), _commitCount(0), @@ -224,7 +224,7 @@ struct MyDocumentStore : public test::DummyDocumentStore DocMap _docs; uint64_t _lastSyncToken; uint32_t _compactLidSpaceLidLimit; - MyDocumentStore(const document::DocumentTypeRepo & repo) + MyDocumentStore(const document::DocumentTypeRepo & repo) noexcept : test::DummyDocumentStore("."), _repo(repo), _docs(), @@ -266,42 +266,47 @@ MyDocumentStore::~MyDocumentStore() = default; struct MySummaryManager : public test::DummySummaryManager { MyDocumentStore _store; - MySummaryManager(const document::DocumentTypeRepo & repo) : _store(repo) {} - virtual search::IDocumentStore &getBackingStore() override { return _store; } + MySummaryManager(const document::DocumentTypeRepo & repo) noexcept : _store(repo) {} + ~MySummaryManager() override; + search::IDocumentStore &getBackingStore() override { return _store; } }; +MySummaryManager::~MySummaryManager() = default; + struct MySummaryAdapter : public test::MockSummaryAdapter { ISummaryManager::SP _sumMgr; MyDocumentStore &_store; MyLidVector _removes; - MySummaryAdapter(const document::DocumentTypeRepo & repo) - : _sumMgr(new MySummaryManager(repo)), + MySummaryAdapter(const document::DocumentTypeRepo & repo) noexcept + : _sumMgr(std::make_shared(repo)), _store(static_cast(_sumMgr->getBackingStore())), _removes() {} - virtual void put(SerialNum serialNum, DocumentIdT lid, const Document &doc) override { + ~MySummaryAdapter() override; + void put(SerialNum serialNum, DocumentIdT lid, const Document &doc) override { _store.write(serialNum, lid, doc); } - virtual void put(SerialNum serialNum, DocumentIdT lid, const vespalib::nbostream & os) override { + void put(SerialNum serialNum, DocumentIdT lid, const vespalib::nbostream & os) override { _store.write(serialNum, lid, os); } - virtual void remove(SerialNum serialNum, const DocumentIdT lid) override { + void remove(SerialNum serialNum, const DocumentIdT lid) override { LOG(info, "MySummaryAdapter::remove(): serialNum(%" PRIu64 "), docId(%u)", serialNum, lid); _store.remove(serialNum, lid); _removes.push_back(lid); } - virtual const search::IDocumentStore &getDocumentStore() const override { + const search::IDocumentStore &getDocumentStore() const override { return _store; } - virtual std::unique_ptr get(const DocumentIdT lid, const DocumentTypeRepo &repo) override { + std::unique_ptr get(const DocumentIdT lid, const DocumentTypeRepo &repo) override { return _store.read(lid, repo); } - virtual void compactLidSpace(uint32_t wantedDocIdLimit) override { + void compactLidSpace(uint32_t wantedDocIdLimit) override { _store.compactLidSpace(wantedDocIdLimit); } }; +MySummaryAdapter::~MySummaryAdapter() = default; struct MyAttributeWriter : public IAttributeWriter { @@ -434,7 +439,7 @@ struct SchemaContext }; SchemaContext::SchemaContext() : - _schema(new Schema()), + _schema(std::make_shared()), _builder() { _schema->addIndexField(Schema::IndexField("i1", DataType::STRING, CollectionType::SINGLE)); @@ -442,7 +447,7 @@ SchemaContext::SchemaContext() : _schema->addAttributeField(Schema::AttributeField("a2", DataType::BOOLEANTREE, CollectionType::SINGLE)); _schema->addAttributeField(Schema::AttributeField("a3", DataType::TENSOR, CollectionType::SINGLE)); _schema->addSummaryField(Schema::SummaryField("s1", DataType::STRING, CollectionType::SINGLE)); - _builder.reset(new DocBuilder(*_schema)); + _builder = std::make_unique(*_schema); } SchemaContext::~SchemaContext() = default; @@ -464,7 +469,7 @@ struct DocumentContext DocumentContext::DocumentContext(const vespalib::string &docId, uint64_t timestamp, DocBuilder &builder) : doc(builder.startDocument(docId).startSummaryField("s1").addStr(docId).endField().endDocument().release()), - upd(new DocumentUpdate(*builder.getDocumentTypeRepo(), builder.getDocumentType(), doc->getId())), + upd(std::make_shared(*builder.getDocumentTypeRepo(), builder.getDocumentType(), doc->getId())), bid(BucketFactory::getNumBucketBits(), doc->getId().getGlobalId().convertToBucketId().getRawId()), ts(timestamp) {} diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h index 9e0e8f66dc0..d8d6e119d9b 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h @@ -11,7 +11,7 @@ struct DummyDocumentStore : public search::IDocumentStore vespalib::string _baseDir; DummyDocumentStore() = default; - DummyDocumentStore(const vespalib::string &baseDir) + DummyDocumentStore(const vespalib::string &baseDir) noexcept : _baseDir(baseDir) {} ~DummyDocumentStore() = default; diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h b/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h index 4d3513c52f5..f826cbbe921 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h @@ -9,7 +9,7 @@ namespace test { struct DummySummaryManager : public ISummaryManager { - virtual ISummarySetup::SP + ISummarySetup::SP createSummarySetup(const vespa::config::search::SummaryConfig &, const vespa::config::search::SummarymapConfig &, const vespa::config::search::summary::JuniperrcConfig &, diff --git a/searchlib/src/vespa/searchlib/index/uri_field.cpp b/searchlib/src/vespa/searchlib/index/uri_field.cpp index 380f67210d3..daf0e6e685e 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.cpp +++ b/searchlib/src/vespa/searchlib/index/uri_field.cpp @@ -78,8 +78,7 @@ UriField::setup(const Schema &schema, } void -UriField::markUsed(UsedFieldsMap &usedFields, - uint32_t field) +UriField::markUsed(UsedFieldsMap &usedFields, uint32_t field) { if (field == Schema::UNKNOWN_FIELD_ID) { return; -- cgit v1.2.3 From e7a01cc7055907cec8af4b4ffd38f7655b54c1ea Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Tue, 16 Aug 2022 09:20:13 +0200 Subject: Correct the auto-suggested Co-authored-by: Valerij Fredriksen --- .../com/yahoo/vespa/hosted/controller/deployment/JobController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 069aa816af5..7114402f824 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -249,7 +249,7 @@ public class JobController { controller.serviceRegistry().runDataStore().putLogs(id, false, logs); } catch (IOException e) { - throw new UncheckedTimeoutException(e); + throw new UncheckedIOException(e); } } if (id.type().isTest()) { -- cgit v1.2.3 From 512a2e9eece834a98c735aab9d09e3662b7e16a1 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 26 Jul 2022 14:48:01 +0000 Subject: hookup new version of vespa-logfmt * adds simple go compilation hook for cmake * stop installing perl version of vespa-logfmt --- CMakeLists.txt | 1 + client/CMakeLists.txt | 14 ++++++++++++++ client/go/.gitignore | 1 + client/go/Makefile | 3 ++- vespalog/CMakeLists.txt | 1 - 5 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 client/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 4fbc756236d..9290b2da189 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,6 +51,7 @@ add_subdirectory(ann_benchmark) add_subdirectory(application-model) add_subdirectory(application-preprocessor) add_subdirectory(athenz-identity-provider-service) +add_subdirectory(client) add_subdirectory(config-bundle) add_subdirectory(config-model) add_subdirectory(config-model-api) diff --git a/client/CMakeLists.txt b/client/CMakeLists.txt new file mode 100644 index 00000000000..1ca6d2b4e60 --- /dev/null +++ b/client/CMakeLists.txt @@ -0,0 +1,14 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +set(GODIR ${CMAKE_CURRENT_SOURCE_DIR}/go) + +file(GLOB_RECURSE GOSRCFILES ${GODIR}/*.go) + +add_custom_command(OUTPUT ${GODIR}/bin/vespa-logfmt + COMMAND make + DEPENDS ${GODIR}/Makefile ${GOSRCFILES} + WORKING_DIRECTORY ${GODIR}) + +add_custom_target(vespalog_logfmt ALL DEPENDS ${GODIR}/bin/vespa-logfmt) + +install(PROGRAMS ${GODIR}/bin/vespa-logfmt DESTINATION bin) diff --git a/client/go/.gitignore b/client/go/.gitignore index 8933bc220cb..baab7c638c6 100644 --- a/client/go/.gitignore +++ b/client/go/.gitignore @@ -6,3 +6,4 @@ share/ !target/ mytestapp/ mytestapp-cache/ +mytmp/ diff --git a/client/go/Makefile b/client/go/Makefile index 78adf299f0e..8a07f880c24 100644 --- a/client/go/Makefile +++ b/client/go/Makefile @@ -131,7 +131,8 @@ clean: rmdir -p $(BIN) $(SHARE)/man/man1 > /dev/null 2>&1 || true test: ci - go test ./... + mkdir -p mytmp + TMPDIR=`pwd`/mytmp go test ./... checkfmt: @bash -c "diff --line-format='%L' <(echo -n) <(gofmt -l .)" || { echo "one or more files need to be formatted: try make fmt to fix this automatically"; exit 1; } diff --git a/vespalog/CMakeLists.txt b/vespalog/CMakeLists.txt index 45410a1d29d..cc419681445 100644 --- a/vespalog/CMakeLists.txt +++ b/vespalog/CMakeLists.txt @@ -17,6 +17,5 @@ vespa_define_module( src/test/threads ) -vespa_install_script(src/vespa-logfmt/vespa-logfmt.pl vespa-logfmt bin) install(FILES src/vespa-logfmt/vespa-logfmt.1 DESTINATION man/man1) install(DIRECTORY DESTINATION var/db/vespa/logcontrol) -- cgit v1.2.3 From 8aba0ce9c53f81c29f12ac3cb07157b7818ac9fa Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 16 Aug 2022 09:18:51 +0000 Subject: sort lines --- CMakeLists.txt | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9290b2da189..8972fada5bf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,15 +52,20 @@ add_subdirectory(application-model) add_subdirectory(application-preprocessor) add_subdirectory(athenz-identity-provider-service) add_subdirectory(client) +add_subdirectory(cloud-tenant-cd) +add_subdirectory(clustercontroller-apps) +add_subdirectory(clustercontroller-core) +add_subdirectory(clustercontroller-reindexer) +add_subdirectory(clustercontroller-utils) +add_subdirectory(config) add_subdirectory(config-bundle) +add_subdirectory(configd) +add_subdirectory(configdefinitions) add_subdirectory(config-model) add_subdirectory(config-model-api) +add_subdirectory(config-model-fat) add_subdirectory(config-provisioning) add_subdirectory(config-proxy) -add_subdirectory(config) -add_subdirectory(config-model-fat) -add_subdirectory(configd) -add_subdirectory(configdefinitions) add_subdirectory(configserver) add_subdirectory(configserver-flags) add_subdirectory(configutil) @@ -69,14 +74,9 @@ add_subdirectory(container-core) add_subdirectory(container-disc) add_subdirectory(container-messagebus) add_subdirectory(container-search) -add_subdirectory(container-search-gui) add_subdirectory(container-search-and-docproc) +add_subdirectory(container-search-gui) add_subdirectory(container-spifly) -add_subdirectory(cloud-tenant-cd) -add_subdirectory(clustercontroller-apps) -add_subdirectory(clustercontroller-core) -add_subdirectory(clustercontroller-reindexer) -add_subdirectory(clustercontroller-utils) add_subdirectory(defaults) add_subdirectory(docproc) add_subdirectory(docprocs) @@ -99,8 +99,8 @@ add_subdirectory(jrt_test) add_subdirectory(linguistics) add_subdirectory(linguistics-components) add_subdirectory(logd) -add_subdirectory(logserver) add_subdirectory(logforwarder) +add_subdirectory(logserver) add_subdirectory(lowercasing_test) add_subdirectory(messagebus) add_subdirectory(messagebus_test) @@ -128,22 +128,22 @@ add_subdirectory(tenant-cd-api) add_subdirectory(vbench) add_subdirectory(vdslib) add_subdirectory(vdstestlib) -add_subdirectory(vespa-athenz) -add_subdirectory(vespa-feed-client) -add_subdirectory(vespa-feed-client-cli) -add_subdirectory(vespa-osgi-testrunner) -add_subdirectory(vespa-testrunner-components) -add_subdirectory(vespa_feed_perf) add_subdirectory(vespa-3party-bundles) +add_subdirectory(vespa-athenz) add_subdirectory(vespabase) add_subdirectory(vespaclient) -add_subdirectory(vespaclient-core) add_subdirectory(vespaclient-container-plugin) +add_subdirectory(vespaclient-core) add_subdirectory(vespaclient-java) +add_subdirectory(vespa-feed-client) +add_subdirectory(vespa-feed-client-cli) +add_subdirectory(vespa_feed_perf) add_subdirectory(vespajlib) add_subdirectory(vespalib) add_subdirectory(vespalog) add_subdirectory(vespamalloc) +add_subdirectory(vespa-osgi-testrunner) +add_subdirectory(vespa-testrunner-components) add_subdirectory(zkfacade) add_subdirectory(zookeeper-command-line-client) add_subdirectory(zookeeper-server) -- cgit v1.2.3 From 6175529d752ee709e56fdf12cf1f6ff876647798 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 16 Aug 2022 12:33:51 +0200 Subject: Don't cache dynamic teaser. --- .../searchsummary/docsummary/dynamicteaserdfw.cpp | 59 ++++++++++------------ 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp index 024046b679b..d9878cd2057 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp @@ -375,40 +375,35 @@ DynamicTeaserDFW::makeDynamicTeaser(uint32_t docid, vespalib::stringref input, G state->_dynteaser._query = _juniper->CreateQueryHandle(iq, nullptr); } - if (docid != state->_dynteaser._docid || - _inputFieldEnumValue != state->_dynteaser._input || - _langFieldEnumValue != state->_dynteaser._lang || - !juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config)) { - LOG(debug, "makeDynamicTeaser: docid (%d,%d), fieldenum (%d,%d), lang (%d,%d) analyse %s", - docid, state->_dynteaser._docid, - _inputFieldEnumValue, state->_dynteaser._input, - _langFieldEnumValue, state->_dynteaser._lang, - (juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config) ? "no" : "yes")); - - if (state->_dynteaser._result != nullptr) - juniper::ReleaseResult(state->_dynteaser._result); - - state->_dynteaser._docid = docid; - state->_dynteaser._input = _inputFieldEnumValue; - state->_dynteaser._lang = _langFieldEnumValue; - state->_dynteaser._config = _juniperConfig.get(); - state->_dynteaser._result = nullptr; - - if (state->_dynteaser._query != nullptr) { - - if (LOG_WOULD_LOG(spam)) { - std::ostringstream hexDump; - hexDump << vespalib::HexDump(input.data(), input.length()); - LOG(spam, "makeDynamicTeaser: docid=%d, input='%s', hexdump:\n%s", - docid, std::string(input.data(), input.length()).c_str(), hexDump.str().c_str()); - } + LOG(debug, "makeDynamicTeaser: docid (%d,%d), fieldenum (%d,%d), lang (%d,%d) analyse %s", + docid, state->_dynteaser._docid, + _inputFieldEnumValue, state->_dynteaser._input, + _langFieldEnumValue, state->_dynteaser._lang, + (juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config) ? "no" : "yes")); + + if (state->_dynteaser._result != nullptr) + juniper::ReleaseResult(state->_dynteaser._result); + + state->_dynteaser._docid = docid; + state->_dynteaser._input = _inputFieldEnumValue; + state->_dynteaser._lang = _langFieldEnumValue; + state->_dynteaser._config = _juniperConfig.get(); + state->_dynteaser._result = nullptr; + + if (state->_dynteaser._query != nullptr) { + + if (LOG_WOULD_LOG(spam)) { + std::ostringstream hexDump; + hexDump << vespalib::HexDump(input.data(), input.length()); + LOG(spam, "makeDynamicTeaser: docid=%d, input='%s', hexdump:\n%s", + docid, std::string(input.data(), input.length()).c_str(), hexDump.str().c_str()); + } - auto langid = static_cast(-1); + auto langid = static_cast(-1); - state->_dynteaser._result = - juniper::Analyse(_juniperConfig.get(), state->_dynteaser._query, - input.data(), input.length(), docid, _inputFieldEnumValue, langid); - } + state->_dynteaser._result = + juniper::Analyse(_juniperConfig.get(), state->_dynteaser._query, + input.data(), input.length(), docid, _inputFieldEnumValue, langid); } juniper::Summary *teaser = (state->_dynteaser._result != nullptr) -- cgit v1.2.3 From 138b5682bf247dfdf52068b42a681ca9d8d5803d Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 16 Aug 2022 10:51:58 +0000 Subject: Add and test uri detection. --- .../src/tests/index/docbuilder/docbuilder_test.cpp | 41 ++++++++++------------ searchlib/src/vespa/searchlib/index/uri_field.cpp | 26 ++++++++------ searchlib/src/vespa/searchlib/index/uri_field.h | 1 + 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp b/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp index 75cccb0d573..9f1027d0522 100644 --- a/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp +++ b/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp @@ -13,8 +13,7 @@ LOG_SETUP("docbuilder_test"); using namespace document; using search::index::schema::CollectionType; -namespace search { -namespace index { +namespace search::index { namespace { @@ -26,15 +25,8 @@ namespace linguistics const vespalib::string SPANTREE_NAME("linguistics"); } -class Test : public vespalib::TestApp { -private: - void testBuilder(); -public: - int Main() override; -}; -void -Test::testBuilder() +TEST("test docBuilder") { Schema s; s.addIndexField(Schema::IndexField("ia", schema::DataType::STRING)); @@ -415,7 +407,7 @@ Test::testBuilder() EXPECT_EQUAL("", *itr++); EXPECT_EQUAL("", *itr++); EXPECT_TRUE(itr == lines.end()); -#if 1 +#if 0 std::cout << "onedoc xml start -----" << std::endl << xml << std::endl << "-------" << std::endl; @@ -479,7 +471,7 @@ Test::testBuilder() expSpans.push_back(Span(15, 9)); expSpans.push_back(Span(15, 9)); ASSERT_TRUE(expSpans == spans); -#if 1 +#if 0 std::cout << "onedoc xml start -----" << std::endl << xml << std::endl << "-------" << std::endl; @@ -490,18 +482,21 @@ Test::testBuilder() } } -int -Test::Main() -{ - TEST_INIT("docbuilder_test"); - - testBuilder(); - - TEST_DONE(); +TEST("test if index names are valid uri parts") { + EXPECT_FALSE(UriField::mightBePartofUri("all")); + EXPECT_FALSE(UriField::mightBePartofUri("fragment")); + EXPECT_FALSE(UriField::mightBePartofUri(".all")); + EXPECT_FALSE(UriField::mightBePartofUri("all.b")); + EXPECT_TRUE(UriField::mightBePartofUri("b.all")); + EXPECT_TRUE(UriField::mightBePartofUri("b.scheme")); + EXPECT_TRUE(UriField::mightBePartofUri("b.host")); + EXPECT_TRUE(UriField::mightBePartofUri("b.port")); + EXPECT_TRUE(UriField::mightBePartofUri("b.hostname")); + EXPECT_TRUE(UriField::mightBePartofUri("b.path")); + EXPECT_TRUE(UriField::mightBePartofUri("b.query")); + EXPECT_TRUE(UriField::mightBePartofUri("b.fragment")); } } -} - -TEST_APPHOOK(search::index::Test); +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/index/uri_field.cpp b/searchlib/src/vespa/searchlib/index/uri_field.cpp index daf0e6e685e..070afc94837 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.cpp +++ b/searchlib/src/vespa/searchlib/index/uri_field.cpp @@ -18,9 +18,7 @@ UriField::UriField() } bool -UriField::valid(const Schema &schema, - uint32_t fieldId, - const Schema::CollectionType &collectionType) +UriField::valid(const Schema &schema, uint32_t fieldId, const Schema::CollectionType &collectionType) { if (fieldId == Schema::UNKNOWN_FIELD_ID) { return false; @@ -36,9 +34,7 @@ UriField::valid(const Schema &schema, } bool -UriField::broken(const Schema &schema, - const Schema::CollectionType & - collectionType) const +UriField::broken(const Schema &schema, const Schema::CollectionType & collectionType) const { return !valid(schema, _all, collectionType) && valid(schema, _scheme, collectionType) && @@ -50,9 +46,7 @@ UriField::broken(const Schema &schema, } bool -UriField::valid(const Schema &schema, - const Schema::CollectionType & - collectionType) const +UriField::valid(const Schema &schema, const Schema::CollectionType & collectionType) const { return valid(schema, _all, collectionType) && valid(schema, _scheme, collectionType) && @@ -64,8 +58,7 @@ UriField::valid(const Schema &schema, } void -UriField::setup(const Schema &schema, - const vespalib::string &field) +UriField::setup(const Schema &schema, const vespalib::string &field) { _all = schema.getIndexFieldId(field); _scheme = schema.getIndexFieldId(field + ".scheme"); @@ -77,6 +70,17 @@ UriField::setup(const Schema &schema, _hostname = schema.getIndexFieldId(field + ".hostname"); } +bool +UriField::mightBePartofUri(vespalib::stringref name) { + size_t dotPos = name.find('.'); + if ((dotPos != 0) && (dotPos != vespalib::string::npos)) { + vespalib::stringref suffix = name.substr(dotPos + 1); + return ((suffix == "all") || (suffix == "scheme") || (suffix == "host") || (suffix == "port") || + (suffix == "path") || (suffix == "query") || (suffix == "fragment") || (suffix == "hostname")); + } + return false; +} + void UriField::markUsed(UsedFieldsMap &usedFields, uint32_t field) { diff --git a/searchlib/src/vespa/searchlib/index/uri_field.h b/searchlib/src/vespa/searchlib/index/uri_field.h index 9b8e4e72b7c..70bf8c01a8c 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.h +++ b/searchlib/src/vespa/searchlib/index/uri_field.h @@ -35,6 +35,7 @@ public: bool valid(const Schema &schema, const Schema::CollectionType &collectionType) const; void setup(const Schema &schema, const vespalib::string &field); void markUsed(UsedFieldsMap &usedFields) const; + static bool mightBePartofUri(vespalib::stringref name); }; } -- cgit v1.2.3 From 6cc84e06d7d24e68c83ba5a3550e64d9fb917096 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 16 Aug 2022 12:58:33 +0200 Subject: Revert "Merge pull request #23669 from vespa-engine/revert-23581-userinput-does-not-use-defaultIndex-while-resolving-indexableitem" This reverts commit d8a38777428495670078d23cd86863829ca8018d, reversing changes made to aa89c25d0ed2e8b0390ef62c6e55feecdd4c7815. --- .../yahoo/prelude/query/parser/AbstractParser.java | 6 ++-- .../yahoo/prelude/query/parser/AdvancedParser.java | 2 +- .../com/yahoo/prelude/query/parser/AllParser.java | 8 +++--- .../com/yahoo/prelude/query/parser/AnyParser.java | 4 +-- .../yahoo/prelude/query/parser/PhraseParser.java | 2 +- .../yahoo/prelude/query/parser/SimpleParser.java | 12 +++++--- .../prelude/query/parser/StructuredParser.java | 8 ++++++ .../yahoo/prelude/query/parser/TokenizeParser.java | 2 +- .../com/yahoo/prelude/query/parser/WebParser.java | 4 +-- .../com/yahoo/search/yql/UserInputTestCase.java | 33 +++++++++++++++++++++- 10 files changed, 62 insertions(+), 19 deletions(-) diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 586d1d32d57..55001ae5915 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -143,7 +143,7 @@ public abstract class AbstractParser implements CustomParser { } setState(parsingLanguage, indexFacts); - Item root = parseItems(); + Item root = parseItems(defaultIndexName); if (filterToParse != null) { AnyParser filterParser = new AnyParser(environment); if (root == null) { @@ -222,8 +222,8 @@ public abstract class AbstractParser implements CustomParser { if (tokenOrNull == null) return false; return kind.equals(tokenOrNull.kind); } - - protected abstract Item parseItems(); + + protected abstract Item parseItems(String defaultIndexName); /** * Assigns the default index to query terms having no default index. The diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java index 690fc67af7e..3358075d670 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java @@ -20,7 +20,7 @@ public class AdvancedParser extends StructuredParser { super(environment); } - protected Item parseItems() { + protected Item parseItems(String defaultIndexName) { return advancedItems(true); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java index 545bb8e777f..09caa72ca59 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java @@ -41,16 +41,16 @@ public class AllParser extends SimpleParser { } @Override - protected Item parseItems() { + protected Item parseItems(String defaultIndexName) { int position = tokens.getPosition(); try { - return parseItemsBody(); + return parseItemsBody(defaultIndexName); } finally { tokens.setPosition(position); } } - protected Item parseItemsBody() { + protected Item parseItemsBody(String defaultIndexName) { // Algorithm: Collect positive, negative, and and'ed items, then combine. CompositeItem and = null; NotItem not = null; // Store negatives here as we go @@ -65,7 +65,7 @@ public class AllParser extends SimpleParser { current = positiveItem(); if (current == null) - current = indexableItem(); + current = indexableItem(defaultIndexName); if (current == null) current = compositeItem(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java index e22043c6b8f..f4ff769ad05 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java @@ -31,8 +31,8 @@ public class AnyParser extends SimpleParser { super(environment); } - protected Item parseItems() { - return anyItems(true); + protected Item parseItems(String defaultIndexName) { + return anyItems(true, defaultIndexName); } Item parseFilter(String filter, Language queryLanguage, IndexFacts.Session indexFacts) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java index 75edf9fbf5c..72eb56dd0fb 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java @@ -16,7 +16,7 @@ public class PhraseParser extends AbstractParser { super(environment); } - protected Item parseItems() { + protected Item parseItems(String defaultIndex) { return forcedPhrase(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index 27bce6bd027..fafbf55a522 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -33,12 +33,12 @@ abstract class SimpleParser extends StructuredParser { * If there's a explicit composite and some other terms, * a rank terms combines them */ - protected Item anyItems(boolean topLevel) { + protected Item anyItems(boolean topLevel, String defaultIndexName) { int position = tokens.getPosition(); Item item = null; try { - item = anyItemsBody(topLevel); + item = anyItemsBody(topLevel, defaultIndexName); return item; } finally { if (item == null) { @@ -47,7 +47,11 @@ abstract class SimpleParser extends StructuredParser { } } - private Item anyItemsBody(boolean topLevel) { + protected Item anyItems(boolean topLevel) { + return anyItems(topLevel, null); + } + + private Item anyItemsBody(boolean topLevel, String defaultIndexName) { Item topLevelItem = null; NotItem not = null; Item item = null; @@ -88,7 +92,7 @@ abstract class SimpleParser extends StructuredParser { } if (item == null) { - item = indexableItem(); + item = indexableItem(defaultIndexName); if (item != null) { if (topLevelItem == null) { topLevelItem = item; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index f993c7a9e02..c668cf66447 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -7,6 +7,7 @@ import com.yahoo.search.query.parser.ParserEnvironment; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import static com.yahoo.prelude.query.parser.Token.Kind.*; @@ -52,11 +53,18 @@ abstract class StructuredParser extends AbstractParser { } protected Item indexableItem() { + return indexableItem(null); + } + + protected Item indexableItem(String defaultIndexName) { int position = tokens.getPosition(); Item item = null; try { String indexName = indexPrefix(); + if (Objects.isNull(indexName)) { + indexName = defaultIndexName; + } setSubmodeFromIndex(indexName, indexFacts); item = number(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java index dbbc321d057..eefbe5fa0d0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java @@ -22,7 +22,7 @@ public final class TokenizeParser extends AbstractParser { } @Override - protected Item parseItems() { + protected Item parseItems(String defaultIndex) { WeakAndItem weakAnd = new WeakAndItem(); Token token; while (null != (token = tokens.next())) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java index d7c7dec4798..40497d94a6d 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java @@ -28,7 +28,7 @@ public class WebParser extends AllParser { } @Override - protected Item parseItemsBody() { + protected Item parseItemsBody(String defaultIndexName) { // Algorithm: Collect positive, negative, and'ed and or'ed elements, then combine. CompositeItem and = null; OrItem or = null; @@ -45,7 +45,7 @@ public class WebParser extends AllParser { current = positiveItem(); if (current == null) - current = indexableItem(); + current = indexableItem(defaultIndexName); if (current != null) { if (and != null && (current instanceof WordItem) && "OR".equals(((WordItem)current).getRawWord())) { diff --git a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java index 8fe451dd095..1e3b52c23af 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java @@ -3,7 +3,10 @@ package com.yahoo.search.yql; import static org.junit.jupiter.api.Assertions.*; -import com.yahoo.search.query.QueryTree; +import com.yahoo.prelude.Index; +import com.yahoo.prelude.IndexFacts; +import com.yahoo.prelude.IndexModel; +import com.yahoo.prelude.SearchDefinition; import org.apache.http.client.utils.URIBuilder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -367,4 +370,32 @@ public class UserInputTestCase { assertEquals("select * from sources * where text_field contains \"boom\"", query.yqlRepresentation()); } + @Test + void testUserInputWithPhraseSegmentingIndex() { + execution = new Execution(searchChain, Execution.Context.createContextStub(createIndexFacts(true))); + URIBuilder builder = searchUri(); + builder.setParameter("wql", "foo&bar"); + builder.setParameter("yql", "select * from sources * where ([{\"defaultIndex\": \"text_field\",\"grammar\": \"any\"}]userInput(@wql))"); + Query query = searchAndAssertNoErrors(builder); + assertEquals("select * from sources * where text_field contains phrase(\"foo\", \"bar\")", query.yqlRepresentation()); + } + + @Test + void testUserInputWithNonPhraseSegmentingIndex() { + execution = new Execution(searchChain, Execution.Context.createContextStub(createIndexFacts(false))); + URIBuilder builder = searchUri(); + builder.setParameter("wql", "foo&bar"); + builder.setParameter("yql", "select * from sources * where ([{\"defaultIndex\": \"text_field\",\"grammar\": \"any\"}]userInput(@wql))"); + Query query = searchAndAssertNoErrors(builder); + assertEquals("select * from sources * where (text_field contains \"foo\" AND text_field contains \"bar\")", query.yqlRepresentation()); + } + + private IndexFacts createIndexFacts(boolean phraseSegmenting) { + SearchDefinition sd = new SearchDefinition("sources"); + Index test = new Index("text_field"); + test.setPhraseSegmenting(phraseSegmenting); + sd.addIndex(test); + return new IndexFacts(new IndexModel(sd)); + } + } -- cgit v1.2.3 From 5d7a6831032badd54854e9ac261bc370e8e740e7 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 16 Aug 2022 13:52:12 +0200 Subject: Cannonicalize --- .../yahoo/prelude/query/parser/AbstractParser.java | 3 +++ .../java/com/yahoo/search/test/QueryTestCase.java | 30 +++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 55001ae5915..9f57512f657 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -133,6 +133,9 @@ public abstract class AbstractParser implements CustomParser { IndexFacts.Session indexFacts, String defaultIndexName, Parsable parsable) { if (queryToParse == null) return null; + if (defaultIndexName != null) + defaultIndexName = indexFacts.getCanonicName(defaultIndexName); + tokenize(queryToParse, defaultIndexName, indexFacts, parsingLanguage); if (parsingLanguage == null && parsable != null) { diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index f8a77445a40..a722552b7d2 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java @@ -452,10 +452,34 @@ public class QueryTestCase { @Test void testDefaultIndex() { - Query q = new Query("?query=hi%20hello%20keyword:kanoo%20" + - "default:munkz%20%22phrases+too%22&default-index=def"); + Query q = new Query("?query=hi%20hello%20keyword:kanoo%20default:munkz%20%22phrases+too%22&default-index=def"); assertEquals("WEAKAND(100) def:hi def:hello keyword:kanoo default:munkz def:\"phrases too\"", - q.getModel().getQueryTree().toString()); + q.getModel().getQueryTree().toString()); + } + + @Test + void testDefaultIndexAlias() { + SearchDefinition test = new SearchDefinition("test"); + Index year = new Index("year"); + year.setNumerical(true); + year.addAlias("yearalias"); + test.addIndex(year); + test.addAlias("yearalias", "year"); + IndexModel indexModel = new IndexModel(test); + + { + Query q = new Query("?default-index=year&type=all"); + q.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + q.getModel().setQueryString("2000"); + assertEquals("select * from sources * where year = 2000", q.yqlRepresentation()); + } + + { + Query q = new Query("?default-index=yearalias&type=all"); + q.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + q.getModel().setQueryString("2000"); + assertEquals("select * from sources * where year = 2000", q.yqlRepresentation()); + } } @Test -- cgit v1.2.3 From 37baffb971083c1bbeecc1001ec4ec1e41e4459f Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 15 Aug 2022 19:49:34 +0200 Subject: Revert "Revert "Revert "Revert "Use a hash_set to quickly check if a field i…"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../proton/documentdb/feedview/feedview_test.cpp | 2 +- .../proton/attribute/attribute_writer.cpp | 13 ++-- .../proton/attribute/ifieldupdatecallback.h | 7 +- .../proton/server/searchable_feed_view.cpp | 2 +- .../searchcore/proton/server/storeonlyfeedview.cpp | 83 ++++++++++++++-------- .../searchcore/proton/server/storeonlyfeedview.h | 24 ++----- 6 files changed, 70 insertions(+), 61 deletions(-) diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index 2957a2a015d..9d9bb64e6b1 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -367,7 +367,7 @@ struct MyAttributeWriter : public IAttributeWriter _updateLid = lid; for (const auto & fieldUpdate : upd.getUpdates()) { search::AttributeVector * attr = getWritableAttribute(fieldUpdate.getField().getName()); - onUpdate.onUpdateField(fieldUpdate.getField().getName(), attr); + onUpdate.onUpdateField(fieldUpdate.getField(), attr); } } void update(SerialNum serialNum, const document::Document &doc, DocumentIdT lid, OnWriteDoneType) override { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index bb25b3da7be..021fc4717af 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -764,9 +764,9 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document for (const auto &fupd : upd.getUpdates()) { LOG(debug, "Retrieving guard for attribute vector '%s'.", fupd.getField().getName().data()); - auto itr = _attrMap.find(fupd.getField().getName()); - AttributeVector * attrp = (itr != _attrMap.end()) ? itr->second.attribute : nullptr; - onUpdate.onUpdateField(fupd.getField().getName(), attrp); + auto found = _attrMap.find(fupd.getField().getName()); + AttributeVector * attrp = (found != _attrMap.end()) ? found->second.attribute : nullptr; + onUpdate.onUpdateField(fupd.getField(), attrp); if (__builtin_expect(attrp == nullptr, false)) { LOG(spam, "Failed to find attribute vector %s", fupd.getField().getName().data()); continue; @@ -776,16 +776,15 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document if (__builtin_expect(attrp->getStatus().getLastSyncToken() >= serialNum, false)) { continue; } - if (itr->second.use_two_phase_put_for_assign_updates && - is_single_assign_update(fupd)) { + if (found->second.use_two_phase_put_for_assign_updates && is_single_assign_update(fupd)) { auto prepare_task = std::make_unique(serialNum, lid, *attrp, get_single_assign_update_field_value(fupd)); auto complete_task = std::make_unique(*prepare_task, onWriteDone); LOG(debug, "About to handle assign update as two phase put for docid %u in attribute vector '%s'", lid, attrp->getName().c_str()); _shared_executor.execute(CpuUsage::wrap(std::move(prepare_task), CpuUsage::Category::WRITE)); - _attributeFieldWriter.executeTask(itr->second.executor_id, std::move(complete_task)); + _attributeFieldWriter.executeTask(found->second.executor_id, std::move(complete_task)); } else { - args[itr->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); + args[found->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); LOG(debug, "About to apply update for docId %u in attribute vector '%s'.", lid, attrp->getName().c_str()); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h index d8872607b44..d3ab970fb39 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h @@ -5,16 +5,17 @@ #include namespace search { class AttributeVector; } +namespace document { class Field; } namespace proton { struct IFieldUpdateCallback { - virtual ~IFieldUpdateCallback() { } - virtual void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) = 0; + virtual ~IFieldUpdateCallback() = default; + virtual void onUpdateField(const document::Field & field, const search::AttributeVector * attr) = 0; }; struct DummyFieldUpdateCallback : IFieldUpdateCallback { - void onUpdateField(vespalib::stringref, const search::AttributeVector *) override {} + void onUpdateField(const document::Field & , const search::AttributeVector *) override {} }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp index 207f1d813d8..7a78b4ba82a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp @@ -34,7 +34,7 @@ SearchableFeedView::SearchableFeedView(StoreOnlyFeedView::Context storeOnlyCtx, const FastAccessFeedView::Context &fastUpdateCtx, Context ctx) : Parent(std::move(storeOnlyCtx), params, fastUpdateCtx), _indexWriter(ctx._indexWriter), - _hasIndexedFields(_schema->getNumIndexFields() > 0) + _hasIndexedFields(getSchema()->getNumIndexFields() > 0) { } SearchableFeedView::~SearchableFeedView() = default; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index a537742b79b..d7c3246cb4b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -7,14 +7,14 @@ #include "putdonecontext.h" #include "removedonecontext.h" #include "updatedonecontext.h" -#include -#include -#include #include #include #include #include #include +#include +#include +#include #include #include #include @@ -88,8 +88,8 @@ SummaryPutDoneContext::SummaryPutDoneContext(FeedToken token, IPendingLidTracker SummaryPutDoneContext::~SummaryPutDoneContext() = default; -std::vector getGidsToRemove(const IDocumentMetaStore &metaStore, - const LidVectorContext::LidVector &lidsToRemove) +std::vector +getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::LidVector &lidsToRemove) { std::vector gids; gids.reserve(lidsToRemove.size()); @@ -102,8 +102,9 @@ std::vector getGidsToRemove(const IDocumentMetaStore &metaSt return gids; } -void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, + const DocumentOperation &op, bool is_removed_doc) { documentmetastore::IStore::Result putRes( meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(), @@ -117,8 +118,9 @@ void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_i assert(op.getLid() == putRes._lid); } -void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, + const DocumentOperation &op, bool is_removed_doc) { assert(meta_store.validLid(op.getPrevLid())); assert(is_removed_doc == op.getPrevMarkedAsRemoved()); @@ -147,6 +149,37 @@ moveMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, c meta_store.move(op.getPrevLid(), op.getLid(), op.get_prepare_serial_num()); } +class UpdateScope final : public IFieldUpdateCallback +{ +private: + const vespalib::hash_set & _indexedFields; + bool _nonAttributeFields; +public: + bool _hasIndexedFields; + + UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd); + bool hasIndexOrNonAttributeFields() const { + return _hasIndexedFields || _nonAttributeFields; + } + void onUpdateField(const document::Field & field, const search::AttributeVector * attr) override; +}; + +UpdateScope::UpdateScope(const vespalib::hash_set & indexedFields, const DocumentUpdate & upd) + : _indexedFields(indexedFields), + _nonAttributeFields(!upd.getFieldPathUpdates().empty()), + _hasIndexedFields(false) +{} + +void +UpdateScope::onUpdateField(const document::Field & field, const search::AttributeVector * attr) { + if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { + _nonAttributeFields = true; + } + if (!_hasIndexedFields && (_indexedFields.find(field.getId()) != _indexedFields.end())) { + _hasIndexedFields = true; + } +} + } // namespace StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms) @@ -160,12 +193,20 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms _pendingLidsForDocStore(), _pendingLidsForCommit(std::move(ctx._pendingLidsForCommit)), _schema(std::move(ctx._schema)), + _indexedFields(), _writeService(ctx._writeService), _params(params), _metaStore(_documentMetaStoreContext->get()), _gidToLidChangeHandler(ctx._gidToLidChangeHandler) { _docType = _repo->getDocumentType(_params._docTypeName.getName()); + if (_schema && _docType) { + for (const auto &indexField : _schema->getIndexFields()) { + document::FieldPath fieldPath; + _docType->buildFieldPath(fieldPath, indexField.getName()); + _indexedFields.insert(fieldPath.back().getFieldRef().getId()); + } + } } StoreOnlyFeedView::~StoreOnlyFeedView() = default; @@ -207,7 +248,7 @@ void StoreOnlyFeedView::putAttributes(SerialNum, Lid, const Document &, OnPutDoneType) {} void -StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const Document::SP &, OnOperationDoneType) {} +StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const std::shared_ptr &, OnOperationDoneType) {} void StoreOnlyFeedView::preparePut(PutOperation &putOp) @@ -285,7 +326,7 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, const DocumentUpdate & upd, OnOperationDoneType, IFieldUpdateCallback & onUpdate) { for (const auto & fieldUpdate : upd.getUpdates()) { - onUpdate.onUpdateField(fieldUpdate.getField().getName(), nullptr); + onUpdate.onUpdateField(fieldUpdate.getField(), nullptr); } } @@ -387,22 +428,6 @@ StoreOnlyFeedView::heartBeatSummary(SerialNum serialNum, DoneCallback onDone) { })); } -StoreOnlyFeedView::UpdateScope::UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd) - : _schema(&schema), - _indexedFields(false), - _nonAttributeFields(!upd.getFieldPathUpdates().empty()) -{} - -void -StoreOnlyFeedView::UpdateScope::onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) { - if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { - _nonAttributeFields = true; - } - if (!_indexedFields && _schema->isIndexField(fieldName)) { - _indexedFields = true; - } -} - void StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) { if ( ! updOp.getUpdate()) { @@ -432,7 +457,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) } auto onWriteDone = createUpdateDoneContext(std::move(token), get_pending_lid_token(updOp), updOp.getUpdate()); - UpdateScope updateScope(*_schema, upd); + UpdateScope updateScope(_indexedFields, upd); updateAttributes(serialNum, lid, upd, onWriteDone, updateScope); if (updateScope.hasIndexOrNonAttributeFields()) { @@ -440,7 +465,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) FutureDoc futureDoc = promisedDoc.get_future().share(); onWriteDone->setDocument(futureDoc); _pendingLidsForDocStore.waitComplete(lid); - if (updateScope._indexedFields) { + if (updateScope._hasIndexedFields) { updateIndexedFields(serialNum, lid, futureDoc, onWriteDone); } PromisedStream promisedStream; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index 2822aa70525..25a98da7ce7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -9,7 +9,6 @@ #include "searchcontext.h" #include #include -#include #include #include #include @@ -18,9 +17,10 @@ #include #include #include +#include #include +#include #include -#include namespace vespalib { class IDestructorCallback; } @@ -118,22 +118,6 @@ public: {} }; -protected: - class UpdateScope : public IFieldUpdateCallback - { - private: - const search::index::Schema *_schema; - public: - bool _indexedFields; - bool _nonAttributeFields; - - UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd); - bool hasIndexOrNonAttributeFields() const { - return _indexedFields || _nonAttributeFields; - } - void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) override; - }; - private: const ISummaryAdapter::SP _summaryAdapter; const IDocumentMetaStoreContext::SP _documentMetaStoreContext; @@ -142,9 +126,9 @@ private: LidReuseDelayer _lidReuseDelayer; PendingLidTracker _pendingLidsForDocStore; std::shared_ptr _pendingLidsForCommit; - + const search::index::Schema::SP _schema; + vespalib::hash_set _indexedFields; protected: - const search::index::Schema::SP _schema; searchcorespi::index::IThreadingService &_writeService; PersistentParams _params; IDocumentMetaStore &_metaStore; -- cgit v1.2.3 From 8c09cc6af89251fda2e9f568a6d85e1442a330bd Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 16 Aug 2022 10:50:46 +0000 Subject: Properly handle URI fields by just looking at the prefix. --- .../searchcore/proton/server/storeonlyfeedview.cpp | 27 +++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index d7c3246cb4b..a9850b5c2b7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -38,7 +39,7 @@ using vespalib::CpuUsage; using vespalib::IDestructorCallback; using vespalib::IllegalStateException; using vespalib::makeLambdaTask; -using vespalib::make_string; +using vespalib::make_string_short::fmt; namespace proton { @@ -110,10 +111,9 @@ putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(), op.getSerializedDocSize(), op.getLid(), op.get_prepare_serial_num())); if (!putRes.ok()) { - throw IllegalStateException( - make_string("Could not put pair for %sdocument with id '%s' and gid '%s'", - is_removed_doc ? "removed " : "", doc_id.toString().c_str(), - doc_id.getGlobalId().toString().c_str())); + throw IllegalStateException(fmt("Could not put pair for %sdocument with id '%s' and gid '%s'", + is_removed_doc ? "removed " : "", doc_id.toString().c_str(), + doc_id.getGlobalId().toString().c_str())); } assert(op.getLid() == putRes._lid); } @@ -129,9 +129,8 @@ removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, cons (void) meta; if (!meta_store.remove(op.getPrevLid(), op.get_prepare_serial_num())) { throw IllegalStateException( - make_string("Could not remove pair for %sdocument with id '%s' and gid '%s'", - is_removed_doc ? "removed " : "", doc_id.toString().c_str(), - gid.toString().c_str())); + fmt("Could not remove pair for %sdocument with id '%s' and gid '%s'", + is_removed_doc ? "removed " : "", doc_id.toString().c_str(), gid.toString().c_str())); } } @@ -202,9 +201,15 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms _docType = _repo->getDocumentType(_params._docTypeName.getName()); if (_schema && _docType) { for (const auto &indexField : _schema->getIndexFields()) { - document::FieldPath fieldPath; - _docType->buildFieldPath(fieldPath, indexField.getName()); - _indexedFields.insert(fieldPath.back().getFieldRef().getId()); + size_t dotPos = indexField.getName().find('.'); + if ((dotPos == vespalib::string::npos) || search::index::UriField::mightBePartofUri(indexField.getName())) { + document::FieldPath fieldPath; + _docType->buildFieldPath(fieldPath, indexField.getName().substr(0, dotPos)); + _indexedFields.insert(fieldPath.back().getFieldRef().getId()); + } else { + throw IllegalStateException("Field '%s' is not a valid index name", indexField.getName().c_str()); + } + } } } -- cgit v1.2.3 From ee33d3dc01bd9c88f7a3c634358ba00a725d989d Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 16 Aug 2022 14:43:54 +0200 Subject: Include iostream to get declaration of std::cerr. --- ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp b/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp index 4af30a5d9d3..f9a4d5b43cc 100644 --- a/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp +++ b/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include -- cgit v1.2.3 From 2e4967745646eae351bd2bad9efe1e58dafc9be0 Mon Sep 17 00:00:00 2001 From: Arnstein Ressem Date: Tue, 16 Aug 2022 15:09:56 +0200 Subject: Release multi-arch images. --- screwdriver/release-container-image.sh | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/screwdriver/release-container-image.sh b/screwdriver/release-container-image.sh index 3ae2a7feb1e..def2f037b63 100755 --- a/screwdriver/release-container-image.sh +++ b/screwdriver/release-container-image.sh @@ -26,27 +26,35 @@ cd $BUILD_DIR git clone --depth 1 https://github.com/vespa-engine/docker-image cd docker-image -docker build --build-arg VESPA_VERSION=$VESPA_VERSION --file Dockerfile \ - --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest \ - --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . +docker info +docker version +docker buildx version +docker buildx install + +unset DOCKER_HOST +docker context create vespa-context --docker "host=tcp://localhost:2376,ca=/certs/client/ca.pem,cert=/certs/client/cert.pem,key=/certs/client/key.pem" +docker context use vespa-context + +docker buildx create --name vespa-builder --driver docker-container --use +docker buildx inspect --bootstrap # Push to Docker Hub -docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" if curl -fsSL https://index.docker.io/v1/repositories/vespaengine/vespa/tags/$VESPA_VERSION &> /dev/null; then echo "Container image docker.io/vespaengine/vespa:$VESPA_VERSION aldready exists." else - docker push docker.io/vespaengine/vespa:$VESPA_VERSION - docker push docker.io/vespaengine/vespa:latest + docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest . fi # Push to GitHub Container Registry -docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io JWT=$(curl -sSL -u aressem:$GHCR_DEPLOY_KEY "https://ghcr.io/token?service=ghcr.io&scope=repository:vespa-engine/vespa:pull" | jq -re '.token') IMAGE_TAGS=$(curl -sSL -H "Authorization: Bearer $JWT" https://ghcr.io/v2/vespa-engine/vespa/tags/list | jq -re '.tags[]') if grep $VESPA_VERSION <<< "$IMAGE_TAGS" &> /dev/null; then echo "Container image ghcr.io/vespa-engine/vespa:$VESPA_VERSION aldready exists." else - docker push ghcr.io/vespa-engine/vespa:$VESPA_VERSION - docker push ghcr.io/vespa-engine/vespa:latest + docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . fi -- cgit v1.2.3 From ebf0b7e6b37a0674da370c5fe0057f1aeac1d5a0 Mon Sep 17 00:00:00 2001 From: Arne H Juul Date: Tue, 16 Aug 2022 15:33:42 +0200 Subject: Revert "Arnej/use go logfmt" --- CMakeLists.txt | 37 ++++++++++++++++++------------------- client/CMakeLists.txt | 14 -------------- client/go/.gitignore | 1 - client/go/Makefile | 3 +-- vespalog/CMakeLists.txt | 1 + 5 files changed, 20 insertions(+), 36 deletions(-) delete mode 100644 client/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 8972fada5bf..4fbc756236d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,21 +51,15 @@ add_subdirectory(ann_benchmark) add_subdirectory(application-model) add_subdirectory(application-preprocessor) add_subdirectory(athenz-identity-provider-service) -add_subdirectory(client) -add_subdirectory(cloud-tenant-cd) -add_subdirectory(clustercontroller-apps) -add_subdirectory(clustercontroller-core) -add_subdirectory(clustercontroller-reindexer) -add_subdirectory(clustercontroller-utils) -add_subdirectory(config) add_subdirectory(config-bundle) -add_subdirectory(configd) -add_subdirectory(configdefinitions) add_subdirectory(config-model) add_subdirectory(config-model-api) -add_subdirectory(config-model-fat) add_subdirectory(config-provisioning) add_subdirectory(config-proxy) +add_subdirectory(config) +add_subdirectory(config-model-fat) +add_subdirectory(configd) +add_subdirectory(configdefinitions) add_subdirectory(configserver) add_subdirectory(configserver-flags) add_subdirectory(configutil) @@ -74,9 +68,14 @@ add_subdirectory(container-core) add_subdirectory(container-disc) add_subdirectory(container-messagebus) add_subdirectory(container-search) -add_subdirectory(container-search-and-docproc) add_subdirectory(container-search-gui) +add_subdirectory(container-search-and-docproc) add_subdirectory(container-spifly) +add_subdirectory(cloud-tenant-cd) +add_subdirectory(clustercontroller-apps) +add_subdirectory(clustercontroller-core) +add_subdirectory(clustercontroller-reindexer) +add_subdirectory(clustercontroller-utils) add_subdirectory(defaults) add_subdirectory(docproc) add_subdirectory(docprocs) @@ -99,8 +98,8 @@ add_subdirectory(jrt_test) add_subdirectory(linguistics) add_subdirectory(linguistics-components) add_subdirectory(logd) -add_subdirectory(logforwarder) add_subdirectory(logserver) +add_subdirectory(logforwarder) add_subdirectory(lowercasing_test) add_subdirectory(messagebus) add_subdirectory(messagebus_test) @@ -128,22 +127,22 @@ add_subdirectory(tenant-cd-api) add_subdirectory(vbench) add_subdirectory(vdslib) add_subdirectory(vdstestlib) -add_subdirectory(vespa-3party-bundles) add_subdirectory(vespa-athenz) +add_subdirectory(vespa-feed-client) +add_subdirectory(vespa-feed-client-cli) +add_subdirectory(vespa-osgi-testrunner) +add_subdirectory(vespa-testrunner-components) +add_subdirectory(vespa_feed_perf) +add_subdirectory(vespa-3party-bundles) add_subdirectory(vespabase) add_subdirectory(vespaclient) -add_subdirectory(vespaclient-container-plugin) add_subdirectory(vespaclient-core) +add_subdirectory(vespaclient-container-plugin) add_subdirectory(vespaclient-java) -add_subdirectory(vespa-feed-client) -add_subdirectory(vespa-feed-client-cli) -add_subdirectory(vespa_feed_perf) add_subdirectory(vespajlib) add_subdirectory(vespalib) add_subdirectory(vespalog) add_subdirectory(vespamalloc) -add_subdirectory(vespa-osgi-testrunner) -add_subdirectory(vespa-testrunner-components) add_subdirectory(zkfacade) add_subdirectory(zookeeper-command-line-client) add_subdirectory(zookeeper-server) diff --git a/client/CMakeLists.txt b/client/CMakeLists.txt deleted file mode 100644 index 1ca6d2b4e60..00000000000 --- a/client/CMakeLists.txt +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -set(GODIR ${CMAKE_CURRENT_SOURCE_DIR}/go) - -file(GLOB_RECURSE GOSRCFILES ${GODIR}/*.go) - -add_custom_command(OUTPUT ${GODIR}/bin/vespa-logfmt - COMMAND make - DEPENDS ${GODIR}/Makefile ${GOSRCFILES} - WORKING_DIRECTORY ${GODIR}) - -add_custom_target(vespalog_logfmt ALL DEPENDS ${GODIR}/bin/vespa-logfmt) - -install(PROGRAMS ${GODIR}/bin/vespa-logfmt DESTINATION bin) diff --git a/client/go/.gitignore b/client/go/.gitignore index baab7c638c6..8933bc220cb 100644 --- a/client/go/.gitignore +++ b/client/go/.gitignore @@ -6,4 +6,3 @@ share/ !target/ mytestapp/ mytestapp-cache/ -mytmp/ diff --git a/client/go/Makefile b/client/go/Makefile index 8a07f880c24..78adf299f0e 100644 --- a/client/go/Makefile +++ b/client/go/Makefile @@ -131,8 +131,7 @@ clean: rmdir -p $(BIN) $(SHARE)/man/man1 > /dev/null 2>&1 || true test: ci - mkdir -p mytmp - TMPDIR=`pwd`/mytmp go test ./... + go test ./... checkfmt: @bash -c "diff --line-format='%L' <(echo -n) <(gofmt -l .)" || { echo "one or more files need to be formatted: try make fmt to fix this automatically"; exit 1; } diff --git a/vespalog/CMakeLists.txt b/vespalog/CMakeLists.txt index cc419681445..45410a1d29d 100644 --- a/vespalog/CMakeLists.txt +++ b/vespalog/CMakeLists.txt @@ -17,5 +17,6 @@ vespa_define_module( src/test/threads ) +vespa_install_script(src/vespa-logfmt/vespa-logfmt.pl vespa-logfmt bin) install(FILES src/vespa-logfmt/vespa-logfmt.1 DESTINATION man/man1) install(DIRECTORY DESTINATION var/db/vespa/logcontrol) -- cgit v1.2.3 From f7a337c3c9b169868a6f911270e9d288dc968636 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 17 Aug 2022 08:19:27 +0200 Subject: Add session id to response for prepare and activate requests Adding session id, as documented in the API docs --- .../http/v2/response/SessionActiveResponse.java | 1 + .../SessionPrepareAndActivateResponse.java | 1 + .../http/v2/response/SessionPrepareResponse.java | 1 + .../server/http/v2/SessionActiveHandlerTest.java | 5 ++++- .../server/http/v2/SessionPrepareHandlerTest.java | 23 ++++++++-------------- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java index 55e640d4ec7..0c8c8946cbc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java @@ -18,6 +18,7 @@ public class SessionActiveResponse extends SlimeJsonResponse { Cursor root = metaData.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("message", message); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java index e436675fb59..617df3868ab 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java @@ -25,6 +25,7 @@ public class SessionPrepareAndActivateResponse extends SlimeJsonResponse { Cursor root = slime.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(result.sessionId())); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + "/application/" + applicationId.application().value() + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java index 563f50d0012..ac745f833f9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java @@ -31,6 +31,7 @@ public class SessionPrepareResponse extends SlimeJsonResponse { Cursor root = deployLog.get().type() != Type.NIX ? deployLog.get() : deployLog.setObject(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("activate", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName.value() + "/session/" + sessionId + "/active"); root.setString("message", "Session " + sessionId + " for tenant '" + tenantName.value() + "' prepared."); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 8638a29cf75..1c71ef0b7fb 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -155,7 +155,10 @@ public class SessionActiveHandlerTest { private void assertActivationMessageOK(ActivateRequest activateRequest, String message) throws IOException { ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); new JsonFormat(true).encode(byteArrayOutputStream, activateRequest.getMetaData().getSlime()); - assertTrue(message.contains("\"tenant\":\"" + tenantName + "\",\"message\":\"Session " + activateRequest.getSessionId() + activatedMessage)); + long sessionId = activateRequest.getSessionId(); + assertTrue(message.contains("\"tenant\":\"" + tenantName)); + assertTrue(message.contains("\"session-id\":\"" + sessionId)); + assertTrue(message.contains("\"message\":\"Session " + sessionId + activatedMessage)); assertTrue(message.contains("/application/v2/tenant/" + tenantName + "/application/" + appName + "/environment/" + "prod" + diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index 66da009946e..2b07cffffce 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -61,7 +61,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { private ConfigserverConfig configserverConfig; private String preparedMessage = " prepared.\"}"; - private String tenantMessage = ""; private TenantRepository tenantRepository; @Rule @@ -90,7 +89,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { .build(); pathPrefix = "/application/v2/tenant/" + tenant + "/session/"; preparedMessage = " for tenant '" + tenant + "' prepared.\""; - tenantMessage = ",\"tenant\":\"" + tenant + "\""; } @Test @@ -123,13 +121,16 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void require_that_activate_url_is_returned_on_success() throws Exception { + public void require_that_response_has_all_fields() throws Exception { long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertNotNull(response); assertEquals(OK, response.getStatus()); - assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + - "/active\",\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + "/active\""); + assertResponseContains(response, "\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"tenant\":\"" + tenant + "\""); + assertResponseContains(response, "\"session-id\":\"" + sessionId + "\""); + assertResponseContains(response, "\"log\":[]"); } @Test @@ -185,15 +186,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { "Session 9999 was not found"); } - @Test - public void require_that_tenant_is_in_response() throws Exception { - long sessionId = createSession(applicationId()); - HttpResponse response = request(HttpRequest.Method.PUT, sessionId); - assertNotNull(response); - assertEquals(OK, response.getStatus()); - assertResponseContains(response, tenantMessage); - } - @Test public void require_that_preparing_with_multiple_tenants_work() throws Exception { SessionHandler handler = createHandler(); @@ -307,7 +299,8 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } private static void assertResponseContains(HttpResponse response, String string) throws IOException { - assertTrue(SessionHandlerTest.getRenderedString(response).contains(string)); + String s = SessionHandlerTest.getRenderedString(response); + assertTrue(s, s.contains(string)); } private static void assertResponseNotContains(HttpResponse response, String string) throws IOException { -- cgit v1.2.3