mongodb/mongo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SERVER-114838, SERVER-115296: Salt ResourceIds with a random value acquired at startup (#45118)

Browse Source 
GitOrigin-RevId: 3517e9d98d24fcde732f6ab952527d049066f31f
This commit is contained in:
Jordi Olivares Provencio 2025-12-18 18:01:58 +01:00 committed by MongoDB Bot
parent 048a17c79f
commit dd9d82c6da
5 changed files with 83 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/mongo/db/concurrency/lock_manager.cpp
View File

@ -698,7 +698,7 @@ void LockManager::getLockInfoArray(const std::map<LockerId, BSONObj>& lockToClie
auto o = BSONObjBuilder(locks->subobjStart());
if (forLogging)
o.append("lockAddr", formatPtr(lock));
o.append("resourceId", lock->resourceId.toString());
o.append("resourceId", toStringForLogging(lock->resourceId));
struct {
StringData key;
LockRequest* iter;

40
src/mongo/db/concurrency/lock_manager_defs.cpp
View File

@ -47,13 +47,13 @@ const ResourceId resourceIdMultiDocumentTransactionsBarrier = ResourceId(
const ResourceId resourceIdReplicationStateTransitionLock = ResourceId(
RESOURCE_GLOBAL, static_cast<uint8_t>(ResourceGlobalId::kReplicationStateTransitionLock));
std::string ResourceId::toString() const {
std::string toStringForLogging(const ResourceId& rId) {
StringBuilder ss;
ss << "{" << _fullHash << ": " << resourceTypeName(getType()) << ", " << getHashId();
if (getType() == RESOURCE_DATABASE || getType() == RESOURCE_COLLECTION ||
getType() == RESOURCE_MUTEX || getType() == RESOURCE_DDL_DATABASE ||
getType() == RESOURCE_DDL_COLLECTION) {
if (auto resourceName = ResourceCatalog::get().name(*this)) {
const auto type = rId.getType();
ss << "{" << rId._fullHash << ": " << resourceTypeName(type) << ", " << rId.getHashId();
if (type == RESOURCE_DATABASE || type == RESOURCE_COLLECTION || type == RESOURCE_MUTEX ||
type == RESOURCE_DDL_DATABASE || type == RESOURCE_DDL_COLLECTION) {
if (auto resourceName = ResourceCatalog::get().name(rId)) {
ss << ", " << *resourceName;
}
}
@ -62,6 +62,34 @@ std::string ResourceId::toString() const {
return ss.str();
}
std::string ResourceId::toStringForErrorMessage() const {
StringBuilder ss;
const auto type = getType();
ss << "{" << resourceTypeName(type);
switch (type) {
case RESOURCE_GLOBAL:
ss << " : " << getHashId();
break;
case RESOURCE_DATABASE:
case RESOURCE_COLLECTION:
case RESOURCE_DDL_DATABASE:
case RESOURCE_DDL_COLLECTION:
case RESOURCE_MUTEX:
if (auto resourceName = ResourceCatalog::get().name(*this)) {
ss << " : " << *resourceName;
}
break;
case ResourceTypesCount:
case RESOURCE_INVALID:
case RESOURCE_METADATA:
case RESOURCE_TENANT:
break;
}
ss << "}";
return ss.str();
}
void LockRequest::initNew(Locker* locker, LockGrantNotification* notify) {
this->locker = locker;
this->notify = notify;

35
src/mongo/db/concurrency/lock_manager_defs.h
View File

@ -35,6 +35,7 @@
#include "mongo/db/database_name.h"
#include "mongo/db/namespace_string.h"
#include "mongo/db/tenant_id.h"
#include "mongo/platform/random.h"
#include "mongo/util/assert_util.h"
#include <cstdint>
@ -238,6 +239,12 @@ constexpr const char* resourceGlobalIdName(ResourceGlobalId id) {
return ResourceGlobalIdNames[static_cast<uint8_t>(id)];
}
inline static uint64_t hashStringDataForResourceId(StringData str, uint64_t salt) {
// We salt the hash with a given random value to generate randomness in ResourceId selection on
// every restart. This aids in testing for detecting lock ordering issues.
return absl::hash_internal::CityHash64WithSeed(str.data(), str.size(), salt);
}
/**
* Uniquely identifies a lockable resource.
*/
@ -248,18 +255,23 @@ public:
MONGO_STATIC_ASSERT(ResourceTypesCount <= (1 << resourceTypeBits));
ResourceId(ResourceType type, const NamespaceString& nss)
: _fullHash(fullHash(type, hashStringData(nss.toStringForResourceId()))) {
: _fullHash(fullHash(type,
hashStringDataForResourceId(nss.toStringForResourceId(),
kHashingSaltForResourceId))) {
verifyNoResourceMutex(type);
}
ResourceId(ResourceType type, const DatabaseName& dbName)
: _fullHash(fullHash(type, hashStringData(dbName.toStringForResourceId()))) {
: _fullHash(fullHash(type,
hashStringDataForResourceId(dbName.toStringForResourceId(),
kHashingSaltForResourceId))) {
verifyNoResourceMutex(type);
}
ResourceId(ResourceType type, uint64_t hashId) : _fullHash(fullHash(type, hashId)) {
verifyNoResourceMutex(type);
}
ResourceId(ResourceType type, const TenantId& tenantId)
: _fullHash{fullHash(type, hashStringData(tenantId.toString()))} {
: _fullHash{fullHash(
type, hashStringDataForResourceId(tenantId.toString(), kHashingSaltForResourceId))} {
verifyNoResourceMutex(type);
}
constexpr ResourceId() : _fullHash(0) {}
@ -285,7 +297,9 @@ public:
return _fullHash & (std::numeric_limits<uint64_t>::max() >> resourceTypeBits);
}
std::string toString() const;
// String representation of the resource type that omits the parts not intended to be read by
// humans. Intended to be used for error messages that are returned to the client.
std::string toStringForErrorMessage() const;
template <typename H>
friend H AbslHashValue(H h, const ResourceId& resource) {
@ -294,6 +308,7 @@ public:
private:
friend class ResourceCatalog;
friend std::string toStringForLogging(const ResourceId&);
ResourceId(uint64_t fullHash) : _fullHash(fullHash) {}
@ -315,11 +330,17 @@ private:
(hashId & (std::numeric_limits<uint64_t>::max() >> resourceTypeBits));
}
static uint64_t hashStringData(StringData str) {
return absl::hash_internal::CityHash64(str.data(), str.size());
}
static inline const uint64_t kHashingSaltForResourceId = [] {
SecureUrbg entropy;
const auto result = entropy();
static_assert(std::is_same_v<std::remove_const_t<decltype(result)>, uint64_t>,
"salting hash entropy must be a uint64");
return result;
}();
};
std::string toStringForLogging(const ResourceId&);
#ifndef MONGO_CONFIG_DEBUG_BUILD
// Treat the resource ids as 64-bit integers in release mode in order to ensure we do
// not spend too much time doing comparisons for hashing.

12
src/mongo/db/concurrency/lock_manager_test.cpp
View File

@ -92,6 +92,18 @@ TEST(ResourceIdTest, Masking) {
}
}
TEST(ResourceIdTest, SaltingWorks) {
const std::string collAName = "db1.collA";
const uint64_t salt1 = 0;
const uint64_t salt2 = 1;
const auto id1Salt1 = hashStringDataForResourceId(collAName, salt1);
const auto id1Salt2 = hashStringDataForResourceId(collAName, salt2);
ASSERT_NE(id1Salt1, id1Salt2);
}
} // namespace resource_id_test
namespace lock_manager_test {

15
src/mongo/db/concurrency/locker.cpp
View File

@ -343,7 +343,7 @@ void Locker::reacquireTicket(OperationContext* opCtx) {
fmt::format("Unable to acquire ticket with mode '{}' due to detected lock "
"conflict for resource {}",
fmt::underlying(_modeForTicket),
it.key().toString()),
it.key().toStringForErrorMessage()),
!_lockManager->hasConflictingRequests(it.key(), it.objAddr()));
}
} while (!_acquireTicket(opCtx, _modeForTicket, Date_t::now() + Milliseconds{100}));
@ -839,7 +839,7 @@ void Locker::dump() const {
BSONObj toBSON() const {
BSONObjBuilder b;
b.append("key", key.toString());
b.append("key", toStringForLogging(key));
b.append("status", lockRequestStatusName(status));
b.append("recursiveCount", static_cast<int>(recursiveCount));
b.append("unlockPending", static_cast<int>(unlockPending));
@ -872,7 +872,7 @@ LockResult Locker::_lockBegin(OperationContext* opCtx, ResourceId resId, LockMod
resType != RESOURCE_DDL_COLLECTION)) {
LOGV2_FATAL(9360800,
"Operation attempted to acquire lock after indicating that it should not",
"resourceId"_attr = resId.toString(),
"resourceId"_attr = resId,
"mode"_attr = modeName(mode));
}
@ -1008,7 +1008,7 @@ void Locker::_lockComplete(OperationContext* opCtx,
resType != RESOURCE_DDL_COLLECTION)) {
LOGV2_FATAL(9360801,
"Operation attempted to acquire lock after indicating that it should not",
"resourceId"_attr = resId.toString(),
"resourceId"_attr = resId,
"mode"_attr = modeName(mode));
}
@ -1026,7 +1026,8 @@ void Locker::_lockComplete(OperationContext* opCtx,
if (!opCtx->uninterruptibleLocksRequested_DO_NOT_USE() && isUserOperation && // NOLINT
MONGO_unlikely(failNonIntentLocksIfWaitNeeded.shouldFail())) {
uassert(ErrorCodes::LockTimeout,
str::stream() << "Cannot immediately acquire lock '" << resId.toString()
str::stream() << "Cannot immediately acquire lock '"
<< resId.toStringForErrorMessage()
<< "'. Timing out due to failpoint.",
(mode == MODE_IS || mode == MODE_IX));
}
@ -1089,8 +1090,8 @@ void Locker::_lockComplete(OperationContext* opCtx,
onTimeout();
}
std::string timeoutMessage = str::stream()
<< "Unable to acquire " << modeName(mode) << " lock on '" << resId.toString()
<< "' within " << timeout << ".";
<< "Unable to acquire " << modeName(mode) << " lock on '"
<< resId.toStringForErrorMessage() << "' within " << timeout << ".";
if (opCtx->getClient()) {
timeoutMessage = str::stream()
<< timeoutMessage << " opId: " << opCtx->getOpID()