From 0e9dabcabb14b3f5d1483595a39b89721e5c0072 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 23 Jul 2021 10:20:50 -0700 Subject: [PATCH] Remove mutex that was only needed for a minor optimization. --- fdbclient/MultiVersionTransaction.actor.cpp | 36 ++++++++++----------- fdbclient/MultiVersionTransaction.h | 2 -- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/fdbclient/MultiVersionTransaction.actor.cpp b/fdbclient/MultiVersionTransaction.actor.cpp index 07444d0584..eedacf80aa 100644 --- a/fdbclient/MultiVersionTransaction.actor.cpp +++ b/fdbclient/MultiVersionTransaction.actor.cpp @@ -896,25 +896,25 @@ MultiVersionDatabase::MultiVersionDatabase(MultiVersionApi* api, api->runOnExternalClients(threadIdx, [this](Reference client) { dbState->addClient(client); }); api->runOnExternalClientsAllThreads([&clusterFilePath](Reference client) { - // This creates a database to initialize some client state on the external library - // We only do this on 6.2+ clients to avoid some bugs associated with older versions - // This deletes the new database immediately to discard its connections + // This creates a database to initialize some client state on the external library. + // We only do this on 6.2+ clients to avoid some bugs associated with older versions. + // This deletes the new database immediately to discard its connections. + // + // Simultaneous attempts to create a database could result in us running this initialization + // code in multiple threads simultaneously. It is necessary that each attempt have a chance + // to run this initialization in case the other fails, and it's safe to run them in parallel. if (client->protocolVersion.hasCloseUnusedConnection() && !client->initialized) { - MutexHolder holder(client->initializationMutex); - - if (!client->initialized) { - try { - Reference newDb = client->api->createDatabase(clusterFilePath.c_str()); - client->initialized = true; - } catch (Error& e) { - // This connection is not initialized. It is still possible to connect with it, - // but we may not see trace logs from this client until a successful connection - // is established. - TraceEvent(SevWarnAlways, "FailedToInitializeExternalClient") - .detail("LibraryPath", client->libPath) - .detail("ClusterFilePath", clusterFilePath) - .error(e); - } + try { + Reference newDb = client->api->createDatabase(clusterFilePath.c_str()); + client->initialized = true; + } catch (Error& e) { + // This connection is not initialized. It is still possible to connect with it, + // but we may not see trace logs from this client until a successful connection + // is established. + TraceEvent(SevWarnAlways, "FailedToInitializeExternalClient") + .detail("LibraryPath", client->libPath) + .detail("ClusterFilePath", clusterFilePath) + .error(e); } } }); diff --git a/fdbclient/MultiVersionTransaction.h b/fdbclient/MultiVersionTransaction.h index 0896d676a8..274df7dd84 100644 --- a/fdbclient/MultiVersionTransaction.h +++ b/fdbclient/MultiVersionTransaction.h @@ -420,8 +420,6 @@ struct ClientInfo : ClientDesc, ThreadSafeReferenceCounted { std::atomic_bool initialized; std::vector> threadCompletionHooks; - Mutex initializationMutex; - ClientInfo() : ClientDesc(std::string(), false), protocolVersion(0), api(nullptr), failed(true), initialized(false) {} ClientInfo(IClientApi* api)