From 5455cacd18e7f904c4ae783022b3956532e5d9c4 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Fri, 10 Dec 2021 20:32:31 -0800 Subject: [PATCH] Fix link error reported in issue 9272 (#9278) Summary: As title, Closes https://github.com/facebook/rocksdb/issues/9272 Since TimestampAssigner-related classes needs to access `WriteBatch::ProtectionInfo` objects which is for internal use only, it's difficult to make `AssignTimestamp` methods a template and put them in the same public header, `include/rocksdb/write_batch.h`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9278 Test Plan: ``` make check # Also manually test following the repro-steps in issue 9272 ``` Reviewed By: ltamasi Differential Revision: D33012686 Pulled By: riversand963 fbshipit-source-id: 89f24a86a1170125bd0b94ef3b32e69aa08bd949 --- db/write_batch.cc | 14 +++++++++ db/write_batch_internal.h | 53 ++++++++++++----------------------- include/rocksdb/write_batch.h | 43 ++++++++++++++++++---------- 3 files changed, 60 insertions(+), 50 deletions(-) diff --git a/db/write_batch.cc b/db/write_batch.cc index c66bcb9ea..6ee264e18 100644 --- a/db/write_batch.cc +++ b/db/write_batch.cc @@ -1223,6 +1223,20 @@ Status WriteBatch::PopSavePoint() { return Status::OK(); } +Status WriteBatch::AssignTimestamp( + const Slice& ts, std::function checker) { + TimestampAssigner ts_assigner(prot_info_.get(), std::move(checker), ts); + return Iterate(&ts_assigner); +} + +Status WriteBatch::AssignTimestamps( + const std::vector& ts_list, + std::function checker) { + SimpleListTimestampAssigner ts_assigner(prot_info_.get(), std::move(checker), + ts_list); + return Iterate(&ts_assigner); +} + class MemTableInserter : public WriteBatch::Handler { SequenceNumber sequence_; diff --git a/db/write_batch_internal.h b/db/write_batch_internal.h index 3e221be32..8b52d93c0 100644 --- a/db/write_batch_internal.h +++ b/db/write_batch_internal.h @@ -265,11 +265,12 @@ class LocalSavePoint { #endif }; -template +template class TimestampAssignerBase : public WriteBatch::Handler { public: - explicit TimestampAssignerBase(WriteBatch::ProtectionInfo* prot_info, - Checker&& checker) + explicit TimestampAssignerBase( + WriteBatch::ProtectionInfo* prot_info, + std::function&& checker) : prot_info_(prot_info), checker_(std::move(checker)) {} ~TimestampAssignerBase() override {} @@ -360,27 +361,25 @@ class TimestampAssignerBase : public WriteBatch::Handler { TimestampAssignerBase& operator=(TimestampAssignerBase&&) = delete; WriteBatch::ProtectionInfo* const prot_info_ = nullptr; - const Checker checker_{}; + const std::function checker_{}; size_t idx_ = 0; }; -template class SimpleListTimestampAssigner - : public TimestampAssignerBase, - Checker> { + : public TimestampAssignerBase { public: - explicit SimpleListTimestampAssigner(WriteBatch::ProtectionInfo* prot_info, - Checker checker, - const std::vector& timestamps) - : TimestampAssignerBase, Checker>( - prot_info, std::move(checker)), + explicit SimpleListTimestampAssigner( + WriteBatch::ProtectionInfo* prot_info, + std::function&& checker, + const std::vector& timestamps) + : TimestampAssignerBase(prot_info, + std::move(checker)), timestamps_(timestamps) {} ~SimpleListTimestampAssigner() override {} private: - friend class TimestampAssignerBase, - Checker>; + friend class TimestampAssignerBase; Status AssignTimestampImpl(uint32_t cf, const Slice& key, size_t idx) { if (idx >= timestamps_.size()) { @@ -398,21 +397,19 @@ class SimpleListTimestampAssigner const std::vector& timestamps_; }; -template -class TimestampAssigner - : public TimestampAssignerBase, Checker> { +class TimestampAssigner : public TimestampAssignerBase { public: explicit TimestampAssigner(WriteBatch::ProtectionInfo* prot_info, - Checker checker, const Slice& ts) - : TimestampAssignerBase, Checker>( - prot_info, std::move(checker)), + std::function&& checker, + const Slice& ts) + : TimestampAssignerBase(prot_info, std::move(checker)), timestamp_(ts) { assert(!timestamp_.empty()); } ~TimestampAssigner() override {} private: - friend class TimestampAssignerBase, Checker>; + friend class TimestampAssignerBase; Status AssignTimestampImpl(uint32_t cf, const Slice& key, size_t /*idx*/) { if (timestamp_.empty()) { @@ -429,18 +426,4 @@ class TimestampAssigner const Slice timestamp_; }; -template -Status WriteBatch::AssignTimestamp(const Slice& ts, Checker checker) { - TimestampAssigner ts_assigner(prot_info_.get(), checker, ts); - return Iterate(&ts_assigner); -} - -template -Status WriteBatch::AssignTimestamps(const std::vector& ts_list, - Checker checker) { - SimpleListTimestampAssigner ts_assigner(prot_info_.get(), checker, - ts_list); - return Iterate(&ts_assigner); -} - } // namespace ROCKSDB_NAMESPACE diff --git a/include/rocksdb/write_batch.h b/include/rocksdb/write_batch.h index 888aaef63..94ab7e62c 100644 --- a/include/rocksdb/write_batch.h +++ b/include/rocksdb/write_batch.h @@ -342,25 +342,29 @@ class WriteBatch : public WriteBatchBase { // Returns true if MarkRollback will be called during Iterate bool HasRollback() const; - struct TimestampChecker final { - Status operator()(uint32_t /*cf*/, size_t& /*ts_sz*/) const { - return Status::OK(); - } - }; - // Experimental. // Assign timestamp to write batch. // This requires that all keys, if enable timestamp, (possibly from multiple // column families) in the write batch have timestamps of the same format. + // // checker: callable object to check the timestamp sizes of column families. + // + // in: cf, the column family id. + // in/out: ts_sz. Input as the expected timestamp size of the column + // family, output as the actual timestamp size of the column family. + // ret: OK if assignment succeeds. + // Status checker(uint32_t cf, size_t& ts_sz); + // // User can call checker(uint32_t cf, size_t& ts_sz) which does the // following: - // 1. find out the timestamp size of cf. + // 1. find out the timestamp size of the column family whose id equals `cf`. // 2. if cf's timestamp size is 0, then set ts_sz to 0 and return OK. // 3. otherwise, compare ts_sz with cf's timestamp size and return - // Status::InvalidArgument() if different. - template - Status AssignTimestamp(const Slice& ts, Checker checker = Checker()); + // Status::InvalidArgument() if different. + Status AssignTimestamp( + const Slice& ts, + std::function checker = + [](uint32_t /*cf*/, size_t& /*ts_sz*/) { return Status::OK(); }); // Experimental. // Assign timestamps to write batch. @@ -369,16 +373,25 @@ class WriteBatch : public WriteBatchBase { // families can enable timestamp, while others disable the feature. // If key does not have timestamp, then put an empty Slice in ts_list as // a placeholder. + // // checker: callable object specified by caller to check the timestamp sizes // of column families. + // + // in: cf, the column family id. + // in/out: ts_sz. Input as the expected timestamp size of the column + // family, output as the actual timestamp size of the column family. + // ret: OK if assignment succeeds. + // Status checker(uint32_t cf, size_t& ts_sz); + // // User can call checker(uint32_t cf, size_t& ts_sz) which does the // following: - // 1. find out the timestamp size of cf. + // 1. find out the timestamp size of the column family whose id equals `cf`. // 2. compare ts_sz with cf's timestamp size and return - // Status::InvalidArgument() if different. - template - Status AssignTimestamps(const std::vector& ts_list, - Checker checker = Checker()); + // Status::InvalidArgument() if different. + Status AssignTimestamps( + const std::vector& ts_list, + std::function checker = + [](uint32_t /*cf*/, size_t& /*ts_sz*/) { return Status::OK(); }); using WriteBatchBase::GetWriteBatch; WriteBatch* GetWriteBatch() override { return this; }