Skip to content

Commit

Permalink
Implement non-exclusive locks
Browse files Browse the repository at this point in the history
Summary:
This is an implementation of non-exclusive locks for pessimistic transactions. It is relatively simple and does not prevent starvation (ie. it's possible that request for exclusive access will never be granted if there are always threads holding shared access). It is done by changing `KeyLockInfo` to hold an set a transaction ids, instead of just one, and adding a flag specifying whether this lock is currently held with exclusive access or not.

Some implementation notes:
- Some lock diagnostic functions had to be updated to return a set of transaction ids for a given lock, eg. `GetWaitingTxn` and `GetLockStatusData`.
- Deadlock detection is a bit more complicated since a transaction can now wait on multiple other transactions. A BFS is done in this case, and deadlock detection depth is now just a limit on the number of transactions we visit.
- Expirable transactions do not work efficiently with shared locks at the moment, but that's okay for now.
Closes facebook#1573

Differential Revision: D4239097

Pulled By: lth

fbshipit-source-id: da7c074
  • Loading branch information
lth authored and Facebook Github Bot committed Dec 6, 2016
1 parent 0b0f235 commit 2005c88
Show file tree
Hide file tree
Showing 14 changed files with 427 additions and 164 deletions.
11 changes: 6 additions & 5 deletions include/rocksdb/utilities/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,11 @@ class Transaction {
// or other errors if this key could not be read.
virtual Status GetForUpdate(const ReadOptions& options,
ColumnFamilyHandle* column_family,
const Slice& key, std::string* value) = 0;
const Slice& key, std::string* value,
bool exclusive = true) = 0;

virtual Status GetForUpdate(const ReadOptions& options, const Slice& key,
std::string* value) = 0;
std::string* value, bool exclusive = true) = 0;

virtual std::vector<Status> MultiGetForUpdate(
const ReadOptions& options,
Expand Down Expand Up @@ -401,10 +402,10 @@ class Transaction {

virtual bool IsDeadlockDetect() const { return false; }

virtual TransactionID GetWaitingTxn(uint32_t* column_family_id,
const std::string** key) const {
virtual std::vector<TransactionID> GetWaitingTxns(uint32_t* column_family_id,
std::string* key) const {
assert(false);
return 0;
return std::vector<TransactionID>();
}

enum TransactionState {
Expand Down
3 changes: 2 additions & 1 deletion include/rocksdb/utilities/transaction_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ struct TransactionOptions {

struct KeyLockInfo {
std::string key;
TransactionID id;
std::vector<TransactionID> ids;
bool exclusive;
};

class TransactionDB : public StackableDB {
Expand Down
5 changes: 0 additions & 5 deletions util/autovector.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,6 @@ class autovector {

autovector& operator=(const autovector& other) { return assign(other); }

// move operation are disallowed since it is very hard to make sure both
// autovectors are allocated from the same function stack.
autovector& operator=(autovector&& other) = delete;
autovector(autovector&& other) = delete;

// -- Iterator Operations
iterator begin() { return iterator(this, 0); }

Expand Down
4 changes: 3 additions & 1 deletion utilities/transactions/optimistic_transaction_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ Status OptimisticTransactionImpl::Rollback() {
}

// Record this key so that we can check it for conflicts at commit time.
//
// 'exclusive' is unused for OptimisticTransaction.
Status OptimisticTransactionImpl::TryLock(ColumnFamilyHandle* column_family,
const Slice& key, bool read_only,
bool untracked) {
bool exclusive, bool untracked) {
if (untracked) {
return Status::OK();
}
Expand Down
3 changes: 2 additions & 1 deletion utilities/transactions/optimistic_transaction_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ class OptimisticTransactionImpl : public TransactionBaseImpl {

protected:
Status TryLock(ColumnFamilyHandle* column_family, const Slice& key,
bool read_only, bool untracked = false) override;
bool read_only, bool exclusive,
bool untracked = false) override;

private:
OptimisticTransactionDB* const txn_db_;
Expand Down
53 changes: 31 additions & 22 deletions utilities/transactions/transaction_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void TransactionBaseImpl::SetSnapshotIfNeeded() {

Status TransactionBaseImpl::TryLock(ColumnFamilyHandle* column_family,
const SliceParts& key, bool read_only,
bool untracked) {
bool exclusive, bool untracked) {
size_t key_size = 0;
for (int i = 0; i < key.num_parts; ++i) {
key_size += key.parts[i].size();
Expand All @@ -109,7 +109,7 @@ Status TransactionBaseImpl::TryLock(ColumnFamilyHandle* column_family,
str.append(key.parts[i].data(), key.parts[i].size());
}

return TryLock(column_family, str, read_only, untracked);
return TryLock(column_family, str, read_only, exclusive, untracked);
}

void TransactionBaseImpl::SetSavePoint() {
Expand Down Expand Up @@ -187,8 +187,9 @@ Status TransactionBaseImpl::Get(const ReadOptions& read_options,

Status TransactionBaseImpl::GetForUpdate(const ReadOptions& read_options,
ColumnFamilyHandle* column_family,
const Slice& key, std::string* value) {
Status s = TryLock(column_family, key, true /* read_only */);
const Slice& key, std::string* value,
bool exclusive) {
Status s = TryLock(column_family, key, true /* read_only */, exclusive);

if (s.ok() && value != nullptr) {
s = Get(read_options, column_family, key, value);
Expand Down Expand Up @@ -222,7 +223,8 @@ std::vector<Status> TransactionBaseImpl::MultiGetForUpdate(

// Lock all keys
for (size_t i = 0; i < num_keys; ++i) {
Status s = TryLock(column_family[i], keys[i], true /* read_only */);
Status s = TryLock(column_family[i], keys[i], true /* read_only */,
true /* exclusive */);
if (!s.ok()) {
// Fail entire multiget if we cannot lock all keys
return std::vector<Status>(num_keys, s);
Expand Down Expand Up @@ -256,7 +258,8 @@ Iterator* TransactionBaseImpl::GetIterator(const ReadOptions& read_options,

Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family,
const Slice& key, const Slice& value) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->Put(column_family, key, value);
Expand All @@ -269,7 +272,8 @@ Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family,
Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family,
const SliceParts& key,
const SliceParts& value) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->Put(column_family, key, value);
Expand All @@ -281,7 +285,8 @@ Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::Merge(ColumnFamilyHandle* column_family,
const Slice& key, const Slice& value) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->Merge(column_family, key, value);
Expand All @@ -293,7 +298,8 @@ Status TransactionBaseImpl::Merge(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family,
const Slice& key) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->Delete(column_family, key);
Expand All @@ -305,7 +311,8 @@ Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family,
const SliceParts& key) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->Delete(column_family, key);
Expand All @@ -317,7 +324,8 @@ Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family,
const Slice& key) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->SingleDelete(column_family, key);
Expand All @@ -329,7 +337,8 @@ Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family,
const SliceParts& key) {
Status s = TryLock(column_family, key, false /* read_only */);
Status s =
TryLock(column_family, key, false /* read_only */, true /* exclusive */);

if (s.ok()) {
GetBatchForWrite()->SingleDelete(column_family, key);
Expand All @@ -341,8 +350,8 @@ Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::PutUntracked(ColumnFamilyHandle* column_family,
const Slice& key, const Slice& value) {
Status s =
TryLock(column_family, key, false /* read_only */, true /* untracked */);
Status s = TryLock(column_family, key, false /* read_only */,
true /* exclusive */, true /* untracked */);

if (s.ok()) {
GetBatchForWrite()->Put(column_family, key, value);
Expand All @@ -355,8 +364,8 @@ Status TransactionBaseImpl::PutUntracked(ColumnFamilyHandle* column_family,
Status TransactionBaseImpl::PutUntracked(ColumnFamilyHandle* column_family,
const SliceParts& key,
const SliceParts& value) {
Status s =
TryLock(column_family, key, false /* read_only */, true /* untracked */);
Status s = TryLock(column_family, key, false /* read_only */,
true /* exclusive */, true /* untracked */);

if (s.ok()) {
GetBatchForWrite()->Put(column_family, key, value);
Expand All @@ -369,8 +378,8 @@ Status TransactionBaseImpl::PutUntracked(ColumnFamilyHandle* column_family,
Status TransactionBaseImpl::MergeUntracked(ColumnFamilyHandle* column_family,
const Slice& key,
const Slice& value) {
Status s =
TryLock(column_family, key, false /* read_only */, true /* untracked */);
Status s = TryLock(column_family, key, false /* read_only */,
true /* exclusive */, true /* untracked */);

if (s.ok()) {
GetBatchForWrite()->Merge(column_family, key, value);
Expand All @@ -382,8 +391,8 @@ Status TransactionBaseImpl::MergeUntracked(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::DeleteUntracked(ColumnFamilyHandle* column_family,
const Slice& key) {
Status s =
TryLock(column_family, key, false /* read_only */, true /* untracked */);
Status s = TryLock(column_family, key, false /* read_only */,
true /* exclusive */, true /* untracked */);

if (s.ok()) {
GetBatchForWrite()->Delete(column_family, key);
Expand All @@ -395,8 +404,8 @@ Status TransactionBaseImpl::DeleteUntracked(ColumnFamilyHandle* column_family,

Status TransactionBaseImpl::DeleteUntracked(ColumnFamilyHandle* column_family,
const SliceParts& key) {
Status s =
TryLock(column_family, key, false /* read_only */, true /* untracked */);
Status s = TryLock(column_family, key, false /* read_only */,
true /* exclusive */, true /* untracked */);

if (s.ok()) {
GetBatchForWrite()->Delete(column_family, key);
Expand Down
12 changes: 7 additions & 5 deletions utilities/transactions/transaction_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class TransactionBaseImpl : public Transaction {
// untracked will be true if called from PutUntracked, DeleteUntracked, or
// MergeUntracked.
virtual Status TryLock(ColumnFamilyHandle* column_family, const Slice& key,
bool read_only, bool untracked = false) = 0;
bool read_only, bool exclusive,
bool untracked = false) = 0;

void SetSavePoint() override;

Expand All @@ -55,11 +56,12 @@ class TransactionBaseImpl : public Transaction {

Status GetForUpdate(const ReadOptions& options,
ColumnFamilyHandle* column_family, const Slice& key,
std::string* value) override;
std::string* value, bool exclusive) override;

Status GetForUpdate(const ReadOptions& options, const Slice& key,
std::string* value) override {
return GetForUpdate(options, db_->DefaultColumnFamily(), key, value);
std::string* value, bool exclusive) override {
return GetForUpdate(options, db_->DefaultColumnFamily(), key, value,
exclusive);
}

std::vector<Status> MultiGet(
Expand Down Expand Up @@ -315,7 +317,7 @@ class TransactionBaseImpl : public Transaction {
std::shared_ptr<TransactionNotifier> snapshot_notifier_ = nullptr;

Status TryLock(ColumnFamilyHandle* column_family, const SliceParts& key,
bool read_only, bool untracked = false);
bool read_only, bool exclusive, bool untracked = false);

WriteBatchBase* GetBatchForWrite();

Expand Down
4 changes: 2 additions & 2 deletions utilities/transactions/transaction_db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ Status TransactionDBImpl::DropColumnFamily(ColumnFamilyHandle* column_family) {
}

Status TransactionDBImpl::TryLock(TransactionImpl* txn, uint32_t cfh_id,
const std::string& key) {
return lock_mgr_.TryLock(txn, cfh_id, key, GetEnv());
const std::string& key, bool exclusive) {
return lock_mgr_.TryLock(txn, cfh_id, key, GetEnv(), exclusive);
}

void TransactionDBImpl::UnLock(TransactionImpl* txn,
Expand Down
3 changes: 2 additions & 1 deletion utilities/transactions/transaction_db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class TransactionDBImpl : public TransactionDB {
using StackableDB::DropColumnFamily;
virtual Status DropColumnFamily(ColumnFamilyHandle* column_family) override;

Status TryLock(TransactionImpl* txn, uint32_t cfh_id, const std::string& key);
Status TryLock(TransactionImpl* txn, uint32_t cfh_id, const std::string& key,
bool exclusive);

void UnLock(TransactionImpl* txn, const TransactionKeyMap* keys);
void UnLock(TransactionImpl* txn, uint32_t cfh_id, const std::string& key);
Expand Down
7 changes: 3 additions & 4 deletions utilities/transactions/transaction_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ TransactionImpl::TransactionImpl(TransactionDB* txn_db,
: TransactionBaseImpl(txn_db->GetRootDB(), write_options),
txn_db_impl_(nullptr),
txn_id_(0),
waiting_txn_id_(0),
waiting_cf_id_(0),
waiting_key_(nullptr),
expiration_time_(0),
Expand Down Expand Up @@ -395,7 +394,7 @@ Status TransactionImpl::LockBatch(WriteBatch* batch,
for (const auto& key_iter : cfh_keys) {
const std::string& key = key_iter;

s = txn_db_impl_->TryLock(this, cfh_id, key);
s = txn_db_impl_->TryLock(this, cfh_id, key, true /* exclusive */);
if (!s.ok()) {
break;
}
Expand All @@ -422,7 +421,7 @@ Status TransactionImpl::LockBatch(WriteBatch* batch,
// the snapshot time.
Status TransactionImpl::TryLock(ColumnFamilyHandle* column_family,
const Slice& key, bool read_only,
bool untracked) {
bool exclusive, bool untracked) {
uint32_t cfh_id = GetColumnFamilyID(column_family);
std::string key_str = key.ToString();
bool previously_locked;
Expand All @@ -448,7 +447,7 @@ Status TransactionImpl::TryLock(ColumnFamilyHandle* column_family,

// lock this key if this transactions hasn't already locked it
if (!previously_locked) {
s = txn_db_impl_->TryLock(this, cfh_id, key_str);
s = txn_db_impl_->TryLock(this, cfh_id, key_str, exclusive);
}

SetSnapshotIfNeeded();
Expand Down
Loading

0 comments on commit 2005c88

Please sign in to comment.