fdb-record-layer icon indicating copy to clipboard operation
fdb-record-layer copied to clipboard

Potential bug on enable Multi-Threaded FDB client in FDBDatabaseFactory

Open mengranwo opened this issue 3 years ago • 1 comments

FDB version 6.3 has introduced this new feature called multi-threaded client and seems like the record layer has added support for it. link However, when I was reading through the doc, look like in order to enable the multi-threaded client feature, you must configure at most one external lib.

Warning

In order to use the multi-threaded client feature, you must configure at least one external client. See :ref:`multi-version client API <multi-version-client-api>` for how to configure an external client.

And I have clarified this with the FDB community

Q: Why do I need to configure an external client if both FDB cluster (server-side) and FDB client lib are on the same 6.3 version? I thought multi-version client is mainly used for communicating with FDB clusters that on a different version.
A: It’s now also used for the multi-threaded client feature

The example provided in FDB repo: https://github.com/apple/foundationdb/pull/4339/files#diff-d2d1739d41f9d18cb4b774fb351781cc7c4d3af07ae028e1a2895b69441880f4R72

I guess we will need to configure at least one external client in the code when setting the thread number for FDB client

mengranwo avatar Oct 27 '21 00:10 mengranwo

Yeah, I believe that that's correct. Barring changes from the FDB core side to remove this requirement, the best we can do might be to update the comments on setThreadsPerClientVersion to warn people about the necessary configuration: https://github.com/FoundationDB/fdb-record-layer/blob/233a72a51182a1f35b9bad34ac29b127a437a12b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBDatabaseFactoryImpl.java#L234

I'm adding the documentation label, as I think that's the action item here (at least on this project), but if you think there's some other way the project could handle this, we're open to alternatives

alecgrieser avatar Nov 09 '21 19:11 alecgrieser