diff options
43 files changed, 360 insertions, 463 deletions
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/OffsetContinuation.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/OffsetContinuation.java index c14f391d1f9..dc49d5859c8 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/OffsetContinuation.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/OffsetContinuation.java @@ -37,7 +37,7 @@ class OffsetContinuation extends EncodableContinuation { return offset; } - public int getMetaEnum() { + public int getFlags() { return flags; } diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/OffsetContinuationTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/OffsetContinuationTestCase.java index 32120cb9ed8..7242e5ba054 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/OffsetContinuationTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/OffsetContinuationTestCase.java @@ -33,7 +33,7 @@ public class OffsetContinuationTestCase { assertEquals(ResultId.valueOf(5), cnt.getResultId()); assertEquals(6, cnt.getTag()); assertEquals(7, cnt.getOffset()); - assertEquals(8, cnt.getMetaEnum()); + assertEquals(8, cnt.getFlags()); for (int i = 0; i < 30; ++i) { cnt = new OffsetContinuation(ResultId.valueOf(1), 2, 3, (1 << i) + (1 << i + 1)); diff --git a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp index 3fa1a8a9b8d..21cf7cb9ae3 100644 --- a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp +++ b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp @@ -9,6 +9,7 @@ #include <vespa/vdslib/state/clusterstate.h> #include <vespa/config-stor-distribution.h> + using namespace storage::spi; using namespace storage; using document::test::makeBucketSpace; @@ -19,14 +20,14 @@ namespace { struct Fixture { BucketContent content; - void insert(DocumentId id, Timestamp timestamp, DocumentMetaEnum meta_flags) { - content.insert(DocEntry::create(timestamp, meta_flags, id)); + void insert(DocumentId id, Timestamp timestamp, int meta_flags) { + content.insert(DocEntry::UP(new DocEntry(timestamp, meta_flags, id))); } Fixture() { - insert(DocumentId("id:ns:type::test:3"), Timestamp(3), DocumentMetaEnum::NONE); - insert(DocumentId("id:ns:type::test:1"), Timestamp(1), DocumentMetaEnum::NONE); - insert(DocumentId("id:ns:type::test:2"), Timestamp(2), DocumentMetaEnum::NONE); + insert(DocumentId("id:ns:type::test:3"), Timestamp(3), NONE); + insert(DocumentId("id:ns:type::test:1"), Timestamp(1), NONE); + insert(DocumentId("id:ns:type::test:2"), Timestamp(2), NONE); } }; @@ -63,13 +64,13 @@ TEST_F("require that BucketContent can provide bucket info", Fixture) { uint32_t lastChecksum = 0; EXPECT_NOT_EQUAL(lastChecksum, f.content.getBucketInfo().getChecksum()); lastChecksum = f.content.getBucketInfo().getChecksum(); - f.insert(DocumentId("id:ns:type::test:3"), Timestamp(4), DocumentMetaEnum::NONE); + f.insert(DocumentId("id:ns:type::test:3"), Timestamp(4), NONE); EXPECT_NOT_EQUAL(lastChecksum, f.content.getBucketInfo().getChecksum()); lastChecksum = f.content.getBucketInfo().getChecksum(); - f.insert(DocumentId("id:ns:type::test:2"), Timestamp(5), DocumentMetaEnum::REMOVE_ENTRY); + f.insert(DocumentId("id:ns:type::test:2"), Timestamp(5), REMOVE_ENTRY); EXPECT_NOT_EQUAL(lastChecksum, f.content.getBucketInfo().getChecksum()); - f.insert(DocumentId("id:ns:type::test:1"), Timestamp(6), DocumentMetaEnum::REMOVE_ENTRY); - f.insert(DocumentId("id:ns:type::test:3"), Timestamp(7), DocumentMetaEnum::REMOVE_ENTRY); + f.insert(DocumentId("id:ns:type::test:1"), Timestamp(6), REMOVE_ENTRY); + f.insert(DocumentId("id:ns:type::test:3"), Timestamp(7), REMOVE_ENTRY); EXPECT_EQUAL(0u, f.content.getBucketInfo().getChecksum()); } diff --git a/persistence/src/tests/spi/clusterstatetest.cpp b/persistence/src/tests/spi/clusterstatetest.cpp index 2186d408791..ac67903244f 100644 --- a/persistence/src/tests/spi/clusterstatetest.cpp +++ b/persistence/src/tests/spi/clusterstatetest.cpp @@ -5,12 +5,10 @@ #include <vespa/vdslib/distribution/distribution.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/config-stor-distribution.h> -#include <vespa/document/base/testdocman.h> #include <gtest/gtest.h> using storage::spi::test::makeSpiBucket; using vespalib::Trinary; -using document::GlobalId; namespace storage::spi { @@ -262,57 +260,4 @@ TEST(ClusterStateTest, node_maintenance_state_is_set_independent_of_bucket_space EXPECT_FALSE(node_marked_as_maintenance_in_state("distributor:3 storage:3 .0.s:m", d, 0, false)); } -TEST(DocEntryTest, test_basics) { - EXPECT_EQ(24, sizeof(DocEntry)); -} - -TEST(DocEntryTest, test_meta_only) { - DocEntry::UP e = DocEntry::create(Timestamp(9), DocumentMetaEnum::NONE); - EXPECT_EQ(9, e->getTimestamp()); - EXPECT_FALSE(e->isRemove()); - EXPECT_EQ(24, e->getSize()); - EXPECT_EQ(nullptr, e->getDocument()); - EXPECT_EQ(nullptr, e->getDocumentId()); - EXPECT_EQ("", e->getDocumentType()); - EXPECT_EQ(GlobalId(), e->getGid()); - - DocEntry::UP r = DocEntry::create(Timestamp(666), DocumentMetaEnum::REMOVE_ENTRY); - EXPECT_EQ(666, r->getTimestamp()); - EXPECT_TRUE(r->isRemove()); -} - -TEST(DocEntryTest, test_docid_only) { - DocEntry::UP e = DocEntry::create(Timestamp(9), DocumentMetaEnum::NONE, DocumentId("id:test:test::1")); - EXPECT_EQ(9, e->getTimestamp()); - EXPECT_FALSE(e->isRemove()); - EXPECT_EQ(16, e->getSize()); - EXPECT_EQ(nullptr, e->getDocument()); - EXPECT_NE(nullptr, e->getDocumentId()); - EXPECT_EQ("test", e->getDocumentType()); - EXPECT_EQ(GlobalId::parse("gid(0xc4ca4238f9f9649222750be2)"), e->getGid()); -} - -TEST(DocEntryTest, test_doctype_and_gid) { - DocEntry::UP e = DocEntry::create(Timestamp(9), DocumentMetaEnum::NONE, "doc_type", GlobalId::parse("gid(0xc4cef118f9f9649222750be2)")); - EXPECT_EQ(9, e->getTimestamp()); - EXPECT_FALSE(e->isRemove()); - EXPECT_EQ(20, e->getSize()); - EXPECT_EQ(nullptr, e->getDocument()); - EXPECT_EQ(nullptr, e->getDocumentId()); - EXPECT_EQ("doc_type", e->getDocumentType()); - EXPECT_EQ(GlobalId::parse("gid(0xc4cef118f9f9649222750be2)"), e->getGid()); -} - -TEST(DocEntryTest, test_document_only) { - document::TestDocMan testDocMan; - DocEntry::UP e = DocEntry::create(Timestamp(9), testDocMan.createRandomDocument(0, 1000)); - EXPECT_EQ(9, e->getTimestamp()); - EXPECT_FALSE(e->isRemove()); - EXPECT_EQ(632, e->getSize()); - EXPECT_NE(nullptr, e->getDocument()); - EXPECT_NE(nullptr, e->getDocumentId()); - EXPECT_EQ("testdoctype1", e->getDocumentType()); - EXPECT_EQ(GlobalId::parse("gid(0x4bc7000087365609f22f1f4b)"), e->getGid()); -} - } diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index 6afdd142457..810f2ad2356 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -5,7 +5,6 @@ #include <vespa/persistence/spi/test.h> #include <vespa/persistence/spi/catchresult.h> #include <vespa/persistence/spi/resource_usage_listener.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/fieldset/fieldsets.h> #include <vespa/document/update/documentupdate.h> #include <vespa/document/update/assignvalueupdate.h> @@ -26,17 +25,15 @@ using document::BucketId; using document::BucketSpace; using document::test::makeBucketSpace; using storage::spi::test::makeSpiBucket; -using storage::spi::test::cloneDocEntry; namespace storage::spi { using PersistenceProviderUP = std::unique_ptr<PersistenceProvider>; -using DocEntryList = std::vector<DocEntry::UP>; namespace { -std::unique_ptr<PersistenceProvider> -getSpi(ConformanceTest::PersistenceFactory &factory, const document::TestDocMan &testDocMan) { +std::unique_ptr<PersistenceProvider> getSpi(ConformanceTest::PersistenceFactory &factory, + const document::TestDocMan &testDocMan) { PersistenceProviderUP result(factory.getPersistenceImplementation( testDocMan.getTypeRepoSP(), *testDocMan.getTypeConfig())); EXPECT_TRUE(!result->initialize().hasError()); @@ -126,7 +123,7 @@ struct DocAndTimestamp */ struct Chunk { - DocEntryList _entries; + std::vector<DocEntry::UP> _entries; }; struct DocEntryIndirectTimestampComparator @@ -169,7 +166,7 @@ doIterate(PersistenceProvider& spi, } size_t -getRemoveEntryCount(const DocEntryList& entries) +getRemoveEntryCount(const std::vector<spi::DocEntry::UP>& entries) { size_t ret = 0; for (size_t i = 0; i < entries.size(); ++i) { @@ -180,13 +177,13 @@ getRemoveEntryCount(const DocEntryList& entries) return ret; } -DocEntryList +std::vector<DocEntry::UP> getEntriesFromChunks(const std::vector<Chunk>& chunks) { - DocEntryList ret; + std::vector<spi::DocEntry::UP> ret; for (size_t chunk = 0; chunk < chunks.size(); ++chunk) { for (size_t i = 0; i < chunks[chunk]._entries.size(); ++i) { - ret.push_back(cloneDocEntry(*chunks[chunk]._entries[i])); + ret.push_back(DocEntry::UP(chunks[chunk]._entries[i]->clone())); } } std::sort(ret.begin(), @@ -196,12 +193,12 @@ getEntriesFromChunks(const std::vector<Chunk>& chunks) } -DocEntryList +std::vector<DocEntry::UP> iterateBucket(PersistenceProvider& spi, const Bucket& bucket, IncludedVersions versions) { - DocEntryList ret; + std::vector<DocEntry::UP> ret; DocumentSelection docSel(""); Selection sel(docSel); @@ -220,7 +217,7 @@ iterateBucket(PersistenceProvider& spi, spi.iterate(iter.getIteratorId(), std::numeric_limits<int64_t>().max(), context); if (result.getErrorCode() != Result::ErrorType::NONE) { - return DocEntryList(); + return std::vector<DocEntry::UP>(); } auto list = result.steal_entries(); std::move(list.begin(), list.end(), std::back_inserter(ret)); @@ -241,7 +238,8 @@ verifyDocs(const std::vector<DocAndTimestamp>& wanted, const std::vector<Chunk>& chunks, const std::set<string>& removes = std::set<string>()) { - DocEntryList retrieved = getEntriesFromChunks(chunks); + std::vector<DocEntry::UP> retrieved( + getEntriesFromChunks(chunks)); size_t removeCount = getRemoveEntryCount(retrieved); // Ensure that we've got the correct number of puts and removes EXPECT_EQ(removes.size(), removeCount); @@ -259,13 +257,15 @@ verifyDocs(const std::vector<DocAndTimestamp>& wanted, } EXPECT_EQ(wanted[wantedIdx].timestamp, entry.getTimestamp()); size_t serSize = wanted[wantedIdx].doc->serialize().size(); - EXPECT_EQ(serSize, size_t(entry.getSize())); + EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize())); + EXPECT_EQ(serSize, size_t(entry.getDocumentSize())); ++wantedIdx; } else { // Remove-entry EXPECT_TRUE(entry.getDocumentId() != 0); size_t serSize = entry.getDocumentId()->getSerializedSize(); - EXPECT_EQ(serSize, size_t(entry.getSize())); + EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize())); + EXPECT_EQ(serSize, size_t(entry.getDocumentSize())); if (removes.find(entry.getDocumentId()->toString()) == removes.end()) { FAIL() << "Got unexpected remove entry for document id " << *entry.getDocumentId(); @@ -697,7 +697,8 @@ TEST_F(ConformanceTest, testPutDuplicate) EXPECT_EQ(1, (int)info.getDocumentCount()); EXPECT_EQ(checksum, info.getChecksum()); } - DocEntryList entries = iterateBucket(*spi, bucket, ALL_VERSIONS); + std::vector<DocEntry::UP> entries( + iterateBucket(*spi, bucket, ALL_VERSIONS)); EXPECT_EQ(size_t(1), entries.size()); } @@ -721,7 +722,8 @@ TEST_F(ConformanceTest, testRemove) EXPECT_EQ(1, (int)info.getDocumentCount()); EXPECT_TRUE(info.getChecksum() != 0); - DocEntryList entries = iterateBucket(*spi, bucket, NEWEST_DOCUMENT_ONLY); + std::vector<DocEntry::UP> entries( + iterateBucket(*spi, bucket, NEWEST_DOCUMENT_ONLY)); EXPECT_EQ(size_t(1), entries.size()); } @@ -739,11 +741,15 @@ TEST_F(ConformanceTest, testRemove) EXPECT_EQ(true, result2.wasFound()); } { - DocEntryList entries = iterateBucket(*spi, bucket,NEWEST_DOCUMENT_ONLY); + std::vector<DocEntry::UP> entries(iterateBucket(*spi, + bucket, + NEWEST_DOCUMENT_ONLY)); EXPECT_EQ(size_t(0), entries.size()); } { - DocEntryList entries = iterateBucket(*spi, bucket,NEWEST_DOCUMENT_OR_REMOVE); + std::vector<DocEntry::UP> entries(iterateBucket(*spi, + bucket, + NEWEST_DOCUMENT_OR_REMOVE)); EXPECT_EQ(size_t(1), entries.size()); } @@ -856,7 +862,8 @@ TEST_F(ConformanceTest, testRemoveMerge) // Remove entry should exist afterwards { - DocEntryList entries = iterateBucket(*spi, bucket, ALL_VERSIONS); + std::vector<DocEntry::UP> entries(iterateBucket( + *spi, bucket, ALL_VERSIONS)); EXPECT_EQ(size_t(2), entries.size()); // Timestamp-sorted by iterateBucket EXPECT_EQ(removeId, *entries.back()->getDocumentId()); @@ -882,7 +889,7 @@ TEST_F(ConformanceTest, testRemoveMerge) } // Must have new remove. We don't check for the presence of the old remove. { - DocEntryList entries = iterateBucket(*spi, bucket, ALL_VERSIONS); + std::vector<DocEntry::UP> entries(iterateBucket(*spi, bucket, ALL_VERSIONS)); EXPECT_TRUE(entries.size() >= 2); EXPECT_EQ(removeId, *entries.back()->getDocumentId()); EXPECT_EQ(Timestamp(11), entries.back()->getTimestamp()); @@ -908,7 +915,7 @@ TEST_F(ConformanceTest, testRemoveMerge) } // Must have newest remove. We don't check for the presence of the old remove. { - DocEntryList entries = iterateBucket(*spi, bucket, ALL_VERSIONS); + std::vector<DocEntry::UP> entries(iterateBucket(*spi, bucket, ALL_VERSIONS)); EXPECT_TRUE(entries.size() >= 2); EXPECT_EQ(removeId, *entries.back()->getDocumentId()); EXPECT_EQ(Timestamp(11), entries.back()->getTimestamp()); @@ -1344,7 +1351,7 @@ TEST_F(ConformanceTest, testIterateRemoves) CreateIteratorResult iter(createIterator(*spi, b, sel, NEWEST_DOCUMENT_OR_REMOVE)); std::vector<Chunk> chunks = doIterate(*spi, iter.getIteratorId(), 4_Ki); - DocEntryList entries = getEntriesFromChunks(chunks); + std::vector<DocEntry::UP> entries = getEntriesFromChunks(chunks); EXPECT_EQ(docs.size(), entries.size()); verifyDocs(nonRemovedDocs, chunks, removedDocs); diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index d947ca51f49..9d9f31b63a3 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -9,7 +9,6 @@ #include <vespa/persistence/spi/i_resource_usage_listener.h> #include <vespa/persistence/spi/resource_usage.h> #include <vespa/persistence/spi/bucketexecutor.h> -#include <vespa/persistence/spi/test.h> #include <vespa/vespalib/util/crc.h> #include <vespa/document/fieldset/fieldsetrepo.h> #include <vespa/vespalib/stllike/asciistream.h> @@ -25,9 +24,6 @@ using vespalib::make_string; using std::binary_search; using std::lower_bound; using document::FixedBucketSpaces; -using document::FieldSet; -using storage::spi::test::cloneDocEntry; -using storage::spi::test::equal; namespace storage::spi::dummy { @@ -166,7 +162,7 @@ BucketContent::insert(DocEntry::SP e) { auto it = lower_bound(_entries.begin(), _entries.end(), e->getTimestamp(), TimestampLess()); if (it != _entries.end()) { if (it->entry->getTimestamp() == e->getTimestamp()) { - if (equal(*it->entry, *e)) { + if (*it->entry.get() == *e) { LOG(debug, "Ignoring duplicate put entry %s", e->toString().c_str()); return; } else { @@ -435,7 +431,7 @@ DummyPersistence::putAsync(const Bucket& b, Timestamp t, Document::SP doc, Conte } } else { LOG(spam, "Inserting document %s", doc->toString(true).c_str()); - auto entry = DocEntry::create(t, Document::UP(doc->clone())); + auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone())); (*bc)->insert(std::move(entry)); bc.reset(); onComplete->onComplete(std::make_unique<Result>()); @@ -493,7 +489,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI } DocEntry::SP entry((*bc)->getEntry(id)); numRemoves += (entry.get() && !entry->isRemove()) ? 1 : 0; - auto remEntry = DocEntry::create(t, DocumentMetaEnum::REMOVE_ENTRY, id); + auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, id); if ((*bc)->hasTimestamp(t)) { (*bc)->eraseEntry(t); @@ -505,7 +501,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI } GetResult -DummyPersistence::get(const Bucket& b, const FieldSet& fieldSet, const DocumentId& did, Context&) const +DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const DocumentId& did, Context&) const { DUMMYPERSISTENCE_VERIFY_INITIALIZED; LOG(debug, "get(%s, %s)", @@ -522,8 +518,8 @@ DummyPersistence::get(const Bucket& b, const FieldSet& fieldSet, const DocumentI return GetResult::make_for_tombstone(entry->getTimestamp()); } else { Document::UP doc(entry->getDocument()->clone()); - if (fieldSet.getType() != FieldSet::Type::ALL) { - FieldSet::stripFields(*doc, fieldSet); + if (fieldSet.getType() != document::FieldSet::Type::ALL) { + document::FieldSet::stripFields(*doc, fieldSet); } return GetResult(std::move(doc), entry->getTimestamp()); } @@ -653,16 +649,22 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con if (currentSize != 0 && currentSize + size > maxByteSize) break; currentSize += size; if (!entry->isRemove() - && it->_fieldSet->getType() != FieldSet::Type::ALL) + && it->_fieldSet->getType() != document::FieldSet::Type::ALL) { assert(entry->getDocument()); // Create new document with only wanted fields. - Document::UP filtered(FieldSet::createDocumentSubsetCopy(*entry->getDocument(), *it->_fieldSet)); - auto ret = DocEntry::create(entry->getTimestamp(), std::move(filtered), entry->getSize()); + Document::UP filtered( + document::FieldSet::createDocumentSubsetCopy( + *entry->getDocument(), + *it->_fieldSet)); + DocEntry::UP ret(new DocEntry(entry->getTimestamp(), + entry->getFlags(), + std::move(filtered), + entry->getPersistedDocumentSize())); entries.push_back(std::move(ret)); } else { // Use entry as-is. - entries.push_back(cloneDocEntry(*entry)); + entries.push_back(DocEntry::UP(entry->clone())); ++fastPath; } } diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h index 3b6fc9f1449..a7a784d0479 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h @@ -9,7 +9,6 @@ #pragma once #include <vespa/persistence/spi/abstractpersistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/base/globalid.h> #include <vespa/document/fieldset/fieldsets.h> #include <vespa/vespalib/stllike/hash_map.h> diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp index f0329e8cc5e..8669bbf666b 100644 --- a/persistence/src/vespa/persistence/spi/docentry.cpp +++ b/persistence/src/vespa/persistence/spi/docentry.cpp @@ -4,104 +4,90 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/vespalib/objects/nbostream.h> #include <sstream> +#include <cassert> namespace storage::spi { -namespace { - -class DocEntryWithId final : public DocEntry { -public: - DocEntryWithId(Timestamp t, DocumentMetaEnum metaEnum, const DocumentId &docId); - ~DocEntryWithId(); - vespalib::string toString() const override; - const DocumentId* getDocumentId() const override { return & _documentId; } - vespalib::stringref getDocumentType() const override { return _documentId.getDocType(); } - GlobalId getGid() const override { return _documentId.getGlobalId(); } -private: - DocumentId _documentId; -}; - -class DocEntryWithTypeAndGid final : public DocEntry { -public: - DocEntryWithTypeAndGid(Timestamp t, DocumentMetaEnum metaEnum, vespalib::stringref docType, GlobalId gid); - ~DocEntryWithTypeAndGid(); - vespalib::string toString() const override; - vespalib::stringref getDocumentType() const override { return _type; } - GlobalId getGid() const override { return _gid; } -private: - vespalib::string _type; - GlobalId _gid; -}; - -class DocEntryWithDoc final : public DocEntry { -public: - DocEntryWithDoc(Timestamp t, DocumentUP doc); - - /** - * Constructor that can be used by providers that already know - * the serialized size of the document, so the potentially expensive - * call to getSerializedSize can be avoided. This value shall be the size of the document _before_ - * any field filtering is performed. - */ - DocEntryWithDoc(Timestamp t, DocumentUP doc, size_t serializedDocumentSize); - ~DocEntryWithDoc(); - vespalib::string toString() const override; - const Document* getDocument() const override { return _document.get(); } - const DocumentId* getDocumentId() const override { return &_document->getId(); } - DocumentUP releaseDocument() override { return std::move(_document); } - vespalib::stringref getDocumentType() const override { return _document->getId().getDocType(); } - GlobalId getGid() const override { return _document->getId().getGlobalId(); } -private: - DocumentUP _document; -}; - -DocEntryWithDoc::DocEntryWithDoc(Timestamp t, DocumentUP doc) - : DocEntry(t, DocumentMetaEnum::NONE, doc->serialize().size()), +DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc) + : _timestamp(t), + _metaFlags(metaFlags), + _persistedDocumentSize(doc->serialize().size()), + _size(_persistedDocumentSize + sizeof(DocEntry)), + _documentId(), _document(std::move(doc)) { } -DocEntryWithDoc::DocEntryWithDoc(Timestamp t, DocumentUP doc, size_t serializedDocumentSize) - : DocEntry(t, DocumentMetaEnum::NONE, serializedDocumentSize), +DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize) + : _timestamp(t), + _metaFlags(metaFlags), + _persistedDocumentSize(serializedDocumentSize), + _size(_persistedDocumentSize + sizeof(DocEntry)), + _documentId(), _document(std::move(doc)) { } -DocEntryWithId::DocEntryWithId(Timestamp t, DocumentMetaEnum metaEnum, const DocumentId& docId) - : DocEntry(t, metaEnum, docId.getSerializedSize()), - _documentId(docId) +DocEntry::DocEntry(Timestamp t, int metaFlags, const DocumentId& docId) + : _timestamp(t), + _metaFlags(metaFlags), + _persistedDocumentSize(docId.getSerializedSize()), + _size(_persistedDocumentSize + sizeof(DocEntry)), + _documentId(new DocumentId(docId)), + _document() { } -DocEntryWithTypeAndGid::DocEntryWithTypeAndGid(Timestamp t, DocumentMetaEnum metaEnum, vespalib::stringref docType, GlobalId gid) - : DocEntry(t, metaEnum, docType.size() + sizeof(gid)), - _type(docType), - _gid(gid) +DocEntry::DocEntry(Timestamp t, int metaFlags) + : _timestamp(t), + _metaFlags(metaFlags), + _persistedDocumentSize(0), + _size(sizeof(DocEntry)), + _documentId(), + _document() { } -DocEntryWithTypeAndGid::~DocEntryWithTypeAndGid() = default; -DocEntryWithId::~DocEntryWithId() = default; -DocEntryWithDoc::~DocEntryWithDoc() = default; +DocEntry::~DocEntry() = default; -vespalib::string -DocEntryWithId::toString() const -{ - std::ostringstream out; - out << "DocEntry(" << getTimestamp() << ", " << int(getMetaEnum()) << ", " << _documentId << ")"; - return out.str(); +DocEntry* +DocEntry::clone() const { + DocEntry* ret; + if (_documentId) { + ret = new DocEntry(_timestamp, _metaFlags, *_documentId); + ret->setPersistedDocumentSize(_persistedDocumentSize); + } else if (_document) { + ret = new DocEntry(_timestamp, _metaFlags, + std::make_unique<Document>(*_document), + _persistedDocumentSize); + } else { + ret = new DocEntry(_timestamp, _metaFlags); + ret->setPersistedDocumentSize(_persistedDocumentSize); + } + return ret; } -vespalib::string -DocEntryWithTypeAndGid::toString() const +const DocumentId* +DocEntry::getDocumentId() const { + return (_document ? &_document->getId() : _documentId.get()); +} + +DocumentUP +DocEntry::releaseDocument() { + return std::move(_document); +} + +DocEntry::SizeType +DocEntry::getDocumentSize() const { - std::ostringstream out; - out << "DocEntry(" << getTimestamp() << ", " << int(getMetaEnum()) << ", " << _type << ", " << _gid << ")"; - return out.str(); + assert(_size >= sizeof(DocEntry)); + return _size - sizeof(DocEntry); } vespalib::string -DocEntryWithDoc::toString() const +DocEntry::toString() const { std::ostringstream out; - out << "DocEntry(" << getTimestamp() << ", " << int(getMetaEnum()) << ", "; - if (_document.get()) { + out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", "; + if (_documentId) { + out << *_documentId; + } else if (_document.get()) { out << "Doc(" << _document->getId() << ")"; } else { out << "metadata only"; @@ -110,47 +96,53 @@ DocEntryWithDoc::toString() const return out.str(); } +std::ostream & +operator << (std::ostream & os, const DocEntry & r) { + return os << r.toString(); } -DocEntry::UP -DocEntry::create(Timestamp t, DocumentMetaEnum metaEnum) { - return UP(new DocEntry(t, metaEnum)); -} -DocEntry::UP -DocEntry::create(Timestamp t, DocumentMetaEnum metaEnum, const DocumentId &docId) { - return std::make_unique<DocEntryWithId>(t, metaEnum, docId); -} -DocEntry::UP -DocEntry::create(Timestamp t, DocumentMetaEnum metaEnum, vespalib::stringref docType, GlobalId gid) { - return std::make_unique<DocEntryWithTypeAndGid>(t, metaEnum, docType, gid); -} -DocEntry::UP -DocEntry::create(Timestamp t, DocumentUP doc) { - return std::make_unique<DocEntryWithDoc>(t, std::move(doc)); -} -DocEntry::UP -DocEntry::create(Timestamp t, DocumentUP doc, SizeType serializedDocumentSize) { - return std::make_unique<DocEntryWithDoc>(t, std::move(doc), serializedDocumentSize); -} +bool +DocEntry::operator==(const DocEntry& entry) const { + if (_timestamp != entry._timestamp) { + return false; + } -DocEntry::~DocEntry() = default; + if (_metaFlags != entry._metaFlags) { + return false; + } -DocumentUP -DocEntry::releaseDocument() { - return {}; -} + if (_documentId) { + if (!entry._documentId) { + return false; + } -vespalib::string -DocEntry::toString() const -{ - std::ostringstream out; - out << "DocEntry(" << _timestamp << ", " << int(_metaEnum) << ", metadata only)"; - return out.str(); -} + if (*_documentId != *entry._documentId) { + return false; + } + } else { + if (entry._documentId) { + return false; + } + } -std::ostream & -operator << (std::ostream & os, const DocEntry & r) { - return os << r.toString(); + if (_document) { + if (!entry._document) { + return false; + } + + if (*_document != *entry._document) { + return false; + } + } else { + if (entry._document) { + return false; + } + } + if (_persistedDocumentSize != entry._persistedDocumentSize) { + return false; + } + + return true; } } diff --git a/persistence/src/vespa/persistence/spi/docentry.h b/persistence/src/vespa/persistence/spi/docentry.h index 9ad06b41e90..3374ef6c02d 100644 --- a/persistence/src/vespa/persistence/spi/docentry.h +++ b/persistence/src/vespa/persistence/spi/docentry.h @@ -14,59 +14,80 @@ #pragma once #include <persistence/spi/types.h> -#include <vespa/document/base/globalid.h> namespace storage::spi { -enum class DocumentMetaEnum { +enum DocumentMetaFlags { NONE = 0x0, REMOVE_ENTRY = 0x1 }; class DocEntry { public: - using SizeType = uint32_t; + typedef uint32_t SizeType; +private: + Timestamp _timestamp; + int _metaFlags; + SizeType _persistedDocumentSize; + SizeType _size; + DocumentIdUP _documentId; + DocumentUP _document; +public: using UP = std::unique_ptr<DocEntry>; using SP = std::shared_ptr<DocEntry>; - DocEntry(const DocEntry &) = delete; - DocEntry & operator=(const DocEntry &) = delete; - DocEntry(DocEntry &&) = delete; - DocEntry & operator=(DocEntry &&) = delete; - virtual ~DocEntry(); - bool isRemove() const { return (_metaEnum == DocumentMetaEnum::REMOVE_ENTRY); } + DocEntry(Timestamp t, int metaFlags, DocumentUP doc); + + /** + * Constructor that can be used by providers that already know + * the serialized size of the document, so the potentially expensive + * call to getSerializedSize can be avoided. + */ + DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize); + DocEntry(Timestamp t, int metaFlags, const DocumentId& docId); + + DocEntry(Timestamp t, int metaFlags); + ~DocEntry(); + DocEntry* clone() const; + const Document* getDocument() const { return _document.get(); } + const DocumentId* getDocumentId() const; + DocumentUP releaseDocument(); + bool isRemove() const { return (_metaFlags & REMOVE_ENTRY); } Timestamp getTimestamp() const { return _timestamp; } - DocumentMetaEnum getMetaEnum() const { return _metaEnum; } + int getFlags() const { return _metaFlags; } + void setFlags(int flags) { _metaFlags = flags; } + /** + * @return In-memory size of this doc entry, including document instance. + * In essence: serialized size of document + sizeof(DocEntry). + */ + SizeType getSize() const { return _size; } /** * If entry contains a document, returns its serialized size. * If entry contains a document id, returns the serialized size of * the id alone. - * Otherwise (i.e. metadata only), returns sizeof(DocEntry). + * Otherwise (i.e. metadata only), returns zero. */ - SizeType getSize() const { return _size; } + SizeType getDocumentSize() const; + /** + * Return size of document as it exists in persisted form. By default + * this will return the serialized size of the entry's document instance, + * but for persistence providers that are able to provide this information + * efficiently, this value can be set explicitly to provide better statistical + * tracking for e.g. visiting operations in the service layer. + * If explicitly set, this value shall be the size of the document _before_ + * any field filtering is performed. + */ + SizeType getPersistedDocumentSize() const { return _persistedDocumentSize; } + /** + * Set persisted size of document. Optional. + * @see getPersistedDocumentSize + */ + void setPersistedDocumentSize(SizeType persistedDocumentSize) { + _persistedDocumentSize = persistedDocumentSize; + } - virtual vespalib::string toString() const; - virtual const Document* getDocument() const { return nullptr; } - virtual const DocumentId* getDocumentId() const { return nullptr; } - virtual vespalib::stringref getDocumentType() const { return vespalib::stringref(); } - virtual GlobalId getGid() const { return GlobalId(); } - virtual DocumentUP releaseDocument(); - static UP create(Timestamp t, DocumentMetaEnum metaEnum); - static UP create(Timestamp t, DocumentMetaEnum metaEnum, const DocumentId &docId); - static UP create(Timestamp t, DocumentMetaEnum metaEnum, vespalib::stringref docType, GlobalId gid); - static UP create(Timestamp t, DocumentUP doc); - static UP create(Timestamp t, DocumentUP doc, SizeType serializedDocumentSize); -protected: - DocEntry(Timestamp t, DocumentMetaEnum metaEnum, SizeType size) - : _timestamp(t), - _metaEnum(metaEnum), - _size(size) - {} -private: - DocEntry(Timestamp t, DocumentMetaEnum metaEnum) : DocEntry(t, metaEnum, sizeof(DocEntry)) { } - Timestamp _timestamp; - DocumentMetaEnum _metaEnum; - SizeType _size; + vespalib::string toString() const; + bool operator==(const DocEntry& entry) const; }; std::ostream & operator << (std::ostream & os, const DocEntry & r); diff --git a/persistence/src/vespa/persistence/spi/result.cpp b/persistence/src/vespa/persistence/spi/result.cpp index a728f93e60a..e458d58fe69 100644 --- a/persistence/src/vespa/persistence/spi/result.cpp +++ b/persistence/src/vespa/persistence/spi/result.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "result.h" -#include "docentry.h" #include <vespa/document/fieldvalue/document.h> #include <vespa/vespalib/stllike/asciistream.h> #include <ostream> @@ -49,24 +48,6 @@ GetResult::~GetResult() = default; BucketIdListResult::~BucketIdListResult() = default; IterateResult::~IterateResult() = default; -IterateResult::IterateResult(IterateResult &&) noexcept = default; -IterateResult & IterateResult::operator=(IterateResult &&) noexcept = default; - -IterateResult::IterateResult(ErrorType error, const vespalib::string& errorMessage) - : Result(error, errorMessage), - _completed(false) -{ } - - -IterateResult::IterateResult(List entries, bool completed) - : _completed(completed), - _entries(std::move(entries)) -{ } - -IterateResult::List -IterateResult::steal_entries() { - return std::move(_entries); -} } diff --git a/persistence/src/vespa/persistence/spi/result.h b/persistence/src/vespa/persistence/spi/result.h index 10c589307ba..c734a885b12 100644 --- a/persistence/src/vespa/persistence/spi/result.h +++ b/persistence/src/vespa/persistence/spi/result.h @@ -3,12 +3,11 @@ #include "bucketinfo.h" #include "bucket.h" +#include "docentry.h" #include <vespa/document/bucket/bucketidlist.h> namespace storage::spi { -class DocEntry; - class Result { public: typedef std::unique_ptr<Result> UP; @@ -280,12 +279,15 @@ private: class IterateResult : public Result { public: - using List = std::vector<std::unique_ptr<DocEntry>>; + typedef std::vector<DocEntry::UP> List; /** * Constructor used when there was an error creating the iterator. */ - IterateResult(ErrorType error, const vespalib::string& errorMessage); + IterateResult(ErrorType error, const vespalib::string& errorMessage) + : Result(error, errorMessage), + _completed(false) + { } /** * Constructor used when the iteration was successful. @@ -294,21 +296,24 @@ public: * * @param completed Set to true if iteration has been completed. */ - IterateResult(List entries, bool completed); + IterateResult(List entries, bool completed) + : _completed(completed), + _entries(std::move(entries)) + { } IterateResult(const IterateResult &) = delete; - IterateResult(IterateResult &&rhs) noexcept; - IterateResult &operator=(IterateResult &&rhs) noexcept; + IterateResult(IterateResult &&rhs) noexcept = default; + IterateResult &operator=(IterateResult &&rhs) noexcept = default; ~IterateResult(); const List& getEntries() const { return _entries; } - List steal_entries(); + List steal_entries() { return std::move(_entries); } bool isCompleted() const { return _completed; } private: bool _completed; - List _entries; + std::vector<DocEntry::UP> _entries; }; } diff --git a/persistence/src/vespa/persistence/spi/test.cpp b/persistence/src/vespa/persistence/spi/test.cpp index 58a8ce3fe52..32381110630 100644 --- a/persistence/src/vespa/persistence/spi/test.cpp +++ b/persistence/src/vespa/persistence/spi/test.cpp @@ -1,9 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "test.h" -#include "docentry.h" #include <vespa/document/test/make_bucket_space.h> -#include <vespa/document/fieldvalue/document.h> using document::BucketId; using document::BucketSpace; @@ -11,45 +9,9 @@ using document::test::makeBucketSpace; namespace storage::spi::test { -Bucket -makeSpiBucket(BucketId bucketId) +Bucket makeSpiBucket(BucketId bucketId) { return Bucket(document::Bucket(makeBucketSpace(), bucketId)); } -std::unique_ptr<DocEntry> -cloneDocEntry(const DocEntry & e) { - std::unique_ptr<DocEntry> ret; - if (e.getDocument()) { - ret = DocEntry::create(e.getTimestamp(), std::make_unique<Document>(*e.getDocument()), e.getSize()); - } else if (e.getDocumentId()) { - ret = DocEntry::create(e.getTimestamp(), e.getMetaEnum(), *e.getDocumentId()); - } else { - ret = DocEntry::create(e.getTimestamp(), e.getMetaEnum()); - } - return ret; -} - -bool -equal(const DocEntry & a, const DocEntry & b) { - if (a.getTimestamp() != b.getTimestamp()) return false; - if (a.getMetaEnum() != b.getMetaEnum()) return false; - if (a.getSize() != b.getSize()) return false; - - if (a.getDocument()) { - if (!b.getDocument()) return false; - if (*a.getDocument() != *b.getDocument()) return false; - } else { - if (b.getDocument()) return false; - } - if (a.getDocumentId()) { - if (!b.getDocumentId()) return false; - if (*a.getDocumentId() != *b.getDocumentId()) return false; - } else { - if (b.getDocumentId()) return false; - } - - return true; -} - } diff --git a/persistence/src/vespa/persistence/spi/test.h b/persistence/src/vespa/persistence/spi/test.h index 1660e5f14fd..af7109ec80c 100644 --- a/persistence/src/vespa/persistence/spi/test.h +++ b/persistence/src/vespa/persistence/spi/test.h @@ -3,16 +3,11 @@ #pragma once #include "bucket.h" -#include <memory> - -namespace storage::spi { class DocEntry; } namespace storage::spi::test { // Helper functions used by unit tests Bucket makeSpiBucket(document::BucketId bucketId); -std::unique_ptr<DocEntry> cloneDocEntry(const DocEntry & entry); -bool equal(const DocEntry & a, const DocEntry & b); } diff --git a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp index 82eac88a53e..fab46e61494 100644 --- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp +++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp @@ -48,9 +48,7 @@ using storage::spi::IncludedVersions; using storage::spi::IterateResult; using storage::spi::Selection; using storage::spi::Timestamp; -using storage::spi::DocumentMetaEnum; using storage::spi::test::makeSpiBucket; -using storage::spi::test::equal; using namespace proton; @@ -276,14 +274,18 @@ struct PairDR : DocumentRetrieverBaseForTest { } }; +size_t getSize() { + return sizeof(DocEntry); +} + size_t getSize(const document::Document &doc) { vespalib::nbostream tmp; doc.serialize(tmp); - return tmp.size(); + return tmp.size() + getSize(); } size_t getSize(const document::DocumentId &id) { - return id.getSerializedSize(); + return id.getSerializedSize() + getSize(); } IDocumentRetriever::SP nil() { return std::make_unique<UnitDR>(); } @@ -384,19 +386,19 @@ void checkDoc(const IDocumentRetriever &dr, const std::string &id, EXPECT_TRUE(DocumentId(id) == doc->getId()); } -void checkEntry(const IterateResult &res, size_t idx, const Timestamp ×tamp, DocumentMetaEnum flags) +void checkEntry(const IterateResult &res, size_t idx, const Timestamp ×tamp, int flags) { ASSERT_LESS(idx, res.getEntries().size()); - auto expect = DocEntry::create(timestamp, flags); - EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); - EXPECT_EQUAL(sizeof(DocEntry), res.getEntries()[idx]->getSize()); + DocEntry expect(timestamp, flags); + EXPECT_EQUAL(expect, *res.getEntries()[idx]); + EXPECT_EQUAL(getSize(), res.getEntries()[idx]->getSize()); } void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, const Timestamp ×tamp) { ASSERT_LESS(idx, res.getEntries().size()); - auto expect = DocEntry::create(timestamp, DocumentMetaEnum::REMOVE_ENTRY, id); - EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); + DocEntry expect(timestamp, storage::spi::REMOVE_ENTRY, id); + EXPECT_EQUAL(expect, *res.getEntries()[idx]); EXPECT_EQUAL(getSize(id), res.getEntries()[idx]->getSize()); EXPECT_GREATER(getSize(id), 0u); } @@ -404,8 +406,8 @@ void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, cons void checkEntry(const IterateResult &res, size_t idx, const Document &doc, const Timestamp ×tamp) { ASSERT_LESS(idx, res.getEntries().size()); - auto expect = DocEntry::create(timestamp, Document::UP(doc.clone())); - EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); + DocEntry expect(timestamp, storage::spi::NONE, Document::UP(doc.clone())); + EXPECT_EQUAL(expect, *res.getEntries()[idx]); EXPECT_EQUAL(getSize(doc), res.getEntries()[idx]->getSize()); EXPECT_GREATER(getSize(doc), 0u); } @@ -606,9 +608,9 @@ TEST("require that using an empty field set returns meta-data only") { IterateResult res = itr.iterate(largeNum); EXPECT_TRUE(res.isCompleted()); EXPECT_EQUAL(3u, res.getEntries().size()); - TEST_DO(checkEntry(res, 0, Timestamp(2), DocumentMetaEnum::NONE)); - TEST_DO(checkEntry(res, 1, Timestamp(3), DocumentMetaEnum::NONE)); - TEST_DO(checkEntry(res, 2, Timestamp(4), DocumentMetaEnum::REMOVE_ENTRY)); + TEST_DO(checkEntry(res, 0, Timestamp(2), storage::spi::NONE)); + TEST_DO(checkEntry(res, 1, Timestamp(3), storage::spi::NONE)); + TEST_DO(checkEntry(res, 2, Timestamp(4), storage::spi::REMOVE_ENTRY)); } TEST("require that entries in other buckets are skipped") { @@ -648,15 +650,15 @@ TEST("require that maxBytes splits iteration results for meta-data only iteratio itr.add(doc("id:ns:document::1", Timestamp(2), bucket(5))); itr.add(cat(rem("id:ns:document::2", Timestamp(3), bucket(5)), doc("id:ns:document::3", Timestamp(4), bucket(5)))); - IterateResult res1 = itr.iterate(2 * sizeof(DocEntry)); + IterateResult res1 = itr.iterate(getSize() + getSize()); EXPECT_TRUE(!res1.isCompleted()); EXPECT_EQUAL(2u, res1.getEntries().size()); - TEST_DO(checkEntry(res1, 0, Timestamp(2), DocumentMetaEnum::NONE)); - TEST_DO(checkEntry(res1, 1, Timestamp(3), DocumentMetaEnum::REMOVE_ENTRY)); + TEST_DO(checkEntry(res1, 0, Timestamp(2), storage::spi::NONE)); + TEST_DO(checkEntry(res1, 1, Timestamp(3), storage::spi::REMOVE_ENTRY)); IterateResult res2 = itr.iterate(largeNum); EXPECT_TRUE(res2.isCompleted()); - TEST_DO(checkEntry(res2, 0, Timestamp(4), DocumentMetaEnum::NONE)); + TEST_DO(checkEntry(res2, 0, Timestamp(4), storage::spi::NONE)); IterateResult res3 = itr.iterate(largeNum); EXPECT_TRUE(res3.isCompleted()); diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp index 278b0c68dab..b39b5dc5734 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "document_iterator.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/searchcore/proton/common/cachedselect.h> #include <vespa/searchcore/proton/common/selectcontext.h> #include <vespa/document/select/gid_filter.h> @@ -9,6 +8,7 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/vespalib/stllike/hash_map.h> +#include <algorithm> #include <vespa/log/log.h> LOG_SETUP(".proton.persistenceengine.document_iterator"); @@ -18,25 +18,23 @@ using storage::spi::DocEntry; using storage::spi::Timestamp; using document::Document; using document::DocumentId; -using storage::spi::DocumentMetaEnum; namespace proton { namespace { -std::unique_ptr<DocEntry> -createDocEntry(Timestamp timestamp, bool removed) { - return DocEntry::create(timestamp, removed ? DocumentMetaEnum::REMOVE_ENTRY : DocumentMetaEnum::NONE); +DocEntry *createDocEntry(Timestamp timestamp, bool removed) { + int flags = removed ? storage::spi::REMOVE_ENTRY : storage::spi::NONE; + return new DocEntry(timestamp, flags); } -std::unique_ptr<DocEntry> -createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ssize_t defaultSerializedSize) { +DocEntry *createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ssize_t defaultSerializedSize) { if (doc) { if (removed) { - return DocEntry::create(timestamp, DocumentMetaEnum::REMOVE_ENTRY, doc->getId()); + return new DocEntry(timestamp, storage::spi::REMOVE_ENTRY, doc->getId()); } else { ssize_t serializedSize = defaultSerializedSize >= 0 ? defaultSerializedSize : doc->serialize().size(); - return DocEntry::create(timestamp, std::move(doc), serializedSize); + return new DocEntry(timestamp, storage::spi::NONE, std::move(doc), serializedSize); } } else { return createDocEntry(timestamp, removed); @@ -214,7 +212,7 @@ public: if (doc && _fields) { document::FieldSet::stripFields(*doc, *_fields); } - _list.push_back(createDocEntry(meta.timestamp, meta.removed, std::move(doc), _defaultSerializedSize)); + _list.emplace_back(createDocEntry(meta.timestamp, meta.removed, std::move(doc), _defaultSerializedSize)); } } @@ -264,12 +262,11 @@ DocumentIterator::fetchCompleteSource(const IDocumentRetriever & source, Iterate } LOG(debug, "metadata count after filtering: %zu", lidsToFetch.size()); - list.reserve(lidsToFetch.size()); if ( _metaOnly ) { for (uint32_t lid : lidsToFetch) { const search::DocumentMetaData & meta = metaData[lidIndexMap[lid]]; assert(lid == meta.lid); - list.push_back(createDocEntry(meta.timestamp, meta.removed)); + list.emplace_back(createDocEntry(meta.timestamp, meta.removed)); } } else { MatchVisitor visitor(matcher, metaData, lidIndexMap, _fields.get(), list, _defaultSerializedSize); diff --git a/storage/src/tests/persistence/splitbitdetectortest.cpp b/storage/src/tests/persistence/splitbitdetectortest.cpp index 5c4dc85e825..d4e84836a5a 100644 --- a/storage/src/tests/persistence/splitbitdetectortest.cpp +++ b/storage/src/tests/persistence/splitbitdetectortest.cpp @@ -15,8 +15,6 @@ using namespace ::testing; namespace storage { -using DocEntryList = std::vector<spi::DocEntry::UP>; - struct SplitBitDetectorTest : Test { document::TestDocMan testDocMan; spi::dummy::DummyPersistence provider; @@ -35,7 +33,7 @@ struct SplitBitDetectorTest : Test { }; TEST_F(SplitBitDetectorTest, two_users) { - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t i = 0; i < 5; ++i) { document::Document::SP doc( testDocMan.createRandomDocumentAtLocation(1, i, 1, 1)); @@ -56,7 +54,7 @@ TEST_F(SplitBitDetectorTest, two_users) { } TEST_F(SplitBitDetectorTest, single_user) { - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t i = 0; i < 10; ++i) { document::Document::SP doc( testDocMan.createRandomDocumentAtLocation(1, i, 1, 1)); @@ -73,7 +71,7 @@ TEST_F(SplitBitDetectorTest, single_user) { TEST_F(SplitBitDetectorTest, max_bits) { int minContentSize = 1, maxContentSize = 1; - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t seed = 0; seed < 10; ++seed) { int location = 1; document::Document::SP doc(testDocMan.createRandomDocumentAtLocation( @@ -94,7 +92,7 @@ TEST_F(SplitBitDetectorTest, max_bits_one_below_max) { provider.createBucket(my_bucket, context); - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t seed = 0; seed < 10; ++seed) { int location = 1 | (seed % 2 == 0 ? 0x8000 : 0); document::Document::SP doc(testDocMan.createRandomDocumentAtLocation( @@ -116,7 +114,7 @@ TEST_F(SplitBitDetectorTest, max_bits_one_below_max) { } TEST_F(SplitBitDetectorTest, unsplittable) { - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t i = 0; i < 10; ++i) { document::Document::SP doc( @@ -132,7 +130,7 @@ TEST_F(SplitBitDetectorTest, unsplittable) { } TEST_F(SplitBitDetectorTest, unsplittable_min_count) { - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; for (uint32_t i = 0; i < 10; ++i) { document::Document::SP doc( diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp index 8cf89b55ad0..146dcab2ba7 100644 --- a/storage/src/tests/persistence/testandsettest.cpp +++ b/storage/src/tests/persistence/testandsettest.cpp @@ -10,7 +10,6 @@ #include <vespa/document/fieldset/fieldsets.h> #include <vespa/persistence/spi/test.h> #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <functional> using std::unique_ptr; @@ -74,7 +73,7 @@ struct TestAndSetTest : PersistenceTestUtils { static std::string expectedDocEntryString( api::Timestamp timestamp, const document::DocumentId & testDocId, - spi::DocumentMetaEnum removeFlag = spi::DocumentMetaEnum::NONE); + spi::DocumentMetaFlags removeFlag = spi::NONE); }; TEST_F(TestAndSetTest, conditional_put_not_executed_on_condition_mismatch) { @@ -151,7 +150,7 @@ TEST_F(TestAndSetTest, conditional_remove_executed_on_condition_match) { ASSERT_EQ(fetchResult(asyncHandler->handleRemove(*remove, createTracker(remove, BUCKET))).getResult(), api::ReturnCode::Result::OK); EXPECT_EQ(expectedDocEntryString(timestampOne, testDocId) + - expectedDocEntryString(timestampTwo, testDocId, spi::DocumentMetaEnum::REMOVE_ENTRY), + expectedDocEntryString(timestampTwo, testDocId, spi::REMOVE_ENTRY), dumpBucket(BUCKET_ID)); } @@ -292,12 +291,12 @@ void TestAndSetTest::assertTestDocumentFoundAndMatchesContent(const document::Fi std::string TestAndSetTest::expectedDocEntryString( api::Timestamp timestamp, const document::DocumentId & docId, - spi::DocumentMetaEnum removeFlag) + spi::DocumentMetaFlags removeFlag) { std::stringstream ss; - ss << "DocEntry(" << timestamp << ", " << int(removeFlag) << ", "; - if (removeFlag == spi::DocumentMetaEnum::REMOVE_ENTRY) { + ss << "DocEntry(" << timestamp << ", " << removeFlag << ", "; + if (removeFlag == spi::REMOVE_ENTRY) { ss << docId << ")\n"; } else { ss << "Doc(" << docId << "))\n"; diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 945a08d910e..494af8a0eff 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -16,7 +16,6 @@ #include <tests/common/teststorageapp.h> #include <tests/common/dummystoragelink.h> #include <tests/storageserver/testvisitormessagesession.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/gtest/gtest.h> #include <thread> @@ -24,8 +23,6 @@ using namespace std::chrono_literals; using document::test::makeBucketSpace; -using document::Document; -using document::DocumentId; using namespace ::testing; namespace storage { @@ -57,7 +54,7 @@ struct TestParams { struct VisitorTest : Test { static uint32_t docCount; - std::vector<Document::SP> _documents; + std::vector<document::Document::SP > _documents; std::unique_ptr<TestVisitorMessageSessionFactory> _messageSessionFactory; std::unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<DummyStorageLink> _top; @@ -95,11 +92,11 @@ struct VisitorTest : Test { void getMessagesAndReply( int expectedCount, TestVisitorMessageSession& session, - std::vector<Document::SP> & docs, - std::vector<DocumentId>& docIds, + std::vector<document::Document::SP >& docs, + std::vector<document::DocumentId>& docIds, std::vector<std::string>& infoMessages, api::ReturnCode::Result returnCode = api::ReturnCode::OK); - uint32_t getMatchingDocuments(std::vector<Document::SP>& docs); + uint32_t getMatchingDocuments(std::vector<document::Document::SP >& docs); protected: void doTestVisitorInstanceHasConsistencyLevel( @@ -215,7 +212,7 @@ VisitorTest::initializeTest(const TestParams& params) uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" << i << ".html"; - _documents.push_back(Document::SP( + _documents.push_back(document::Document::SP( _node->getTestDocMan().createDocument(content, uri.str()))); const document::DocumentType& type(_documents.back()->getType()); _documents.back()->setValue(type.getField("headerval"), @@ -278,8 +275,8 @@ void VisitorTest::getMessagesAndReply( int expectedCount, TestVisitorMessageSession& session, - std::vector<Document::SP >& docs, - std::vector<DocumentId>& docIds, + std::vector<document::Document::SP >& docs, + std::vector<document::DocumentId>& docIds, std::vector<std::string>& infoMessages, api::ReturnCode::Result result) { @@ -354,7 +351,7 @@ VisitorTest::verifyCreateVisitorReply( } uint32_t -VisitorTest::getMatchingDocuments(std::vector<Document::SP >& docs) { +VisitorTest::getMatchingDocuments(std::vector<document::Document::SP >& docs) { uint32_t equalCount = 0; for (uint32_t i=0; i<docs.size(); ++i) { for (uint32_t j=0; j<_documents.size(); ++j) { @@ -384,7 +381,11 @@ VisitorTest::sendGetIterReply(GetIterCommand& cmd, assert(maxDocuments < _documents.size()); size_t documentCount = maxDocuments != 0 ? maxDocuments : _documents.size(); for (size_t i = 0; i < documentCount; ++i) { - reply->getEntries().push_back(spi::DocEntry::create(spi::Timestamp(1000 + i), Document::UP(_documents[i]->clone()))); + reply->getEntries().emplace_back( + std::make_unique<spi::DocEntry>( + spi::Timestamp(1000 + i), + spi::NONE, + document::Document::UP(_documents[i]->clone()))); } if (documentCount == _documents.size() || overrideCompleted) { reply->setCompleted(); @@ -480,8 +481,8 @@ TEST_F(VisitorTest, normal_usage) { sendGetIterReply(*getIterCmd); - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; getMessagesAndReply(_documents.size(), getSession(0), docs, docIds, infoMessages); ASSERT_EQ(0, infoMessages.size()); @@ -546,8 +547,8 @@ TEST_F(VisitorTest, document_api_client_error) { sendGetIterReply(*getIterCmd, api::ReturnCode(api::ReturnCode::OK), 1); } - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; getMessagesAndReply(1, getSession(0), docs, docIds, infoMessages, api::ReturnCode::INTERNAL_FAILURE); @@ -586,8 +587,8 @@ TEST_F(VisitorTest, no_document_api_resending_for_failed_visitor) { sendGetIterReply(*getIterCmd, api::ReturnCode(api::ReturnCode::OK), 2, true); } - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; // Use non-critical result. Visitor info message should be received // after we send a NOT_CONNECTED reply. Failing this message as well @@ -689,8 +690,8 @@ TEST_F(VisitorTest, no_visitor_notification_for_transient_failures) { ASSERT_NO_FATAL_FAILURE(initializeTest()); ASSERT_NO_FATAL_FAILURE(sendInitialCreateVisitorAndGetIterRound()); - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; // Have to make sure time increases in visitor thread so that resend // times are reached. @@ -733,8 +734,8 @@ TEST_F(VisitorTest, notification_sent_if_transient_error_retried_many_times) { ASSERT_NO_FATAL_FAILURE(initializeTest()); sendInitialCreateVisitorAndGetIterRound(); - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; // Have to make sure time increases in visitor thread so that resend // times are reached. @@ -773,8 +774,8 @@ VisitorTest::doCompleteVisitingSession( 1, true); - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; getMessagesAndReply(1, getSession(0), docs, docIds, infoMessages); @@ -834,8 +835,8 @@ TEST_F(VisitorTest, no_more_iterators_sent_while_memory_used_above_limit) { std::this_thread::sleep_for(100ms); ASSERT_EQ(0, _bottom->getNumCommands()); - std::vector<Document::SP> docs; - std::vector<DocumentId> docIds; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> docIds; std::vector<std::string> infoMessages; getMessagesAndReply(1, getSession(0), docs, docIds, infoMessages); @@ -897,8 +898,8 @@ struct ReindexingVisitorTest : VisitorTest { void respond_to_client_put(api::ReturnCode::Result result) { // Reply to the Put from "client" back to the visitor - std::vector<Document::SP> docs; - std::vector<DocumentId> doc_ids; + std::vector<document::Document::SP> docs; + std::vector<document::DocumentId> doc_ids; std::vector<std::string> info_messages; getMessagesAndReply(1, getSession(0), docs, doc_ids, info_messages, result); } diff --git a/storage/src/vespa/storage/persistence/asynchandler.cpp b/storage/src/vespa/storage/persistence/asynchandler.cpp index 3d24ee87879..b5161673af3 100644 --- a/storage/src/vespa/storage/persistence/asynchandler.cpp +++ b/storage/src/vespa/storage/persistence/asynchandler.cpp @@ -6,7 +6,6 @@ #include "bucketownershipnotifier.h" #include "bucketprocessor.h" #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/persistence/spi/catchresult.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/document/update/documentupdate.h> diff --git a/storage/src/vespa/storage/persistence/bucketprocessor.h b/storage/src/vespa/storage/persistence/bucketprocessor.h index 002cba6c15c..c605a9338e3 100644 --- a/storage/src/vespa/storage/persistence/bucketprocessor.h +++ b/storage/src/vespa/storage/persistence/bucketprocessor.h @@ -7,14 +7,11 @@ #pragma once #include <vespa/persistence/spi/bucket.h> +#include <vespa/persistence/spi/docentry.h> #include <vespa/persistence/spi/context.h> -#include <persistence/spi/types.h> namespace document { class FieldSet; } -namespace storage::spi { - struct PersistenceProvider; - class DocEntry; -} +namespace storage::spi { struct PersistenceProvider; } namespace storage { diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index b9739fcf734..0c9cecdb6a1 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -6,7 +6,7 @@ #include "apply_bucket_diff_state.h" #include <vespa/storage/persistence/filestorage/mergestatus.h> #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> +#include <vespa/persistence/spi/catchresult.h> #include <vespa/vdslib/distribution/distribution.h> #include <vespa/document/fieldset/fieldsets.h> #include <vespa/vespalib/objects/nbostream.h> @@ -130,8 +130,11 @@ void update_op_metrics(FileStorThreadMetrics& metrics, const api::StorageReply & } // anonymous namespace void -MergeHandler::populateMetaData(const spi::Bucket& bucket, Timestamp maxTimestamp, - DocEntryList& entries, spi::Context& context) const +MergeHandler::populateMetaData( + const spi::Bucket& bucket, + Timestamp maxTimestamp, + std::vector<spi::DocEntry::UP>& entries, + spi::Context& context) const { spi::DocumentSelection docSel(""); @@ -147,7 +150,9 @@ MergeHandler::populateMetaData(const spi::Bucket& bucket, Timestamp maxTimestamp if (createIterResult.getErrorCode() != spi::Result::ErrorType::NONE) { std::ostringstream ss; ss << "Failed to create iterator for " - << bucket << ": " << createIterResult.getErrorMessage(); + << bucket + << ": " + << createIterResult.getErrorMessage(); throw std::runtime_error(ss.str()); } spi::IteratorId iteratorId(createIterResult.getIteratorId()); @@ -158,7 +163,9 @@ MergeHandler::populateMetaData(const spi::Bucket& bucket, Timestamp maxTimestamp if (result.getErrorCode() != spi::Result::ErrorType::NONE) { std::ostringstream ss; ss << "Failed to iterate for " - << bucket << ": " << result.getErrorMessage(); + << bucket + << ": " + << result.getErrorMessage(); throw std::runtime_error(ss.str()); } auto list = result.steal_entries(); @@ -234,7 +241,7 @@ MergeHandler::buildBucketInfoList( } } - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; populateMetaData(bucket, maxTimestamp, entries, context); for (const auto& entry : entries) { @@ -395,7 +402,7 @@ MergeHandler::fetchLocalData( IteratorGuard iteratorGuard(_spi, iteratorId, context); // Fetch all entries - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; entries.reserve(slots.size()); bool fetchedAllLocalData = false; bool chunkLimitReached = false; @@ -550,7 +557,7 @@ MergeHandler::applyDiffLocally( uint32_t notNeededByteCount = 0; async_results->mark_stale_bucket_info(); - DocEntryList entries; + std::vector<spi::DocEntry::UP> entries; populateMetaData(bucket, MAX_TIMESTAMP, entries, context); const document::DocumentTypeRepo & repo = _env.getDocumentTypeRepo(); diff --git a/storage/src/vespa/storage/persistence/mergehandler.h b/storage/src/vespa/storage/persistence/mergehandler.h index f52fe63bc2b..5e66b364242 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.h +++ b/storage/src/vespa/storage/persistence/mergehandler.h @@ -16,6 +16,7 @@ #include "types.h" #include "merge_bucket_info_syncer.h" #include <vespa/persistence/spi/bucket.h> +#include <vespa/persistence/spi/docentry.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/storage/common/cluster_context.h> #include <vespa/storage/common/messagesender.h> @@ -29,9 +30,9 @@ namespace storage { namespace spi { struct PersistenceProvider; class Context; - class DocEntry; } class PersistenceUtil; +class ApplyBucketDiffEntryResult; class ApplyBucketDiffState; class MergeStatus; @@ -81,7 +82,6 @@ public: void configure(bool async_apply_bucket_diff) noexcept; private: - using DocEntryList = std::vector<std::unique_ptr<spi::DocEntry>>; const framework::Clock &_clock; const ClusterContext &_cluster_context; PersistenceUtil &_env; @@ -117,7 +117,7 @@ private: */ void populateMetaData(const spi::Bucket&, Timestamp maxTimestamp, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, spi::Context& context) const; Document::UP deserializeDiffDocument( diff --git a/storage/src/vespa/storage/persistence/messages.cpp b/storage/src/vespa/storage/persistence/messages.cpp index cf05ccd65b8..c6f7442cfdc 100644 --- a/storage/src/vespa/storage/persistence/messages.cpp +++ b/storage/src/vespa/storage/persistence/messages.cpp @@ -1,8 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "messages.h" -#include <vespa/persistence/spi/docentry.h> #include <ostream> +#include <cassert> using document::BucketSpace; diff --git a/storage/src/vespa/storage/persistence/messages.h b/storage/src/vespa/storage/persistence/messages.h index 86cf1ac3b06..594506f53ba 100644 --- a/storage/src/vespa/storage/persistence/messages.h +++ b/storage/src/vespa/storage/persistence/messages.h @@ -2,6 +2,7 @@ #pragma once #include <vespa/storageapi/message/internal.h> +#include <vespa/persistence/spi/docentry.h> #include <vespa/persistence/spi/bucket.h> #include <vespa/persistence/spi/selection.h> #include <vespa/persistence/spi/read_consistency.h> @@ -9,8 +10,6 @@ namespace storage { -namespace spi { class DocEntry; } - class GetIterCommand : public api::InternalCommand { private: document::Bucket _bucket; @@ -45,10 +44,9 @@ private: class GetIterReply : public api::InternalReply { private: - using List = std::vector<std::unique_ptr<spi::DocEntry>>; document::Bucket _bucket; - List _entries; - bool _completed; + std::vector<spi::DocEntry::UP> _entries; + bool _completed; public: typedef std::unique_ptr<GetIterReply> UP; @@ -60,9 +58,13 @@ public: document::Bucket getBucket() const override { return _bucket; } - const List & getEntries() const { return _entries; } + const std::vector<spi::DocEntry::UP>& getEntries() const { + return _entries; + } - List & getEntries() { return _entries; } + std::vector<spi::DocEntry::UP>& getEntries() { + return _entries; + } void setCompleted(bool completed = true) { _completed = completed; } bool isCompleted() const { return _completed; } diff --git a/storage/src/vespa/storage/persistence/processallhandler.cpp b/storage/src/vespa/storage/persistence/processallhandler.cpp index 6d6723a0185..6a2b5e79450 100644 --- a/storage/src/vespa/storage/persistence/processallhandler.cpp +++ b/storage/src/vespa/storage/persistence/processallhandler.cpp @@ -5,7 +5,6 @@ #include "persistenceutil.h" #include <vespa/document/fieldset/fieldsets.h> #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/log/log.h> @@ -32,7 +31,7 @@ public: if (e.getDocument() != nullptr) { ost << "Doc(" << e.getDocument()->getId() << ")" << ", " << e.getDocument()->getId().getGlobalId().toString() - << ", size: " << e.getSize(); + << ", size: " << e.getPersistedDocumentSize(); } else if (e.getDocumentId() != nullptr) { ost << *e.getDocumentId() << ", " << e.getDocumentId()->getGlobalId().toString(); diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp index b7344098698..752c1175f22 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp @@ -2,7 +2,6 @@ #include "provider_error_wrapper.h" #include "persistenceutil.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/vespalib/util/idestructorcallback.h> namespace storage { diff --git a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp index 74813e2e891..9a7a451b906 100644 --- a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp +++ b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp @@ -3,7 +3,6 @@ #include "simplemessagehandler.h" #include "persistenceutil.h" #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/document/base/exceptions.h> #include <vespa/document/fieldset/fieldsetrepo.h> diff --git a/storage/src/vespa/storage/persistence/splitbitdetector.cpp b/storage/src/vespa/storage/persistence/splitbitdetector.cpp index 2a9ac635cff..bd472868e06 100644 --- a/storage/src/vespa/storage/persistence/splitbitdetector.cpp +++ b/storage/src/vespa/storage/persistence/splitbitdetector.cpp @@ -3,7 +3,6 @@ #include "splitbitdetector.h" #include "bucketprocessor.h" #include <vespa/persistence/spi/persistenceprovider.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/bucket/bucketidfactory.h> #include <vespa/document/base/documentid.h> #include <vespa/document/fieldset/fieldsets.h> diff --git a/storage/src/vespa/storage/visiting/countvisitor.cpp b/storage/src/vespa/storage/visiting/countvisitor.cpp index 3971544a9a0..e20699aa799 100644 --- a/storage/src/vespa/storage/visiting/countvisitor.cpp +++ b/storage/src/vespa/storage/visiting/countvisitor.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "countvisitor.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/fieldvalue/document.h> #include <vespa/documentapi/messagebus/messages/visitor.h> #include <vespa/vespalib/util/stringfmt.h> @@ -23,7 +22,7 @@ CountVisitor::CountVisitor(StorageComponent& component, void CountVisitor::handleDocuments(const document::BucketId& /*bucketId*/, - DocEntryList& entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) { for (size_t i = 0; i < entries.size(); ++i) { diff --git a/storage/src/vespa/storage/visiting/countvisitor.h b/storage/src/vespa/storage/visiting/countvisitor.h index 4120a96225f..4c436b3d07c 100644 --- a/storage/src/vespa/storage/visiting/countvisitor.h +++ b/storage/src/vespa/storage/visiting/countvisitor.h @@ -18,11 +18,11 @@ public: CountVisitor(StorageComponent&, const vdslib::Parameters& params); - void completedVisiting(HitCounter&) override; + virtual void completedVisiting(HitCounter&) override; private: void handleDocuments(const document::BucketId& bucketId, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) override; bool _doScheme; diff --git a/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp b/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp index 3419d329a06..704aaa219cb 100644 --- a/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp +++ b/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "dumpvisitorsingle.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/update/documentupdate.h> #include <vespa/document/fieldvalue/document.h> #include <vespa/documentapi/messagebus/messages/putdocumentmessage.h> @@ -17,8 +16,8 @@ DumpVisitorSingle::DumpVisitorSingle(StorageComponent& component, const vdslib:: { } -void DumpVisitorSingle::handleDocuments(const document::BucketId&, - DocEntryList& entries, +void DumpVisitorSingle::handleDocuments(const document::BucketId& /*bucketId*/, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) { LOG(debug, "Visitor %s handling block of %zu documents.", @@ -26,7 +25,7 @@ void DumpVisitorSingle::handleDocuments(const document::BucketId&, for (size_t i = 0; i < entries.size(); ++i) { spi::DocEntry& entry(*entries[i]); - const uint32_t docSize = entry.getSize(); + const uint32_t docSize = entry.getDocumentSize(); if (entry.isRemove()) { hitCounter.addHit(*entry.getDocumentId(), docSize); sendMessage(std::make_unique<documentapi::RemoveDocumentMessage>(*entry.getDocumentId())); diff --git a/storage/src/vespa/storage/visiting/dumpvisitorsingle.h b/storage/src/vespa/storage/visiting/dumpvisitorsingle.h index c98bad17e84..81f4ab7e989 100644 --- a/storage/src/vespa/storage/visiting/dumpvisitorsingle.h +++ b/storage/src/vespa/storage/visiting/dumpvisitorsingle.h @@ -19,7 +19,7 @@ public: const vdslib::Parameters& params); private: - void handleDocuments(const document::BucketId&, DocEntryList&, HitCounter&) override; + void handleDocuments(const document::BucketId&, std::vector<spi::DocEntry::UP>&, HitCounter&) override; }; struct DumpVisitorSingleFactory : public VisitorFactory { diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp index 1c3c97d7dcb..df965f93ae6 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp @@ -2,7 +2,6 @@ #include "recoveryvisitor.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/documentapi/messagebus/messages/visitor.h> #include <vespa/vespalib/text/stringtokenizer.h> @@ -32,7 +31,7 @@ RecoveryVisitor::RecoveryVisitor(StorageComponent& component, void RecoveryVisitor::handleDocuments(const document::BucketId& bid, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) { std::lock_guard guard(_mutex); diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.h b/storage/src/vespa/storage/visiting/recoveryvisitor.h index e850eca3f37..045c8aeb484 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.h +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.h @@ -22,7 +22,7 @@ public: private: void handleDocuments(const document::BucketId& bucketId, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) override; void completedBucket(const document::BucketId&, HitCounter&) override; diff --git a/storage/src/vespa/storage/visiting/reindexing_visitor.cpp b/storage/src/vespa/storage/visiting/reindexing_visitor.cpp index 0b08c52bdc4..528f83e29cc 100644 --- a/storage/src/vespa/storage/visiting/reindexing_visitor.cpp +++ b/storage/src/vespa/storage/visiting/reindexing_visitor.cpp @@ -3,7 +3,6 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/documentapi/messagebus/messages/putdocumentmessage.h> #include <vespa/storage/common/reindexing_constants.h> -#include <vespa/persistence/spi/docentry.h> #include <vespa/log/log.h> LOG_SETUP(".visitor.instance.reindexing_visitor"); @@ -15,8 +14,8 @@ ReindexingVisitor::ReindexingVisitor(StorageComponent& component) { } -void ReindexingVisitor::handleDocuments(const document::BucketId& , - DocEntryList & entries, +void ReindexingVisitor::handleDocuments(const document::BucketId& /*bucketId*/, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) { auto lock_token = make_lock_access_token(); @@ -27,7 +26,7 @@ void ReindexingVisitor::handleDocuments(const document::BucketId& , // We don't reindex removed documents, as that would be very silly. continue; } - const uint32_t doc_size = entry->getSize(); + const uint32_t doc_size = entry->getDocumentSize(); hitCounter.addHit(*entry->getDocumentId(), doc_size); auto msg = std::make_unique<documentapi::PutDocumentMessage>(entry->releaseDocument()); msg->setApproxSize(doc_size); diff --git a/storage/src/vespa/storage/visiting/reindexing_visitor.h b/storage/src/vespa/storage/visiting/reindexing_visitor.h index d9e18542818..9bde9903617 100644 --- a/storage/src/vespa/storage/visiting/reindexing_visitor.h +++ b/storage/src/vespa/storage/visiting/reindexing_visitor.h @@ -20,7 +20,7 @@ public: ~ReindexingVisitor() override = default; private: - void handleDocuments(const document::BucketId&, DocEntryList&, HitCounter&) override; + void handleDocuments(const document::BucketId&, std::vector<spi::DocEntry::UP>&, HitCounter&) override; bool remap_docapi_message_error_code(api::ReturnCode& in_out_code) override; vespalib::string make_lock_access_token() const; }; diff --git a/storage/src/vespa/storage/visiting/testvisitor.cpp b/storage/src/vespa/storage/visiting/testvisitor.cpp index 10c79ea6f18..b5ace673e48 100644 --- a/storage/src/vespa/storage/visiting/testvisitor.cpp +++ b/storage/src/vespa/storage/visiting/testvisitor.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "testvisitor.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/documentapi/messagebus/messages/visitor.h> #include <sstream> @@ -40,7 +39,7 @@ TestVisitor::startingVisitor(const std::vector<document::BucketId>& buckets) void TestVisitor::handleDocuments(const document::BucketId& /*bucketId*/, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& /*hitCounter*/) { std::ostringstream ost; diff --git a/storage/src/vespa/storage/visiting/testvisitor.h b/storage/src/vespa/storage/visiting/testvisitor.h index 989581ac121..482b1347faa 100644 --- a/storage/src/vespa/storage/visiting/testvisitor.h +++ b/storage/src/vespa/storage/visiting/testvisitor.h @@ -20,7 +20,7 @@ private: void startingVisitor(const std::vector<document::BucketId>& buckets) override; void handleDocuments(const document::BucketId& bucketId, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) override; void completedBucket(const document::BucketId& bucket, HitCounter& hitCounter) override; diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index b66285f5048..dfb78122e07 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -2,7 +2,6 @@ #include "visitor.h" #include "visitormetrics.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/storageframework/generic/clock/timer.h> #include <vespa/storageapi/message/datagram.h> #include <vespa/storage/persistence/messages.h> @@ -815,11 +814,12 @@ Visitor::onGetIterReply(const std::shared_ptr<GetIterReply>& reply, uint64_t size = 0; for (const auto& entry : reply->getEntries()) { - size += entry->getSize(); + size += entry->getPersistedDocumentSize(); } _visitorStatistics.setDocumentsVisited( - _visitorStatistics.getDocumentsVisited() + reply->getEntries().size()); + _visitorStatistics.getDocumentsVisited() + + reply->getEntries().size()); _visitorStatistics.setBytesVisited(_visitorStatistics.getBytesVisited() + size); } catch (std::exception& e) { LOG(warning, "handleDocuments threw exception %s", e.what()); diff --git a/storage/src/vespa/storage/visiting/visitor.h b/storage/src/vespa/storage/visiting/visitor.h index 8857a54e8df..52b2d586c78 100644 --- a/storage/src/vespa/storage/visiting/visitor.h +++ b/storage/src/vespa/storage/visiting/visitor.h @@ -18,6 +18,7 @@ #include <vespa/storage/common/storagecomponent.h> #include <vespa/storage/common/visitorfactory.h> #include <vespa/documentapi/messagebus/messages/documentmessage.h> +#include <vespa/persistence/spi/docentry.h> #include <vespa/persistence/spi/selection.h> #include <vespa/persistence/spi/read_consistency.h> #include <list> @@ -38,10 +39,6 @@ namespace documentapi { namespace storage { -namespace spi { - class DocEntry; -} - namespace api { class ReturnCode; class StorageCommand; @@ -360,7 +357,6 @@ protected: // error code, false if the DocumentAPI message should be retried later. [[nodiscard]] virtual bool remap_docapi_message_error_code(api::ReturnCode& in_out_code); public: - using DocEntryList = std::vector<std::unique_ptr<spi::DocEntry>>; Visitor(StorageComponent& component); virtual ~Visitor(); @@ -402,7 +398,7 @@ public: * vector of documents arrive from the persistence layer. */ virtual void handleDocuments(const document::BucketId&, - DocEntryList & entries, + std::vector<spi::DocEntry::UP>& entries, HitCounter& hitCounter) = 0; /** diff --git a/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp b/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp index a936146fd26..b6244521a46 100644 --- a/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp +++ b/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp @@ -10,8 +10,6 @@ #include <vespa/searchvisitor/searchvisitor.h> #include <vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h> #include <vespa/storageframework/defaultimplementation/clock/fakeclock.h> -#include <vespa/persistence/spi/docentry.h> - #include <vespa/log/log.h> LOG_SETUP("searchvisitor_test"); @@ -55,13 +53,14 @@ SearchVisitorTest::SearchVisitorTest() : SearchVisitorTest::~SearchVisitorTest() = default; -Visitor::DocEntryList +std::vector<spi::DocEntry::UP> createDocuments(const vespalib::string & dir) { (void) dir; - Visitor::DocEntryList documents; + std::vector<spi::DocEntry::UP> documents; spi::Timestamp ts; - auto e = spi::DocEntry::create(ts, std::make_unique<Document>()); + document::Document::UP doc(new document::Document()); + spi::DocEntry::UP e(new spi::DocEntry(ts, 0, std::move(doc))); documents.push_back(std::move(e)); return documents; } @@ -73,7 +72,7 @@ SearchVisitorTest::testCreateSearchVisitor(const vespalib::string & dir, const v VisitorFactory & factory(sFactory); std::unique_ptr<Visitor> sv(static_cast<SearchVisitor *>(factory.makeVisitor(*_component, _env, params))); document::BucketId bucketId; - Visitor::DocEntryList documents(createDocuments(dir)); + std::vector<spi::DocEntry::UP> documents(createDocuments(dir)); Visitor::HitCounter hitCounter; sv->handleDocuments(bucketId, documents, hitCounter); } diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index 460a0886ee2..cc6323c3053 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -3,7 +3,6 @@ #include "querytermdata.h" #include "searchenvironment.h" #include "searchvisitor.h" -#include <vespa/persistence/spi/docentry.h> #include <vespa/document/datatype/positiondatatype.h> #include <vespa/document/datatype/documenttype.h> #include <vespa/document/datatype/weightedsetdatatype.h> @@ -865,7 +864,7 @@ SearchVisitor::compatibleDocumentTypes(const document::DocumentType& typeA, void SearchVisitor::handleDocuments(const document::BucketId&, - DocEntryList & entries, + std::vector<storage::spi::DocEntry::UP>& entries, HitCounter& hitCounter) { (void) hitCounter; diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h index 20ab1ccf325..dff263b8418 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h @@ -298,7 +298,7 @@ private: // Inherit doc from Visitor void handleDocuments(const document::BucketId&, - DocEntryList& entries, + std::vector<storage::spi::DocEntry::UP>& entries, HitCounter& hitCounter) override; bool compatibleDocumentTypes(const document::DocumentType& typeA, |