diff --git a/src/database/storage_gc_master.hpp b/src/database/storage_gc_master.hpp index 91fe3adf6..51fda3529 100644 --- a/src/database/storage_gc_master.hpp +++ b/src/database/storage_gc_master.hpp @@ -51,7 +51,7 @@ class StorageGcMaster : public StorageGc { // All workers reported back at least once if (min_safe > 0) { tx_engine_.GarbageCollectCommitLog(min_safe); - LOG(INFO) << "Clearing master commit log with tx:" << min_safe; + LOG(INFO) << "Clearing master commit log with tx: " << min_safe; } } } diff --git a/src/storage/locking/record_lock.cpp b/src/storage/locking/record_lock.cpp index 3bee379db..c2ea27ac9 100644 --- a/src/storage/locking/record_lock.cpp +++ b/src/storage/locking/record_lock.cpp @@ -45,9 +45,13 @@ std::experimental::optional FindOldestTxInLockCycle( } // namespace +bool RecordLock::TryLock(tx::transaction_id_t tx_id) { + tx::transaction_id_t unlocked{0}; + return owner_.compare_exchange_strong(unlocked, tx_id); +} + LockStatus RecordLock::Lock(const tx::Transaction &tx, tx::Engine &engine) { - if (lock_.try_lock()) { - owner_ = tx.id_; + if (TryLock(tx.id_)) { return LockStatus::Acquired; } @@ -59,10 +63,8 @@ LockStatus RecordLock::Lock(const tx::Transaction &tx, tx::Engine &engine) { // might be active for a dead transaction. By asking the transaction engine // for transaction info, we'll make the worker refresh it's knowledge about // live transactions and release obsolete locks. - auto info = engine.Info(owner); - if (!info.is_active()) { - if (lock_.try_lock()) { - owner_ = tx.id_; + if (owner == 0 || !engine.Info(owner).is_active()) { + if (TryLock(tx.id_)) { return LockStatus::Acquired; } } @@ -103,8 +105,7 @@ LockStatus RecordLock::Lock(const tx::Transaction &tx, tx::Engine &engine) { throw LockTimeoutException( "Transaction was aborted since it was oldest in a lock cycle"); } - if (lock_.try_lock()) { - owner_ = tx.id_; + if (TryLock(tx.id_)) { return LockStatus::Acquired; } if (owner != owner_) { @@ -125,6 +126,6 @@ LockStatus RecordLock::Lock(const tx::Transaction &tx, tx::Engine &engine) { "Transaction locked for more than {} seconds", kTimeout.count())); } -void RecordLock::Unlock() { lock_.unlock(); } +void RecordLock::Unlock() { owner_ = 0; } constexpr std::chrono::duration RecordLock::kTimeout; diff --git a/src/storage/locking/record_lock.hpp b/src/storage/locking/record_lock.hpp index 581aae089..12046cfd3 100644 --- a/src/storage/locking/record_lock.hpp +++ b/src/storage/locking/record_lock.hpp @@ -12,7 +12,7 @@ namespace tx { class Engine; class Transaction; -}; +}; // namespace tx class RecordLock { public: @@ -21,14 +21,11 @@ class RecordLock { void Unlock(); private: + bool TryLock(tx::transaction_id_t tx_id); + // Arbitrary choosen constant, postgresql uses 1 second so do we. constexpr static std::chrono::duration kTimeout{ std::chrono::seconds(1)}; - // TODO: Because of the current architecture it is somewhat OK to use SpinLock - // here. Once we reimplement worker architecture to execute some other - // transaction in this thread while other is waiting for a lock this will had - // to change to something else. - SpinLock lock_; std::atomic owner_{0}; };