Skip to content

Commit

Permalink
Allow setting the on_lock_lost after construction
Browse files Browse the repository at this point in the history
This will be useful if we want to construct a ReliableStorageLockGuard
but decide later what to do in case a lock is lost.

Also changes ReliableStorageLockGuard to hold a copy of the lock instead
of just a reference. The lock is a cheap object to copy, so it makes
easier to avoid errors where lock is destructed before the guard for it.
  • Loading branch information
IvoDD committed Dec 9, 2024
1 parent 39dcb6e commit e870a13
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
14 changes: 12 additions & 2 deletions cpp/arcticdb/util/reliable_storage_lock-inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ std::optional<AcquiredLockId> ReliableStorageLock<ClockType>::try_take_next_id(c
return lock_id;
}

inline ReliableStorageLockGuard::ReliableStorageLockGuard(const ReliableStorageLock<> &lock, AcquiredLockId acquired_lock, folly::Func&& on_lost_lock) :
inline ReliableStorageLockGuard::ReliableStorageLockGuard(const ReliableStorageLock<> &lock, AcquiredLockId acquired_lock, std::optional<folly::Func>&& on_lost_lock) :
lock_(lock), acquired_lock_(std::nullopt), on_lost_lock_(std::move(on_lost_lock)) {
acquired_lock_ = acquired_lock;
// We heartbeat 5 times per lock timeout to extend the lock.
Expand All @@ -211,7 +211,9 @@ inline ReliableStorageLockGuard::ReliableStorageLockGuard(const ReliableStorageL
inline void ReliableStorageLockGuard::cleanup_on_lost_lock() {
// We do not use shutdown because we don't want to run it from within a FunctionScheduler thread to avoid a deadlock
extend_lock_heartbeat_.cancelAllFunctions();
on_lost_lock_();
if (on_lost_lock_.has_value()){
on_lost_lock_.value()();
}
}

inline ReliableStorageLockGuard::~ReliableStorageLockGuard() {
Expand All @@ -221,6 +223,14 @@ inline ReliableStorageLockGuard::~ReliableStorageLockGuard() {
}
}

inline void ReliableStorageLockGuard::set_on_lost_lock(folly::Func &&on_lost_lock) {
on_lost_lock_ = std::make_optional<folly::Func>(std::move(on_lost_lock));
if (!acquired_lock_.has_value()) {
// Lock was lost before we set on_lost_lock. Running callback immediately.
on_lost_lock_.value()();
}
}

inline void ReliableStorageLockManager::take_lock_guard(const ReliableStorageLock<> &lock) {
auto acquired = lock.retry_until_take_lock();
guard = std::make_shared<ReliableStorageLockGuard>(lock, acquired, [](){
Expand Down
10 changes: 7 additions & 3 deletions cpp/arcticdb/util/reliable_storage_lock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ template <class ClockType = util::SysClock>
class ReliableStorageLock {
public:
ReliableStorageLock(const std::string& base_name, const std::shared_ptr<Store> store, timestamp timeout);
ReliableStorageLock(const ReliableStorageLock<ClockType>& other) = default;

AcquiredLockId retry_until_take_lock() const;
std::optional<AcquiredLockId> try_take_lock() const;
Expand All @@ -55,14 +56,17 @@ class ReliableStorageLock {
// via the on_lock_lost.
class ReliableStorageLockGuard {
public:
ReliableStorageLockGuard(const ReliableStorageLock<> &lock, AcquiredLockId acquired_lock, folly::Func&& on_lost_lock);
ReliableStorageLockGuard(const ReliableStorageLock<>& lock, AcquiredLockId acquired_lock, std::optional<folly::Func>&& on_lost_lock);

~ReliableStorageLockGuard();

// Will immediately trigger [on_lost_lock] if lock is already lost.
void set_on_lost_lock(folly::Func&& on_lost_lock);
private:
void cleanup_on_lost_lock();
const ReliableStorageLock<> &lock_;
const ReliableStorageLock<> lock_;
std::optional<AcquiredLockId> acquired_lock_;
folly::Func on_lost_lock_;
std::optional<folly::Func> on_lost_lock_;
folly::FunctionScheduler extend_lock_heartbeat_;
};

Expand Down

0 comments on commit e870a13

Please sign in to comment.