diff options
18 files changed, 92 insertions, 35 deletions
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<PersistenceProvider::TimeStampAndDocumentId> ids; + std::vector<spi::IdAndTimestamp> 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<TimeStampAndDocumentId> ids, OperationComplete::UP onComplete) +DummyPersistence::removeAsync(const Bucket& b, std::vector<spi::IdAndTimestamp> 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<TimeStampAndDocumentId> ids, OperationComplete::UP) override; + void removeAsync(const Bucket& b, std::vector<spi::IdAndTimestamp> 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<TimeStampAndDocumentId> ids; - ids.emplace_back(timestamp, id); + std::vector<IdAndTimestamp> 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 <vespa/document/base/documentid.h> + +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<CatchResult>(); auto future = catcher->future_result(); - std::vector<TimeStampAndDocumentId> ids; - ids.emplace_back(timestamp, docId); + std::vector<IdAndTimestamp> ids; + ids.emplace_back(docId, timestamp); removeAsync(bucket, std::move(ids), std::move(catcher)); return dynamic_cast<const RemoveResult &>(*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<TimeStampAndDocumentId> ids, OperationComplete::UP) = 0; + virtual void removeAsync(const Bucket&, std::vector<IdAndTimestamp> ids, OperationComplete::UP) = 0; /** * @see remove() diff --git a/searchcore/src/vespa/searchcore/bmcluster/spi_bm_feed_handler.cpp b/searchcore/src/vespa/searchcore/bmcluster/spi_bm_feed_handler.cpp index 69013e8d7c5..dcdba3b0715 100644 --- a/searchcore/src/vespa/searchcore/bmcluster/spi_bm_feed_handler.cpp +++ b/searchcore/src/vespa/searchcore/bmcluster/spi_bm_feed_handler.cpp @@ -134,8 +134,8 @@ SpiBmFeedHandler::remove(const document::Bucket& bucket, const DocumentId& docum auto provider = get_provider(bucket); if (provider) { Bucket spi_bucket(bucket); - std::vector<storage::spi::PersistenceProvider::TimeStampAndDocumentId> ids; - ids.emplace_back(Timestamp(timestamp), document_id); + std::vector<storage::spi::IdAndTimestamp> ids; + ids.emplace_back(document_id, Timestamp(timestamp)); provider->removeAsync(spi_bucket, std::move(ids), std::make_unique<MyOperationComplete>(provider, _errors, spi_bucket, tracker)); } else { ++_errors; diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp index 5a0bcb1cd41..81a7244ba1d 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp @@ -370,21 +370,21 @@ PersistenceEngine::putAsync(const Bucket &bucket, Timestamp ts, storage::spi::Do } void -PersistenceEngine::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentId> ids, OperationComplete::UP onComplete) +PersistenceEngine::removeAsync(const Bucket& b, std::vector<storage::spi::IdAndTimestamp> ids, OperationComplete::UP onComplete) { if (ids.size() == 1) { - removeAsyncSingle(b, ids[0].first, ids[0].second, std::move(onComplete)); + removeAsyncSingle(b, ids[0].timestamp, ids[0].id, std::move(onComplete)); } else { removeAsyncMulti(b, std::move(ids), std::move(onComplete)); } } void -PersistenceEngine::removeAsyncMulti(const Bucket& b, std::vector<TimeStampAndDocumentId> ids, OperationComplete::UP onComplete) { +PersistenceEngine::removeAsyncMulti(const Bucket& b, std::vector<storage::spi::IdAndTimestamp> ids, OperationComplete::UP onComplete) { ReadGuard rguard(_rwMutex); //TODO Group per document type/handler and handle in one go. - for (const TimeStampAndDocumentId & stampedId : ids) { - const document::DocumentId & id = stampedId.second; + for (const auto & stampedId : ids) { + const document::DocumentId & id = stampedId.id; if (!id.hasDocType()) { return onComplete->onComplete( std::make_unique<RemoveResult>(Result::ErrorType::PERMANENT_ERROR, @@ -399,11 +399,11 @@ PersistenceEngine::removeAsyncMulti(const Bucket& b, std::vector<TimeStampAndDoc } } auto transportContext = std::make_shared<AsyncRemoveTransportContext>(ids.size(), std::move(onComplete)); - for (const TimeStampAndDocumentId & stampedId : ids) { - const document::DocumentId & id = stampedId.second; + for (const auto & stampedId : ids) { + const document::DocumentId & id = stampedId.id; DocTypeName docType(id.getDocType()); IPersistenceHandler *handler = getHandler(rguard, b.getBucketSpace(), docType); - handler->handleRemove(feedtoken::make(transportContext), b, stampedId.first, id); + handler->handleRemove(feedtoken::make(transportContext), b, stampedId.timestamp, id); } } diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h index a8886e19def..c16cc6e6a83 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h @@ -89,7 +89,7 @@ private: ClusterState::SP savedClusterState(BucketSpace bucketSpace) const; std::shared_ptr<BucketExecutor> get_bucket_executor() noexcept { return _bucket_executor.lock(); } void removeAsyncSingle(const Bucket&, Timestamp, const document::DocumentId &id, OperationComplete::UP); - void removeAsyncMulti(const Bucket&, std::vector<TimeStampAndDocumentId> ids, OperationComplete::UP); + void removeAsyncMulti(const Bucket&, std::vector<storage::spi::IdAndTimestamp> ids, OperationComplete::UP); public: typedef std::unique_ptr<PersistenceEngine> UP; @@ -107,7 +107,7 @@ public: void setActiveStateAsync(const Bucket&, BucketInfo::ActiveState, OperationComplete::UP) override; BucketInfoResult getBucketInfo(const Bucket&) const override; void putAsync(const Bucket &, Timestamp, storage::spi::DocumentSP, OperationComplete::UP) override; - void removeAsync(const Bucket&, std::vector<TimeStampAndDocumentId> ids, OperationComplete::UP) override; + void removeAsync(const Bucket&, std::vector<storage::spi::IdAndTimestamp> ids, OperationComplete::UP) override; void updateAsync(const Bucket&, Timestamp, storage::spi::DocumentUpdateSP, OperationComplete::UP) override; GetResult get(const Bucket&, const document::FieldSet&, const document::DocumentId&, Context&) const override; CreateIteratorResult diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp index 7e0b96b1d82..fcb56c4a553 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp @@ -106,11 +106,11 @@ PersistenceProviderWrapper::putAsync(const spi::Bucket& bucket, spi::Timestamp t } void -PersistenceProviderWrapper::removeAsync(const spi::Bucket& bucket, std::vector<TimeStampAndDocumentId> ids, +PersistenceProviderWrapper::removeAsync(const spi::Bucket& bucket, std::vector<spi::IdAndTimestamp> ids, spi::OperationComplete::UP onComplete) { - for (const TimeStampAndDocumentId & stampedId : ids) { - LOG_SPI("remove(" << bucket << ", " << stampedId.first << ", " << stampedId.second << ")"); + for (const auto & stampedId : ids) { + LOG_SPI("remove(" << bucket << ", " << stampedId.timestamp << ", " << stampedId.id << ")"); } CHECK_ERROR_ASYNC(spi::RemoveResult, FAIL_REMOVE, onComplete); _spi.removeAsync(bucket, std::move(ids), std::move(onComplete)); diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.h b/storage/src/tests/persistence/common/persistenceproviderwrapper.h index 3c93bc91d85..ec7ca70d7c7 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.h +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.h @@ -106,7 +106,7 @@ public: spi::BucketIdListResult listBuckets(BucketSpace bucketSpace) const override; spi::BucketInfoResult getBucketInfo(const spi::Bucket&) const override; void putAsync(const spi::Bucket&, spi::Timestamp, spi::DocumentSP, spi::OperationComplete::UP) override; - void removeAsync(const spi::Bucket&, std::vector<TimeStampAndDocumentId> ids, spi::OperationComplete::UP) override; + void removeAsync(const spi::Bucket&, std::vector<spi::IdAndTimestamp> ids, spi::OperationComplete::UP) override; void removeIfFoundAsync(const spi::Bucket&, spi::Timestamp, const spi::DocumentId&, spi::OperationComplete::UP) override; void updateAsync(const spi::Bucket&, spi::Timestamp, spi::DocumentUpdateSP, spi::OperationComplete::UP) override; spi::GetResult get(const spi::Bucket&, const document::FieldSet&, const spi::DocumentId&, spi::Context&) const override; diff --git a/storage/src/vespa/storage/persistence/asynchandler.cpp b/storage/src/vespa/storage/persistence/asynchandler.cpp index f5d29fb32a7..d5bf733a30c 100644 --- a/storage/src/vespa/storage/persistence/asynchandler.cpp +++ b/storage/src/vespa/storage/persistence/asynchandler.cpp @@ -114,13 +114,13 @@ bucketStatesAreSemanticallyEqual(const api::BucketInfo& a, const api::BucketInfo class UnrevertableRemoveEntryProcessor : public BucketProcessor::EntryProcessor { public: - using DocumentIdsAndTimeStamps = std::vector<std::pair<spi::Timestamp, spi::DocumentId>>; + using DocumentIdsAndTimeStamps = std::vector<spi::IdAndTimestamp>; UnrevertableRemoveEntryProcessor(DocumentIdsAndTimeStamps & to_remove) : _to_remove(to_remove) {} void process(spi::DocEntry& entry) override { - _to_remove.emplace_back(entry.getTimestamp(), *entry.getDocumentId()); + _to_remove.emplace_back(*entry.getDocumentId(), entry.getTimestamp()); } private: DocumentIdsAndTimeStamps & _to_remove; diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 012d5c2619d..ae68a694c90 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -522,9 +522,9 @@ MergeHandler::applyDiffEntry(std::shared_ptr<ApplyBucketDiffState> async_results _clock, _env._metrics.merge_handler_metrics.put_latency); _spi.putAsync(bucket, timestamp, std::move(doc), std::move(complete)); } else { - std::vector<spi::PersistenceProvider::TimeStampAndDocumentId> ids; - ids.emplace_back(timestamp, e._docName); - auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), ids[0].second, + std::vector<spi::IdAndTimestamp> ids; + ids.emplace_back(document::DocumentId(e._docName), timestamp); + auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), ids[0].id, std::move(throttle_token), "remove", _clock, _env._metrics.merge_handler_metrics.remove_latency); _spi.removeAsync(bucket, std::move(ids), std::move(complete)); diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp index 1be9679c641..9e55c0f9088 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp @@ -152,7 +152,7 @@ ProviderErrorWrapper::putAsync(const spi::Bucket &bucket, spi::Timestamp ts, spi } void -ProviderErrorWrapper::removeAsync(const spi::Bucket &bucket, std::vector<TimeStampAndDocumentId> ids, +ProviderErrorWrapper::removeAsync(const spi::Bucket &bucket, std::vector<spi::IdAndTimestamp> ids, spi::OperationComplete::UP onComplete) { onComplete->addResultHandler(this); diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.h b/storage/src/vespa/storage/persistence/provider_error_wrapper.h index 7bd406a8758..82447fe4549 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.h +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.h @@ -58,7 +58,7 @@ public: void register_error_listener(std::shared_ptr<ProviderErrorListener> listener); void putAsync(const spi::Bucket &, spi::Timestamp, spi::DocumentSP, spi::OperationComplete::UP) override; - void removeAsync(const spi::Bucket&, std::vector<TimeStampAndDocumentId>, spi::OperationComplete::UP) override; + void removeAsync(const spi::Bucket&, std::vector<spi::IdAndTimestamp>, spi::OperationComplete::UP) override; void removeIfFoundAsync(const spi::Bucket&, spi::Timestamp, const document::DocumentId&, spi::OperationComplete::UP) override; void updateAsync(const spi::Bucket &, spi::Timestamp, spi::DocumentUpdateSP, spi::OperationComplete::UP) override; void setActiveStateAsync(const spi::Bucket& b, spi::BucketInfo::ActiveState newState, spi::OperationComplete::UP onComplete) override; |