Address skipped PR comments from multithreaded client PR
This commit is contained in:
parent
ab1c51618a
commit
5324f127c9
|
@ -132,7 +132,7 @@ If you suspect that a client process's workload may be saturating the network th
|
|||
Multi-threaded Client
|
||||
=====================
|
||||
|
||||
FoundationDB client library can start multiple worker threads for each version of client that is loaded. Every single cluster will be serviced by a one client thread. If the client is connected to only one cluster, exactly one thread would be active and the rest will remain idle. Hence, using this feature is useful when the client is actively using more than one cluster.
|
||||
FoundationDB client library can start multiple worker threads for each version of client that is loaded. Every single cluster will be serviced by one of the client threads. If the client is connected to only one cluster, exactly one thread would be active and the rest will remain idle. Hence, using this feature is useful when the client is actively using more than one cluster.
|
||||
|
||||
Clients can be configured to use worker-threads by setting the ``FDBNetworkOptions::CLIENT_THREADS_PER_VERSION`` option.
|
||||
|
||||
|
|
|
@ -293,9 +293,7 @@ void DLApi::init() {
|
|||
if (unlinkOnLoad) {
|
||||
int err = unlink(fdbCPath.c_str());
|
||||
if (err) {
|
||||
TraceEvent(SevError, "ErrorUnlinkingTempClientLibraryFile")
|
||||
.detail("errno", errno)
|
||||
.detail("LibraryPath", fdbCPath);
|
||||
TraceEvent(SevError, "ErrorUnlinkingTempClientLibraryFile").GetLastError().detail("LibraryPath", fdbCPath);
|
||||
throw platform_error();
|
||||
}
|
||||
}
|
||||
|
@ -1015,14 +1013,20 @@ void MultiVersionApi::setCallbacksOnExternalThreads() {
|
|||
}
|
||||
void MultiVersionApi::addExternalLibrary(std::string path) {
|
||||
std::string filename = basename(path);
|
||||
// we need at least one external library thread to run this library.
|
||||
threadCount = std::max(threadCount, 1);
|
||||
|
||||
if (filename.empty() || !fileExists(path)) {
|
||||
TraceEvent("ExternalClientNotFound").detail("LibraryPath", filename);
|
||||
throw file_not_found();
|
||||
}
|
||||
|
||||
MutexHolder holder(lock);
|
||||
if (networkStartSetup) {
|
||||
throw invalid_option(); // SOMEDAY: it might be good to allow clients to be added after the network is setup
|
||||
}
|
||||
|
||||
// external libraries always run on their own thread; ensure we allocate at least one thread to run this library.
|
||||
threadCount = std::max(threadCount, 1);
|
||||
|
||||
if (externalClientDescriptions.count(filename) == 0) {
|
||||
TraceEvent("AddingExternalClient").detail("LibraryPath", filename);
|
||||
externalClientDescriptions.emplace(std::make_pair(filename, ClientDesc(path, true)));
|
||||
|
@ -1032,7 +1036,13 @@ void MultiVersionApi::addExternalLibrary(std::string path) {
|
|||
void MultiVersionApi::addExternalLibraryDirectory(std::string path) {
|
||||
TraceEvent("AddingExternalClientDirectory").detail("Directory", path);
|
||||
std::vector<std::string> files = platform::listFiles(path, DYNAMIC_LIB_EXT);
|
||||
// we need at least one external library thread to run these libraries.
|
||||
|
||||
MutexHolder holder(lock);
|
||||
if (networkStartSetup) {
|
||||
throw invalid_option(); // SOMEDAY: it might be good to allow clients to be added after the network is setup
|
||||
}
|
||||
|
||||
// external libraries always run on their own thread; ensure we allocate at least one thread to run this library.
|
||||
threadCount = std::max(threadCount, 1);
|
||||
|
||||
for(auto filename : files) {
|
||||
|
@ -1074,13 +1084,15 @@ std::vector<std::pair<std::string, bool>> MultiVersionApi::copyExternalLibraryPe
|
|||
break;
|
||||
}
|
||||
if (readCount == -1) {
|
||||
throw platform_error;
|
||||
TraceEvent("ExternalClientCopyFailedReadError").GetLastError().detail("LibraryPath", path);
|
||||
throw platform_error();
|
||||
}
|
||||
ssize_t written = 0;
|
||||
while (written != readCount) {
|
||||
ssize_t writeCount = write(tempFd, buf + written, readCount - written);
|
||||
if (writeCount == -1) {
|
||||
throw platform_error;
|
||||
TraceEvent("ExternalClientCopyFailedWriteError").GetLastError().detail("LibraryPath", path);
|
||||
throw platform_error();
|
||||
}
|
||||
written += writeCount;
|
||||
}
|
||||
|
@ -1097,7 +1109,8 @@ std::vector<std::pair<std::string, bool>> MultiVersionApi::copyExternalLibraryPe
|
|||
#else
|
||||
std::vector<std::pair< std::string, bool> > MultiVersionApi::copyExternalLibraryPerThread(std::string path) {
|
||||
if (threadCount > 1) {
|
||||
throw platform_error(); // not supported
|
||||
TraceEvent(SevError, "MultipleClientThreadsUnsupportedOnWindows");
|
||||
throw unsupported_operation();
|
||||
}
|
||||
std::vector<std::pair<std::string, bool>> paths;
|
||||
paths.push_back({ path , false });
|
||||
|
@ -1356,7 +1369,7 @@ Reference<IDatabase> MultiVersionApi::createDatabase(const char *clusterFilePath
|
|||
}
|
||||
std::string clusterFile(clusterFilePath);
|
||||
|
||||
if (threadCount > 1) {
|
||||
if (threadCount > 1 || localClientDisabled) {
|
||||
ASSERT(localClientDisabled);
|
||||
ASSERT(!bypassMultiClientApi);
|
||||
|
||||
|
|
|
@ -110,7 +110,7 @@ description is not currently required but encouraged.
|
|||
<Option name="disable_local_client" code="64"
|
||||
description="Prevents connections through the local client, allowing only connections through externally loaded client libraries." />
|
||||
<Option name="client_threads_per_version" code="65"
|
||||
paramType="Int" paramDescription="Number of client threads to be spawned. Each server will be serviced by a single client thread."
|
||||
paramType="Int" paramDescription="Number of client threads to be spawned. Each cluster will be serviced by a single client thread."
|
||||
description="Spawns multiple worker threads for each version of the client that is loaded. Setting this to a number greater than one implies disable_local_client." />
|
||||
<Option name="disable_client_statistics_logging" code="70"
|
||||
description="Disables logging of client statistics, such as sampled transaction activity." />
|
||||
|
|
Loading…
Reference in New Issue