diff options
13 files changed, 175 insertions, 194 deletions
diff --git a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp index 21cf7cb9ae3..3b30f5e29ca 100644 --- a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp +++ b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp @@ -9,7 +9,6 @@ #include <vespa/vdslib/state/clusterstate.h> #include <vespa/config-stor-distribution.h> - using namespace storage::spi; using namespace storage; using document::test::makeBucketSpace; @@ -21,7 +20,7 @@ struct Fixture { BucketContent content; void insert(DocumentId id, Timestamp timestamp, int meta_flags) { - content.insert(DocEntry::UP(new DocEntry(timestamp, meta_flags, id))); + content.insert(DocEntry::create(timestamp, meta_flags, id)); } Fixture() { diff --git a/persistence/src/tests/spi/clusterstatetest.cpp b/persistence/src/tests/spi/clusterstatetest.cpp index 9d03cb982c8..205661ba930 100644 --- a/persistence/src/tests/spi/clusterstatetest.cpp +++ b/persistence/src/tests/spi/clusterstatetest.cpp @@ -262,62 +262,42 @@ TEST(ClusterStateTest, node_maintenance_state_is_set_independent_of_bucket_space } TEST(DocEntryTest, test_basics) { - EXPECT_EQ(40u, sizeof(DocEntry)); + EXPECT_EQ(24, sizeof(DocEntry)); } TEST(DocEntryTest, test_meta_only) { - DocEntry e(Timestamp(9), 0); - EXPECT_EQ(9, e.getTimestamp()); - EXPECT_FALSE(e.isRemove()); - EXPECT_EQ(40, e.getSize()); - EXPECT_EQ(0, e.getDocumentSize()); - EXPECT_EQ(0, e.getPersistedDocumentSize()); - EXPECT_EQ(nullptr, e.getDocument()); - EXPECT_EQ(nullptr, e.getDocumentId()); - - e.setPersistedDocumentSize(7); - EXPECT_EQ(40, e.getSize()); - EXPECT_EQ(0, e.getDocumentSize()); - EXPECT_EQ(7, e.getPersistedDocumentSize()); - - DocEntry r(Timestamp(666), 1); - EXPECT_EQ(666, r.getTimestamp()); - EXPECT_TRUE(r.isRemove()); + DocEntry::UP e = DocEntry::create(Timestamp(9), 0); + EXPECT_EQ(9, e->getTimestamp()); + EXPECT_FALSE(e->isRemove()); + EXPECT_EQ(24, e->getSize()); + EXPECT_EQ(0, e->getDocumentSize()); + EXPECT_EQ(nullptr, e->getDocument()); + EXPECT_EQ(nullptr, e->getDocumentId()); + + DocEntry::UP r = DocEntry::create(Timestamp(666), 1); + EXPECT_EQ(666, r->getTimestamp()); + EXPECT_TRUE(r->isRemove()); } TEST(DocEntryTest, test_docid_only) { - DocEntry e(Timestamp(9), 0, DocumentId("id:test:test::1")); - EXPECT_EQ(9, e.getTimestamp()); - EXPECT_FALSE(e.isRemove()); - EXPECT_EQ(56, e.getSize()); - EXPECT_EQ(16, e.getDocumentSize()); - EXPECT_EQ(16, e.getPersistedDocumentSize()); - EXPECT_EQ(nullptr, e.getDocument()); - EXPECT_NE(nullptr, e.getDocumentId()); - - e.setPersistedDocumentSize(7); - EXPECT_EQ(56, e.getSize()); - EXPECT_EQ(16, e.getDocumentSize()); - EXPECT_EQ(7, e.getPersistedDocumentSize()); - + DocEntry::UP e = DocEntry::create(Timestamp(9), 0, DocumentId("id:test:test::1")); + EXPECT_EQ(9, e->getTimestamp()); + EXPECT_FALSE(e->isRemove()); + EXPECT_EQ(48, e->getSize()); + EXPECT_EQ(16, e->getDocumentSize()); + EXPECT_EQ(nullptr, e->getDocument()); + EXPECT_NE(nullptr, e->getDocumentId()); } TEST(DocEntryTest, test_document_only) { document::TestDocMan testDocMan; - DocEntry e(Timestamp(9), 0, testDocMan.createRandomDocument(0, 1000)); - EXPECT_EQ(9, e.getTimestamp()); - EXPECT_FALSE(e.isRemove()); - EXPECT_EQ(672, e.getSize()); - EXPECT_EQ(632, e.getDocumentSize()); - EXPECT_EQ(632, e.getPersistedDocumentSize()); - EXPECT_NE(nullptr, e.getDocument()); - EXPECT_NE(nullptr, e.getDocumentId()); - - e.setPersistedDocumentSize(7); - EXPECT_EQ(672, e.getSize()); - EXPECT_EQ(632, e.getDocumentSize()); - EXPECT_EQ(7, e.getPersistedDocumentSize()); - + DocEntry::UP e = DocEntry::create(Timestamp(9), 0, testDocMan.createRandomDocument(0, 1000)); + EXPECT_EQ(9, e->getTimestamp()); + EXPECT_FALSE(e->isRemove()); + EXPECT_EQ(664, e->getSize()); + EXPECT_EQ(632, e->getDocumentSize()); + EXPECT_NE(nullptr, e->getDocument()); + EXPECT_NE(nullptr, e->getDocumentId()); } } diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index 57b6b282e60..ec68f02f8b0 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -258,14 +258,14 @@ verifyDocs(const std::vector<DocAndTimestamp>& wanted, } EXPECT_EQ(wanted[wantedIdx].timestamp, entry.getTimestamp()); size_t serSize = wanted[wantedIdx].doc->serialize().size(); - EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize())); + EXPECT_EQ(serSize + entry.getOwnSize(), 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 + sizeof(DocEntry), size_t(entry.getSize())); + EXPECT_EQ(serSize + entry.getOwnSize(), 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 " diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index 142b7faad9d..19c03a97f05 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -25,6 +25,7 @@ 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; @@ -434,7 +435,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 = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone())); + auto entry = DocEntry::create(t, NONE, Document::UP(doc->clone())); (*bc)->insert(std::move(entry)); bc.reset(); onComplete->onComplete(std::make_unique<Result>()); @@ -492,7 +493,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI } DocEntry::SP entry((*bc)->getEntry(id)); numRemoves += (entry.get() && !entry->isRemove()) ? 1 : 0; - auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, id); + auto remEntry = DocEntry::create(t, REMOVE_ENTRY, id); if ((*bc)->hasTimestamp(t)) { (*bc)->eraseEntry(t); @@ -504,7 +505,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI } GetResult -DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const DocumentId& did, Context&) const +DummyPersistence::get(const Bucket& b, const FieldSet& fieldSet, const DocumentId& did, Context&) const { DUMMYPERSISTENCE_VERIFY_INITIALIZED; LOG(debug, "get(%s, %s)", @@ -521,8 +522,8 @@ DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const return GetResult::make_for_tombstone(entry->getTimestamp()); } else { Document::UP doc(entry->getDocument()->clone()); - if (fieldSet.getType() != document::FieldSet::Type::ALL) { - document::FieldSet::stripFields(*doc, fieldSet); + if (fieldSet.getType() != FieldSet::Type::ALL) { + FieldSet::stripFields(*doc, fieldSet); } return GetResult(std::move(doc), entry->getTimestamp()); } @@ -652,18 +653,13 @@ 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() != document::FieldSet::Type::ALL) + && it->_fieldSet->getType() != FieldSet::Type::ALL) { assert(entry->getDocument()); // Create new document with only wanted fields. - Document::UP filtered( - document::FieldSet::createDocumentSubsetCopy( - *entry->getDocument(), - *it->_fieldSet)); - DocEntry::UP ret(new DocEntry(entry->getTimestamp(), - entry->getFlags(), - std::move(filtered), - entry->getPersistedDocumentSize())); + Document::UP filtered(FieldSet::createDocumentSubsetCopy(*entry->getDocument(), *it->_fieldSet)); + auto ret = DocEntry::create(entry->getTimestamp(), entry->getFlags(), + std::move(filtered), entry->getDocumentSize()); entries.push_back(std::move(ret)); } else { // Use entry as-is. diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp index a48f0656b23..e586b4e414f 100644 --- a/persistence/src/vespa/persistence/spi/docentry.cpp +++ b/persistence/src/vespa/persistence/spi/docentry.cpp @@ -4,73 +4,75 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/vespalib/objects/nbostream.h> #include <sstream> -#include <cassert> namespace storage::spi { -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)) -{ } +namespace { + +class DocEntryWithId final : public DocEntry { +public: + DocEntryWithId(Timestamp t, int metaFlags, const DocumentId &docId); + ~DocEntryWithId(); + vespalib::string toString() const override; + const DocumentId* getDocumentId() const override { return _documentId.get(); } +private: + SizeType getOwnSize() const override{ return sizeof(DocEntryWithId); } + DocumentIdUP _documentId; +}; + +class DocEntryWithDoc final : public DocEntry { +public: + DocEntryWithDoc(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. This value shall be the size of the document _before_ + * any field filtering is performed. + */ + DocEntryWithDoc(Timestamp t, int metaFlags, 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); } +private: + SizeType getOwnSize() const override { return sizeof(DocEntryWithDoc); } + DocumentUP _document; +}; -DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize) - : _timestamp(t), - _metaFlags(metaFlags), - _persistedDocumentSize(serializedDocumentSize), - _size(_persistedDocumentSize + sizeof(DocEntry)), - _documentId(), +DocEntryWithDoc::DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc) + : DocEntry(t, metaFlags, doc->serialize().size()), _document(std::move(doc)) { } -DocEntry::DocEntry(Timestamp t, int metaFlags, const DocumentId& docId) - : _timestamp(t), - _metaFlags(metaFlags), - _persistedDocumentSize(docId.getSerializedSize()), - _size(_persistedDocumentSize + sizeof(DocEntry)), - _documentId(std::make_unique<DocumentId>(docId)), - _document() +DocEntryWithDoc::DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize) + : DocEntry(t, metaFlags, serializedDocumentSize), + _document(std::move(doc)) { } -DocEntry::DocEntry(Timestamp t, int metaFlags) - : _timestamp(t), - _metaFlags(metaFlags), - _persistedDocumentSize(0), - _size(sizeof(DocEntry)), - _documentId(), - _document() +DocEntryWithId::DocEntryWithId(Timestamp t, int metaFlags, const DocumentId& docId) + : DocEntry(t, metaFlags, docId.getSerializedSize()), + _documentId(std::make_unique<DocumentId>(docId)) { } -DocEntry::~DocEntry() = default; +DocEntryWithId::~DocEntryWithId() = default; +DocEntryWithDoc::~DocEntryWithDoc() = default; -const DocumentId* -DocEntry::getDocumentId() const { - return (_document ? &_document->getId() : _documentId.get()); -} - -DocumentUP -DocEntry::releaseDocument() { - return std::move(_document); -} - -DocEntry::SizeType -DocEntry::getDocumentSize() const +vespalib::string +DocEntryWithId::toString() const { - assert(_size >= sizeof(DocEntry)); - return _size - sizeof(DocEntry); + std::ostringstream out; + out << "DocEntry(" << getTimestamp() << ", " << getFlags() << ", " << *_documentId << ")"; + return out.str(); } vespalib::string -DocEntry::toString() const +DocEntryWithDoc::toString() const { std::ostringstream out; - out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", "; - if (_documentId) { - out << *_documentId; - } else if (_document.get()) { + out << "DocEntry(" << getTimestamp() << ", " << getFlags() << ", "; + if (_document.get()) { out << "Doc(" << _document->getId() << ")"; } else { out << "metadata only"; @@ -79,6 +81,40 @@ DocEntry::toString() const return out.str(); } +} + +DocEntry::UP +DocEntry::create(Timestamp t, int metaFlags) { + return std::make_unique<DocEntry>(t, metaFlags); +} +DocEntry::UP +DocEntry::create(Timestamp t, int metaFlags, const DocumentId &docId) { + return std::make_unique<DocEntryWithId>(t, metaFlags, docId); +} +DocEntry::UP +DocEntry::create(Timestamp t, int metaFlags, DocumentUP doc) { + return std::make_unique<DocEntryWithDoc>(t, metaFlags, std::move(doc)); +} +DocEntry::UP +DocEntry::create(Timestamp t, int metaFlags, DocumentUP doc, SizeType serializedDocumentSize) { + return std::make_unique<DocEntryWithDoc>(t, metaFlags, std::move(doc), serializedDocumentSize); +} + +DocEntry::~DocEntry() = default; + +DocumentUP +DocEntry::releaseDocument() { + return {}; +} + +vespalib::string +DocEntry::toString() const +{ + std::ostringstream out; + out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", metadata only)"; + return out.str(); +} + std::ostream & operator << (std::ostream & os, const DocEntry & r) { return os << r.toString(); diff --git a/persistence/src/vespa/persistence/spi/docentry.h b/persistence/src/vespa/persistence/spi/docentry.h index 8109c7f04a5..90be8eedc0c 100644 --- a/persistence/src/vespa/persistence/spi/docentry.h +++ b/persistence/src/vespa/persistence/spi/docentry.h @@ -24,37 +24,16 @@ enum DocumentMetaFlags { class DocEntry { public: - typedef uint32_t SizeType; -private: - Timestamp _timestamp; - int _metaFlags; - SizeType _persistedDocumentSize; - SizeType _size; - DocumentIdUP _documentId; - DocumentUP _document; -public: + using SizeType = uint32_t; using UP = std::unique_ptr<DocEntry>; using SP = std::shared_ptr<DocEntry>; - 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(Timestamp t, int metaFlags) : DocEntry(t, metaFlags, 0) { } DocEntry(const DocEntry &) = delete; DocEntry & operator=(const DocEntry &) = delete; DocEntry(DocEntry &&) = delete; DocEntry & operator=(DocEntry &&) = delete; - ~DocEntry(); - const Document* getDocument() const { return _document.get(); } - const DocumentId* getDocumentId() const; - DocumentUP releaseDocument(); + virtual ~DocEntry(); bool isRemove() const { return (_metaFlags & REMOVE_ENTRY); } Timestamp getTimestamp() const { return _timestamp; } int getFlags() const { return _metaFlags; } @@ -62,33 +41,34 @@ public: * @return In-memory size of this doc entry, including document instance. * In essence: serialized size of document + sizeof(DocEntry). */ - SizeType getSize() const { return _size; } + SizeType getSize() const { return _size + getOwnSize() ; } + virtual SizeType getOwnSize() const { return sizeof(DocEntry); } /** * 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 zero. */ - 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; - } + SizeType getDocumentSize() const { return _size; } - vespalib::string toString() const; + virtual vespalib::string toString() const; + virtual const Document* getDocument() const { return nullptr; } + virtual const DocumentId* getDocumentId() const { return nullptr; } + virtual DocumentUP releaseDocument(); + static UP create(Timestamp t, int metaFlags); + static UP create(Timestamp t, int metaFlags, const DocumentId &docId); + static UP create(Timestamp t, int metaFlags, DocumentUP doc); + static UP create(Timestamp t, int metaFlags, DocumentUP doc, SizeType serializedDocumentSize); +protected: + DocEntry(Timestamp t, int metaFlags, SizeType size) + : _timestamp(t), + _metaFlags(metaFlags), + _size(size) + {} +private: + Timestamp _timestamp; + int _metaFlags; + SizeType _size; }; std::ostream & operator << (std::ostream & os, const DocEntry & r); diff --git a/persistence/src/vespa/persistence/spi/test.cpp b/persistence/src/vespa/persistence/spi/test.cpp index c61f21a3893..0f10b839f98 100644 --- a/persistence/src/vespa/persistence/spi/test.cpp +++ b/persistence/src/vespa/persistence/spi/test.cpp @@ -21,15 +21,13 @@ std::unique_ptr<DocEntry> cloneDocEntry(const DocEntry & e) { std::unique_ptr<DocEntry> ret; if (e.getDocument()) { - ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags(), + ret = DocEntry::create(e.getTimestamp(), e.getFlags(), std::make_unique<Document>(*e.getDocument()), - e.getPersistedDocumentSize()); + e.getDocumentSize()); } else if (e.getDocumentId()) { - ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags(), *e.getDocumentId()); - ret->setPersistedDocumentSize(e.getPersistedDocumentSize()); + ret = DocEntry::create(e.getTimestamp(), e.getFlags(), *e.getDocumentId()); } else { - ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags()); - ret->setPersistedDocumentSize(e.getPersistedDocumentSize()); + ret = DocEntry::create(e.getTimestamp(), e.getFlags()); } return ret; } @@ -38,7 +36,7 @@ bool equal(const DocEntry & a, const DocEntry & b) { if (a.getTimestamp() != b.getTimestamp()) return false; if (a.getFlags() != b.getFlags()) return false; - if (a.getPersistedDocumentSize() != b.getPersistedDocumentSize()) return false; + if (a.getDocumentSize() != b.getDocumentSize()) return false; if (a.getDocument()) { if (!b.getDocument()) return false; 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 b93553c3aa3..209523cd0aa 100644 --- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp +++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp @@ -275,18 +275,14 @@ 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() + getSize(); + return tmp.size(); } size_t getSize(const document::DocumentId &id) { - return id.getSerializedSize() + getSize(); + return id.getSerializedSize(); } IDocumentRetriever::SP nil() { return std::make_unique<UnitDR>(); } @@ -390,26 +386,26 @@ void checkDoc(const IDocumentRetriever &dr, const std::string &id, void checkEntry(const IterateResult &res, size_t idx, const Timestamp ×tamp, int flags) { ASSERT_LESS(idx, res.getEntries().size()); - DocEntry expect(timestamp, flags); - EXPECT_TRUE(equal(expect, *res.getEntries()[idx])); - EXPECT_EQUAL(getSize(), res.getEntries()[idx]->getSize()); + auto expect = DocEntry::create(timestamp, flags); + EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); + EXPECT_EQUAL(0u, res.getEntries()[idx]->getDocumentSize()); } void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, const Timestamp ×tamp) { ASSERT_LESS(idx, res.getEntries().size()); - DocEntry expect(timestamp, storage::spi::REMOVE_ENTRY, id); - EXPECT_TRUE(equal(expect, *res.getEntries()[idx])); - EXPECT_EQUAL(getSize(id), res.getEntries()[idx]->getSize()); + auto expect = DocEntry::create(timestamp, storage::spi::REMOVE_ENTRY, id); + EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); + EXPECT_EQUAL(getSize(id), res.getEntries()[idx]->getDocumentSize()); EXPECT_GREATER(getSize(id), 0u); } void checkEntry(const IterateResult &res, size_t idx, const Document &doc, const Timestamp ×tamp) { ASSERT_LESS(idx, res.getEntries().size()); - DocEntry expect(timestamp, storage::spi::NONE, Document::UP(doc.clone())); - EXPECT_TRUE(equal(expect, *res.getEntries()[idx])); - EXPECT_EQUAL(getSize(doc), res.getEntries()[idx]->getSize()); + auto expect = DocEntry::create(timestamp, storage::spi::NONE, Document::UP(doc.clone())); + EXPECT_TRUE(equal(*expect, *res.getEntries()[idx])); + EXPECT_EQUAL(getSize(doc), res.getEntries()[idx]->getDocumentSize()); EXPECT_GREATER(getSize(doc), 0u); } @@ -630,8 +626,8 @@ TEST("require that maxBytes splits iteration results") { 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(getSize(Document(*DataType::DOCUMENT, DocumentId("id:ns:document::1"))) + - getSize(DocumentId("id:ns:document::2"))); + IterateResult res1 = itr.iterate(getSize(Document(*DataType::DOCUMENT, DocumentId("id:ns:document::1"))) + sizeof(DocEntry) + 8 + + getSize(DocumentId("id:ns:document::2")) + sizeof(DocEntry) + 8); EXPECT_TRUE(!res1.isCompleted()); EXPECT_EQUAL(2u, res1.getEntries().size()); TEST_DO(checkEntry(res1, 0, Document(*DataType::DOCUMENT, DocumentId("id:ns:document::1")), Timestamp(2))); @@ -651,7 +647,7 @@ 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(getSize() + getSize()); + IterateResult res1 = itr.iterate(2 * sizeof(DocEntry)); EXPECT_TRUE(!res1.isCompleted()); EXPECT_EQUAL(2u, res1.getEntries().size()); TEST_DO(checkEntry(res1, 0, Timestamp(2), storage::spi::NONE)); diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp index f7b38fa6560..39c93a0dd1a 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp @@ -33,10 +33,10 @@ std::unique_ptr<DocEntry> createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ssize_t defaultSerializedSize) { if (doc) { if (removed) { - return std::make_unique<DocEntry>(timestamp, storage::spi::REMOVE_ENTRY, doc->getId()); + return DocEntry::create(timestamp, storage::spi::REMOVE_ENTRY, doc->getId()); } else { ssize_t serializedSize = defaultSerializedSize >= 0 ? defaultSerializedSize : doc->serialize().size(); - return std::make_unique<DocEntry>(timestamp, storage::spi::NONE, std::move(doc), serializedSize); + return DocEntry::create(timestamp, storage::spi::NONE, std::move(doc), serializedSize); } } else { return createDocEntry(timestamp, removed); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 1b5f077aaf0..82ccdb60736 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -382,11 +382,8 @@ 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().emplace_back( - std::make_unique<spi::DocEntry>( - spi::Timestamp(1000 + i), - spi::NONE, - document::Document::UP(_documents[i]->clone()))); + reply->getEntries().push_back(spi::DocEntry::create(spi::Timestamp(1000 + i), spi::NONE, + document::Document::UP(_documents[i]->clone()))); } if (documentCount == _documents.size() || overrideCompleted) { reply->setCompleted(); diff --git a/storage/src/vespa/storage/persistence/processallhandler.cpp b/storage/src/vespa/storage/persistence/processallhandler.cpp index 3da6372cda8..c23d246d463 100644 --- a/storage/src/vespa/storage/persistence/processallhandler.cpp +++ b/storage/src/vespa/storage/persistence/processallhandler.cpp @@ -32,7 +32,7 @@ public: if (e.getDocument() != nullptr) { ost << "Doc(" << e.getDocument()->getId() << ")" << ", " << e.getDocument()->getId().getGlobalId().toString() - << ", size: " << e.getPersistedDocumentSize(); + << ", size: " << e.getDocumentSize(); } else if (e.getDocumentId() != nullptr) { ost << *e.getDocumentId() << ", " << e.getDocumentId()->getGlobalId().toString(); diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 7da397b670a..ac2c918910c 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -815,12 +815,11 @@ Visitor::onGetIterReply(const std::shared_ptr<GetIterReply>& reply, uint64_t size = 0; for (const auto& entry : reply->getEntries()) { - size += entry->getPersistedDocumentSize(); + size += entry->getDocumentSize(); } _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/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp b/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp index 104af486c90..fcbff9a0dbb 100644 --- a/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp +++ b/streamingvisitors/src/tests/searchvisitor/searchvisitor_test.cpp @@ -62,7 +62,7 @@ createDocuments(const vespalib::string & dir) std::vector<spi::DocEntry::UP> documents; spi::Timestamp ts; document::Document::UP doc(new document::Document()); - spi::DocEntry::UP e(new spi::DocEntry(ts, 0, std::move(doc))); + auto e = spi::DocEntry::create(ts, 0, std::move(doc)); documents.push_back(std::move(e)); return documents; } |