From db527eaeb7e70b882bcf14f0f6b4c6f8bd0196b9 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Thu, 7 Jul 2022 12:15:37 +0000 Subject: Add wrapper for tuple and update APIs to use this Feels more intuitive to have a tuple that implies "document foo at timestamp bar" rather than the current inverse of "timestamp bar with document foo". --- .../conformancetest/conformancetest.cpp | 4 +-- .../persistence/dummyimpl/dummypersistence.cpp | 8 ++--- .../vespa/persistence/dummyimpl/dummypersistence.h | 2 +- .../src/vespa/persistence/spi/CMakeLists.txt | 1 + .../spi/abstractpersistenceprovider.cpp | 4 +-- .../src/vespa/persistence/spi/id_and_timestamp.cpp | 17 ++++++++++ .../src/vespa/persistence/spi/id_and_timestamp.h | 38 ++++++++++++++++++++++ .../vespa/persistence/spi/persistenceprovider.cpp | 4 +-- .../vespa/persistence/spi/persistenceprovider.h | 3 +- 9 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 persistence/src/vespa/persistence/spi/id_and_timestamp.cpp create mode 100644 persistence/src/vespa/persistence/spi/id_and_timestamp.h (limited to 'persistence/src') diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index bf1c828e2e6..e03403f601d 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -765,11 +765,11 @@ TEST_F(ConformanceTest, testRemoveMulti) docs.push_back(testDocMan.createRandomDocumentAtLocation(0x01, i)); } - std::vector ids; + std::vector ids; for (size_t i(0); i < docs.size(); i++) { spi->put(bucket1, Timestamp(i), docs[i]); if (i & 0x1) { - ids.emplace_back(Timestamp(i), docs[i]->getId()); + ids.emplace_back(docs[i]->getId(), Timestamp(i)); } } diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index 81bfdf7f9a3..6eabadc2f86 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -476,16 +476,16 @@ DummyPersistence::updateAsync(const Bucket& bucket, Timestamp ts, DocumentUpdate } void -DummyPersistence::removeAsync(const Bucket& b, std::vector ids, OperationComplete::UP onComplete) +DummyPersistence::removeAsync(const Bucket& b, std::vector ids, OperationComplete::UP onComplete) { DUMMYPERSISTENCE_VERIFY_INITIALIZED; assert(b.getBucketSpace() == FixedBucketSpaces::default_space()); BucketContentGuard::UP bc(acquireBucketWithLock(b)); uint32_t numRemoves(0); - for (const TimeStampAndDocumentId & stampedId : ids) { - const DocumentId & id = stampedId.second; - Timestamp t = stampedId.first; + for (const spi::IdAndTimestamp & stampedId : ids) { + const DocumentId & id = stampedId.id; + Timestamp t = stampedId.timestamp; LOG(debug, "remove(%s, %" PRIu64 ", %s)", b.toString().c_str(), uint64_t(t), id.toString().c_str()); while (!bc) { diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h index e015185e5b0..56602b3ab00 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h @@ -160,7 +160,7 @@ public: BucketInfoResult getBucketInfo(const Bucket&) const override; GetResult get(const Bucket&, const document::FieldSet&, const DocumentId&, Context&) const override; void putAsync(const Bucket&, Timestamp, DocumentSP, OperationComplete::UP) override; - void removeAsync(const Bucket& b, std::vector ids, OperationComplete::UP) override; + void removeAsync(const Bucket& b, std::vector ids, OperationComplete::UP) override; void updateAsync(const Bucket&, Timestamp, DocumentUpdateSP, OperationComplete::UP) override; CreateIteratorResult diff --git a/persistence/src/vespa/persistence/spi/CMakeLists.txt b/persistence/src/vespa/persistence/spi/CMakeLists.txt index e4bae1c7551..bc94020ae95 100644 --- a/persistence/src/vespa/persistence/spi/CMakeLists.txt +++ b/persistence/src/vespa/persistence/spi/CMakeLists.txt @@ -10,6 +10,7 @@ vespa_add_library(persistence_spi OBJECT context.cpp docentry.cpp exceptions.cpp + id_and_timestamp.cpp persistenceprovider.cpp read_consistency.cpp resource_usage.cpp diff --git a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp index f301a9c5428..04d06235f59 100644 --- a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp +++ b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp @@ -10,8 +10,8 @@ void AbstractPersistenceProvider::removeIfFoundAsync(const Bucket& b, Timestamp timestamp, const DocumentId& id, OperationComplete::UP onComplete) { - std::vector ids; - ids.emplace_back(timestamp, id); + std::vector ids; + ids.emplace_back(id, timestamp); removeAsync(b, std::move(ids), std::move(onComplete)); } diff --git a/persistence/src/vespa/persistence/spi/id_and_timestamp.cpp b/persistence/src/vespa/persistence/spi/id_and_timestamp.cpp new file mode 100644 index 00000000000..fba45990744 --- /dev/null +++ b/persistence/src/vespa/persistence/spi/id_and_timestamp.cpp @@ -0,0 +1,17 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "id_and_timestamp.h" + +namespace storage::spi { + +IdAndTimestamp::IdAndTimestamp() : id(), timestamp(0) {} +IdAndTimestamp::IdAndTimestamp(document::DocumentId id_, Timestamp timestamp_) noexcept + : id(std::move(id_)), + timestamp(timestamp_) +{} + +IdAndTimestamp::IdAndTimestamp(const IdAndTimestamp&) = default; +IdAndTimestamp& IdAndTimestamp::operator=(const IdAndTimestamp&) = default; +IdAndTimestamp::IdAndTimestamp(IdAndTimestamp&&) noexcept = default; +IdAndTimestamp& IdAndTimestamp::operator=(IdAndTimestamp&&) noexcept = default; + +} diff --git a/persistence/src/vespa/persistence/spi/id_and_timestamp.h b/persistence/src/vespa/persistence/spi/id_and_timestamp.h new file mode 100644 index 00000000000..d8cdba3d063 --- /dev/null +++ b/persistence/src/vespa/persistence/spi/id_and_timestamp.h @@ -0,0 +1,38 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include "types.h" +#include + +namespace storage::spi { + +/** + * Convenience wrapper for referencing a document ID at a particular timestamp. + * + * Prefer this instead of a std::pair due to named fields and a pre-provided hash function. + */ +struct IdAndTimestamp { + document::DocumentId id; + Timestamp timestamp; + + IdAndTimestamp(); + IdAndTimestamp(document::DocumentId id_, Timestamp timestamp_) noexcept; + + IdAndTimestamp(const IdAndTimestamp&); + IdAndTimestamp& operator=(const IdAndTimestamp&); + IdAndTimestamp(IdAndTimestamp&&) noexcept; + IdAndTimestamp& operator=(IdAndTimestamp&&) noexcept; + + bool operator==(const IdAndTimestamp& rhs) const noexcept { + return ((id == rhs.id) && (timestamp == rhs.timestamp)); + } + + struct hash { + size_t operator()(const IdAndTimestamp& id_ts) const noexcept { + const size_t h = document::GlobalId::hash()(id_ts.id.getGlobalId()); + return h ^ (id_ts.timestamp + 0x9e3779b9U + (h << 6U) + (h >> 2U)); // Basically boost::hash_combine + } + }; +}; + +} diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp index 03cefb8df89..911b3753b1f 100644 --- a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp +++ b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp @@ -44,8 +44,8 @@ RemoveResult PersistenceProvider::remove(const Bucket& bucket, Timestamp timestamp, const DocumentId & docId) { auto catcher = std::make_unique(); auto future = catcher->future_result(); - std::vector ids; - ids.emplace_back(timestamp, docId); + std::vector ids; + ids.emplace_back(docId, timestamp); removeAsync(bucket, std::move(ids), std::move(catcher)); return dynamic_cast(*future.get()); } diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.h b/persistence/src/vespa/persistence/spi/persistenceprovider.h index a90d39e8334..d3e1465e528 100644 --- a/persistence/src/vespa/persistence/spi/persistenceprovider.h +++ b/persistence/src/vespa/persistence/spi/persistenceprovider.h @@ -4,6 +4,7 @@ #include "bucket.h" #include "bucketinfo.h" #include "context.h" +#include "id_and_timestamp.h" #include "result.h" #include "selection.h" #include "clusterstate.h" @@ -170,7 +171,7 @@ struct PersistenceProvider * @param timestamp The timestamp for the new bucket entry. * @param id The ID to remove */ - virtual void removeAsync(const Bucket&, std::vector ids, OperationComplete::UP) = 0; + virtual void removeAsync(const Bucket&, std::vector ids, OperationComplete::UP) = 0; /** * @see remove() -- cgit v1.2.3