diff options
author | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2020-05-25 11:56:09 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2020-05-26 13:55:53 +0000 |
commit | 2c8db3de8a721af856fee825af5a48d10313ff1e (patch) | |
tree | c448617bdfdd577ab1aad8d73ac7cf369a97dcb4 /persistence | |
parent | 1026f130814d5063fce95092a860e01e10f4ecca (diff) |
Propagate provider tombstone metadata to internal SPI GetResult
Diffstat (limited to 'persistence')
4 files changed, 59 insertions, 16 deletions
diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index b45a4131e7c..036a2b87692 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -625,6 +625,7 @@ TEST_F(ConformanceTest, testPutNewDocumentVersion) EXPECT_EQ(Result::ErrorType::NONE, gr.getErrorCode()); EXPECT_EQ(Timestamp(4), gr.getTimestamp()); + EXPECT_FALSE(gr.is_tombstone()); if (!((*doc2)==gr.getDocument())) { std::cerr << "Document returned is not the expected one: \n" @@ -676,6 +677,7 @@ TEST_F(ConformanceTest, testPutOlderDocumentVersion) EXPECT_EQ(Result::ErrorType::NONE, gr.getErrorCode()); EXPECT_EQ(Timestamp(5), gr.getTimestamp()); EXPECT_EQ(*doc1, gr.getDocument()); + EXPECT_FALSE(gr.is_tombstone()); } TEST_F(ConformanceTest, testPutDuplicate) @@ -798,8 +800,9 @@ TEST_F(ConformanceTest, testRemove) context); EXPECT_EQ(Result::ErrorType::NONE, getResult.getErrorCode()); - EXPECT_EQ(Timestamp(0), getResult.getTimestamp()); - EXPECT_TRUE(!getResult.hasDocument()); + EXPECT_EQ(Timestamp(9), getResult.getTimestamp()); + EXPECT_TRUE(getResult.is_tombstone()); + EXPECT_FALSE(getResult.hasDocument()); } TEST_F(ConformanceTest, testRemoveMerge) @@ -939,6 +942,7 @@ TEST_F(ConformanceTest, testUpdate) EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(4), result.getTimestamp()); + EXPECT_FALSE(result.is_tombstone()); EXPECT_EQ(document::IntFieldValue(42), static_cast<document::IntFieldValue&>( *result.getDocument().getValue("headerval"))); @@ -953,8 +957,9 @@ TEST_F(ConformanceTest, testUpdate) context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); - EXPECT_EQ(Timestamp(0), result.getTimestamp()); - EXPECT_TRUE(!result.hasDocument()); + EXPECT_EQ(Timestamp(5), result.getTimestamp()); + EXPECT_FALSE(result.hasDocument()); + EXPECT_TRUE(result.is_tombstone()); } { @@ -968,8 +973,9 @@ TEST_F(ConformanceTest, testUpdate) { GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); - EXPECT_EQ(Timestamp(0), result.getTimestamp()); - EXPECT_TRUE(!result.hasDocument()); + EXPECT_EQ(Timestamp(5), result.getTimestamp()); + EXPECT_FALSE(result.hasDocument()); + EXPECT_TRUE(result.is_tombstone()); } update->setCreateIfNonExistent(true); @@ -985,6 +991,7 @@ TEST_F(ConformanceTest, testUpdate) GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(7), result.getTimestamp()); + EXPECT_FALSE(result.is_tombstone()); EXPECT_EQ(document::IntFieldValue(42), reinterpret_cast<document::IntFieldValue&>( *result.getDocument().getValue("headerval"))); @@ -1008,6 +1015,7 @@ TEST_F(ConformanceTest, testGet) EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(0), result.getTimestamp()); + EXPECT_FALSE(result.is_tombstone()); } spi->put(bucket, Timestamp(3), doc1, context); @@ -1017,6 +1025,7 @@ TEST_F(ConformanceTest, testGet) doc1->getId(), context); EXPECT_EQ(*doc1, result.getDocument()); EXPECT_EQ(Timestamp(3), result.getTimestamp()); + EXPECT_FALSE(result.is_tombstone()); } spi->remove(bucket, Timestamp(4), doc1->getId(), context); @@ -1026,7 +1035,8 @@ TEST_F(ConformanceTest, testGet) doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); - EXPECT_EQ(Timestamp(0), result.getTimestamp()); + EXPECT_EQ(Timestamp(4), result.getTimestamp()); + EXPECT_TRUE(result.is_tombstone()); } } diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index 5720a0ba662..92fea5ff6e4 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -533,7 +533,10 @@ DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const if (!bc.get()) { } else { DocEntry::SP entry((*bc)->getEntry(did)); - if (entry.get() == 0 || entry->isRemove()) { + if (!entry) { + return GetResult(); + } else if (entry->isRemove()) { + return GetResult::make_for_tombstone(entry->getTimestamp()); } else { Document::UP doc(entry->getDocument()->clone()); if (fieldSet.getType() != document::FieldSet::ALL) { diff --git a/persistence/src/vespa/persistence/spi/result.cpp b/persistence/src/vespa/persistence/spi/result.cpp index f5131f08b1f..8de6245a8d2 100644 --- a/persistence/src/vespa/persistence/spi/result.cpp +++ b/persistence/src/vespa/persistence/spi/result.cpp @@ -30,8 +30,17 @@ std::ostream & operator << (std::ostream & os, const Result::ErrorType &errorCod GetResult::GetResult(Document::UP doc, Timestamp timestamp) : Result(), _timestamp(timestamp), - _doc(std::move(doc)) -{ } + _doc(std::move(doc)), + _is_tombstone(false) +{ +} + +GetResult::GetResult(Timestamp removed_at_ts) + : _timestamp(removed_at_ts), + _doc(), + _is_tombstone(true) +{ +} GetResult::~GetResult() = default; BucketIdListResult::~BucketIdListResult() = default; diff --git a/persistence/src/vespa/persistence/spi/result.h b/persistence/src/vespa/persistence/spi/result.h index fe8e74706bb..714d71e37ee 100644 --- a/persistence/src/vespa/persistence/spi/result.h +++ b/persistence/src/vespa/persistence/spi/result.h @@ -156,13 +156,20 @@ public: */ GetResult(ErrorType error, const vespalib::string& errorMessage) : Result(error, errorMessage), - _timestamp(0) { } + _timestamp(0), + _is_tombstone(false) + { + } /** * Constructor to use when we didn't find the document in question. */ GetResult() - : _timestamp(0) { } + : _timestamp(0), + _doc(), + _is_tombstone(false) + { + } /** * Constructor to use when we found the document asked for. @@ -172,12 +179,22 @@ public: */ GetResult(DocumentUP doc, Timestamp timestamp); - ~GetResult(); + static GetResult make_for_tombstone(Timestamp removed_at_ts) { + return GetResult(removed_at_ts); + } + + ~GetResult() override; - Timestamp getTimestamp() const { return _timestamp; } + [[nodiscard]] Timestamp getTimestamp() const { + return _timestamp; + } - bool hasDocument() const { - return _doc.get() != NULL; + [[nodiscard]] bool hasDocument() const { + return (_doc.get() != nullptr); + } + + [[nodiscard]] bool is_tombstone() const noexcept { + return _is_tombstone; } const Document& getDocument() const { @@ -193,8 +210,12 @@ public: } private: + // Explicitly creates a tombstone (remove entry) GetResult with no document. + explicit GetResult(Timestamp removed_at_ts); + Timestamp _timestamp; DocumentSP _doc; + bool _is_tombstone; }; class BucketIdListResult : public Result { |