Skip to content

Commit

Permalink
[#25322] docdb: Remove use of dynamic dispatch in SharedLockManager/O…
Browse files Browse the repository at this point in the history 
…bjectLockManager

Summary:
C++ classes with at least one method declared `virtual` result in a vtable being generated
and a pointer to it stored in every instance in order to handle dynamic dispatch. This is
incompatible with placing instances of the class in shared memory, since the vtable location is at
different addresses in different processes due to ASLR.

We currently use dynamic dispatch to handle differences in LockTracker functionality for
SharedLockManager (do nothing) and ObjectLockManager (do something). This diff refactors this code
to use static dispatch instead, in preparation for moving the lock manager into shared memory.

Some locking specific code was also moved from docdb.h/docdb.cc to its own file.
Jira: DB-14528

Test Plan: Jenkins

Reviewers: bkolagani

Reviewed By: bkolagani

Subscribers: ybase

Differential Revision: https://phorge.dev.yugabyte.com/D40721
  • Loading branch information
@es1024
es1024 committed Dec 17, 2024
1 parent e764658 commit 4bf9d89
Show file tree
Hide file tree
Showing 12 changed files with 504 additions and 409 deletions.
1 change: 1 addition & 0 deletions src/yb/docdb/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ set(DOCDB_SRCS
key_bounds.cc
kv_debug.cc
lock_batch.cc
lock_util.cc
pgsql_operation.cc
ql_rocksdb_storage.cc
ql_rowwise_iterator_interface.cc
Expand Down
148 changes: 5 additions & 143 deletions src/yb/docdb/docdb.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,33 +93,24 @@ namespace {
// key should be valid prefix of doc key, ending with some complete primitive value or group end.
Status ApplyIntent(
RefCntPrefix key, dockv::IntentTypeSet intent_types,
LockBatchEntries<RefCntPrefix>* keys_locked) {
LockBatchEntries<SharedLockManager>* keys_locked) {
RSTATUS_DCHECK(!intent_types.None(), InternalError, "Empty intent types is not allowed");
// Have to strip kGroupEnd from end of key, because when only hash key is specified, we will
// get two kGroupEnd at end of strong intent.
RETURN_NOT_OK(dockv::RemoveGroupEndSuffix(&key));
keys_locked->push_back(
LockBatchEntry<RefCntPrefix> {std::move(key), intent_types});
LockBatchEntry<SharedLockManager> {std::move(key), intent_types});
return Status::OK();
}

Status FormSharedLock(
ObjectLockPrefix&& key, dockv::IntentTypeSet intent_types,
LockBatchEntries<ObjectLockPrefix>* keys_locked) {
SCHECK(!intent_types.None(), InternalError, "Empty intent types is not allowed");
keys_locked->push_back(
LockBatchEntry<ObjectLockPrefix> {.key = std::move(key), .intent_types = intent_types});
return Status::OK();
}

Result<DetermineKeysToLockResult<RefCntPrefix>> DetermineKeysToLock(
Result<DetermineKeysToLockResult<SharedLockManager>> DetermineKeysToLock(
const std::vector<std::unique_ptr<DocOperation>>& doc_write_ops,
const ArenaList<LWKeyValuePairPB>& read_pairs,
IsolationLevel isolation_level,
RowMarkType row_mark_type,
bool transactional_table,
dockv::PartialRangeKeyIntents partial_range_key_intents) {
DetermineKeysToLockResult<RefCntPrefix> result;
DetermineKeysToLockResult<SharedLockManager> result;
boost::container::small_vector<RefCntPrefix, 8> doc_paths;
boost::container::small_vector<size_t, 32> key_prefix_lengths;
result.need_read_snapshot = false;
Expand Down Expand Up @@ -195,47 +186,6 @@ Result<DetermineKeysToLockResult<RefCntPrefix>> DetermineKeysToLock(
return result;
}

// Collapse keys_locked into a unique set of keys with intent_types representing the union of
// intent_types originally present. In other words, suppose keys_locked is originally the following:
// [
// (k1, {kWeakRead, kWeakWrite}),
// (k1, {kStrongRead}),
// (k2, {kWeakRead}),
// (k3, {kStrongRead}),
// (k2, {kStrongWrite}),
// ]
// Then after calling FilterKeysToLock we will have:
// [
// (k1, {kWeakRead, kWeakWrite, kStrongRead}),
// (k2, {kWeakRead}),
// (k3, {kStrongRead, kStrongWrite}),
// ]
// Note that only keys which appear in order in keys_locked will be collapsed in this manner.
template <typename T>
void FilterKeysToLock(LockBatchEntries<T> *keys_locked) {
if (keys_locked->empty()) {
return;
}

std::sort(keys_locked->begin(), keys_locked->end(),
[](const auto& lhs, const auto& rhs) {
return lhs.key < rhs.key;
});

auto w = keys_locked->begin();
for (auto it = keys_locked->begin(); ++it != keys_locked->end();) {
if (it->key == w->key) {
w->intent_types |= it->intent_types;
} else {
++w;
*w = *it;
}
}

++w;
keys_locked->erase(w, keys_locked->end());
}

} // namespace

Result<PrepareDocWriteOperationResult> PrepareDocWriteOperation(
Expand All @@ -262,7 +212,7 @@ Result<PrepareDocWriteOperationResult> PrepareDocWriteOperation(
}
result.need_read_snapshot = determine_keys_to_lock_result.need_read_snapshot;

FilterKeysToLock<RefCntPrefix>(&determine_keys_to_lock_result.lock_batch);
FilterKeysToLock<SharedLockManager>(&determine_keys_to_lock_result.lock_batch);
VLOG_WITH_FUNC(4) << "filtered determine_keys_to_lock_result="
<< determine_keys_to_lock_result.ToString();
const MonoTime start_time = (tablet_metrics != nullptr) ? MonoTime::Now() : MonoTime();
Expand Down Expand Up @@ -524,93 +474,5 @@ void CombineExternalIntents(
provider->SetValue(buffer.AsSlice());
}

// We associate a list of <KeyEntryType, IntentTypeSet> to each table lock type such that the
// table lock conflict matrix of postgres is preserved.
//
// For instance, let's consider 'ROW_SHARE' and 'EXCLUSIVE' lock modes.
// 1. 'ROW_SHARE' lock mode on object would lead to the following keys
// [<object/object hash/other prefix> kWeakObjectLock] [kStrongRead]
// 2. 'EXCLUSIVE' lock mode on the same object would lead to the following keys
// [<object/object hash/other prefix> kWeakObjectLock] [kWeakWrite]
// [<object/object hash/other prefix> kStrongObjectLock] [kStrongRead, kStrongWrite]
//
// When checking conflicts for the same key, '[<object/object hash/other prefix> kWeakObjectLock]'
// in this case, we see that the intents requested are [kStrongRead] and [kWeakWrite] for modes
// 'ROW_SHARE' and 'EXCLUSIVE' respectively. And since the above intenttype sets conflict among
// themselves, we successfully detect the conflict.
const std::vector<std::pair<KeyEntryType, dockv::IntentTypeSet>>& GetEntriesForLockType(
TableLockType lock) {
static const std::array<
std::vector<std::pair<KeyEntryType, dockv::IntentTypeSet>>,
TableLockType_ARRAYSIZE> lock_entries = {{
// NONE
{{}},
// ACCESS_SHARE
{{
{KeyEntryType::kWeakObjectLock, dockv::IntentTypeSet {dockv::IntentType::kWeakRead}}
}},
// ROW_SHARE
{{
{KeyEntryType::kWeakObjectLock, dockv::IntentTypeSet {dockv::IntentType::kStrongRead}}
}},
// ROW_EXCLUSIVE
{{
{KeyEntryType::kStrongObjectLock, dockv::IntentTypeSet {dockv::IntentType::kWeakRead}}
}},
// SHARE_UPDATE_EXCLUSIVE
{{
{
KeyEntryType::kStrongObjectLock,
dockv::IntentTypeSet {dockv::IntentType::kStrongRead, dockv::IntentType::kWeakWrite}
}
}},
// SHARE
{{
{KeyEntryType::kStrongObjectLock, dockv::IntentTypeSet {dockv::IntentType::kStrongWrite}}
}},
// SHARE_ROW_EXCLUSIVE
{{
{
KeyEntryType::kStrongObjectLock,
dockv::IntentTypeSet {dockv::IntentType::kWeakRead, dockv::IntentType::kStrongWrite}
}
}},
// EXCLUSIVE
{{
{KeyEntryType::kWeakObjectLock, dockv::IntentTypeSet {dockv::IntentType::kWeakWrite}},
{
KeyEntryType::kStrongObjectLock,
dockv::IntentTypeSet {dockv::IntentType::kStrongRead, dockv::IntentType::kStrongWrite}
}
}},
// ACCESS_EXCLUSIVE
{{
{KeyEntryType::kWeakObjectLock, dockv::IntentTypeSet {dockv::IntentType::kStrongWrite}},
{
KeyEntryType::kStrongObjectLock,
dockv::IntentTypeSet {dockv::IntentType::kStrongRead, dockv::IntentType::kStrongWrite}
}
}}
}};
return lock_entries[lock];
}

// Returns a DetermineKeysToLockResult object with its lock_batch containing a list of entries with
// 'key' as <object id, KeyEntry> and 'intent_types' set.
Result<DetermineKeysToLockResult<ObjectLockPrefix>> DetermineObjectsToLock(
const google::protobuf::RepeatedPtrField<ObjectLockPB>& objects_to_lock) {
DetermineKeysToLockResult<ObjectLockPrefix> result;
for (const auto& object_lock : objects_to_lock) {
SCHECK(object_lock.has_object_oid(), IllegalState, "ObjectLockPB has empty object oid");
SCHECK(object_lock.has_database_oid(), IllegalState, "ObjectLockPB has empty database oid");
for (const auto& [lock_key, intent_types] : GetEntriesForLockType(object_lock.lock_type())) {
ObjectLockPrefix key(object_lock.database_oid(), object_lock.object_oid(), lock_key);
RETURN_NOT_OK(FormSharedLock(std::move(key), intent_types, &result.lock_batch));
}
}
FilterKeysToLock<ObjectLockPrefix>(&result.lock_batch);
return result;
}

} // namespace docdb
} // namespace yb
34 changes: 3 additions & 31 deletions src/yb/docdb/docdb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "yb/docdb/docdb.pb.h"
#include "yb/docdb/docdb_types.h"
#include "yb/docdb/lock_batch.h"
#include "yb/docdb/lock_util.h"
#include "yb/dockv/subdocument.h"
#include "yb/dockv/value.h"

Expand Down Expand Up @@ -157,19 +158,6 @@ Status EnumerateIntents(
const dockv::EnumerateIntentsCallback& functor,
dockv::PartialRangeKeyIntents partial_range_key_intents);

// With the exception of table-locks/object-locks, type T below always takes value 'RefCntPrefix'.
// The TSLocalLockManager instantiates an ObjectLockManager that uses LockManagerImpl with
// 'ObjectLockPrefix' type and the relevant locking codepath uses DetermineKeysToLockResult struct
// with type 'ObjectLockPrefix'.
template <typename T>
struct DetermineKeysToLockResult {
LockBatchEntries<T> lock_batch;
bool need_read_snapshot;

std::string ToString() const {
return YB_STRUCT_TO_STRING(lock_batch, need_read_snapshot);
}
};

// replicated_batches_state format does not matter at this point, because it is just
// appended to appropriate value.
Expand Down Expand Up @@ -275,21 +263,5 @@ void CombineExternalIntents(
SubTransactionId subtransaction_id,
ExternalIntentsProvider* provider);

// We achieve the same table lock conflict matrix as that of pg documented here,
// https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-TABLES
//
// We only have 4 lock/intent modes kWeak/kStrong Read/Write, but to generate the above conflict
// matrix, we would need more lock types. Instead of introducing additional lock types, we use two
// KeyEntryType values and associate a list of <KeyEntryType, IntentTypeSet> to each table lock.
// Since our conflict detection mechanism checks conflicts against each key, we indirectly achieve
// the exact same conflict matrix. Refer comments on the function definition for more details.
const std::vector<std::pair<dockv::KeyEntryType, dockv::IntentTypeSet>>&
GetEntriesForLockType(TableLockType lock);

// Returns DetermineKeysToLockResult<ObjectLockPrefix> which can further be passed to
// ObjectLockManager to acquire locks against the required objects with the given lock type.
Result<DetermineKeysToLockResult<ObjectLockPrefix>> DetermineObjectsToLock(
const google::protobuf::RepeatedPtrField<ObjectLockPB>& objects_to_lock);

} // namespace docdb
} // namespace yb
} // namespace docdb
} // namespace yb
4 changes: 2 additions & 2 deletions src/yb/docdb/docdb_fwd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ struct VersionedTransaction;

using DocKeyHash = uint16_t;
using DocReadContextPtr = std::shared_ptr<DocReadContext>;
template <typename T>
using LockBatchEntries = std::vector<LockBatchEntry<T>>;
template <typename LockManager>
using LockBatchEntries = std::vector<LockBatchEntry<LockManager>>;
// Lock state stores the number of locks acquired for each intent type.
// The count for each intent type resides in sequential bits (block) in lock state.
// For example the count of locks on a particular intent type could be received as:
Expand Down
14 changes: 7 additions & 7 deletions src/yb/docdb/lock_batch.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace yb {
namespace docdb {

LockBatch::LockBatch(
SharedLockManager* lock_manager, LockBatchEntries<RefCntPrefix>&& key_to_intent_type,
SharedLockManager* lock_manager, LockBatchEntries<SharedLockManager>&& key_to_intent_type,
CoarseTimePoint deadline) : data_(std::move(key_to_intent_type), lock_manager) {
Init(deadline);
if (!data_.status.ok()) {
Expand All @@ -45,7 +45,7 @@ LockBatch::LockBatch(UnlockedBatch* unlocked_batch, CoarseTimePoint deadline)
}

void LockBatch::Init(CoarseTimePoint deadline) {
if (!empty() && !data_.shared_lock_manager->Lock(&data_.key_to_type, deadline)) {
if (!empty() && !data_.shared_lock_manager->Lock(data_.key_to_type, deadline)) {
std::string batch_str;
if (FLAGS_dump_lock_keys) {
batch_str = Format(", batch: $0", data_.key_to_type);
Expand Down Expand Up @@ -81,13 +81,13 @@ void LockBatch::MoveFrom(LockBatch* other) {
other->data_.key_to_type.clear();
}

template <typename T>
std::string LockBatchEntry<T>::ToString() const {
template <typename LockManager>
std::string LockBatchEntry<LockManager>::ToString() const {
return YB_STRUCT_TO_STRING(key, intent_types, existing_state);
}

UnlockedBatch::UnlockedBatch(
LockBatchEntries<RefCntPrefix>&& key_to_type,
LockBatchEntries<SharedLockManager>&& key_to_type,
SharedLockManager* shared_lock_manager):
key_to_type_(std::move(key_to_type)), shared_lock_manager_(shared_lock_manager) {}

Expand All @@ -108,8 +108,8 @@ void UnlockedBatch::MoveFrom(UnlockedBatch* other) {
other->shared_lock_manager_ = nullptr;
}

template struct LockBatchEntry<RefCntPrefix>;
template struct LockBatchEntry<ObjectLockPrefix>;
template struct LockBatchEntry<SharedLockManager>;
template struct LockBatchEntry<ObjectLockManager>;

} // namespace docdb
} // namespace yb
Loading

0 comments on commit 4bf9d89

Please sign in to comment.