Usability: Decouple API from thread model
48656c6c6f opened this issue · 8 comments
The current architecture forces a specific thread model that does not suite all situations. I would like to see threads moved out of the library, allowing the application to decide on the thread model to utilize.
An alternative could be to support two model, one with threads and one without. But, I think offering the async thread model as a separate library that builds on top of this one would be best. I know there is already SyncClient, but its composed of the async client, meaning the thread pool is still created and used.
For some ops, e.g., set, get, it is quite doable to have a implementation without creating new thread as the SyncClient, for some ops, e.g., lock, we will need to maintain a lease and keepalive it periodly in the background to implements the correct semantic.
I have proposed another appoarch in #12, where a seperate thread pool will be associated to a client, and the thread pool will be destroyed once the client got destructed. How do you think about such solution ?
BTW may I know the background about why you need to avoiding creating thread?
My use case is a distributed system. Depending on the platform, there may be limited threads while most are needed for other purposes. Beyond that, forcing a particular threading model reduces usability across all use cases. The API could be refactored to expect functions to be called within threads created by the application.
True downsides of the present approach include assumptions around signal handling, size of thread pool, and number of dependencies. Are the threads created using the parents signal mask or with all blocked? How might that affect an application? Size of thread pool relative to the CPU and competing priorities?
Do one thing really well and provide an interface for others to do an additional thing on top. In other words, there’s no reason the thread model need be tied to the implementation of the client.
Re #12, a compromise would be to allow the consumer of the API to provide an implementation of a thread pool interface, thereby allowing the application to control thread creation. I still feel removal of threading from the library entirely is the better approach.
Agree with the points above.
I will try to revise the implementation the SyncClient, and makes the Client a wrapper around the SyncClient. Some operations, e.g., lock, watch, keepalive, will be hard to use under the sync client, as it requires the user to maintain more contexts and the library will rely on those contexts. But with Client (the async interface), those ops will be more friendly.
I have try to split a build target etcd-cpp-apiv3-core to contains the sync part to avoid requires the boost asio and initializing the internal thread pool inside cpprestsdk, even it doesn't require that.
Thanks for the insights, your driven cases has convinced me.
BTW supporting the users to "provide a thread pool interface" won't happen soon, IMO, as we requires cpprestsdk to work, it will initialize its own boost thread pool, and I haven't find a good alternative for the future/promise feature in the cpprestsdk.
What I'm saying is that cpprestsdk should be removed from the library and left to the consumer to decide whether to use it. That said, there are sufficient differences in approach that I will need to move forward with a separate implementation.
What I'm saying is that cpprestsdk should be removed from the library and left to the consumer to decide whether to use it. That said, there are sufficient differences in approach that I will need to move forward with a separate implementation.
Yes. In my previous comment, where the cpprestsdk will be removed from the etcd-cpp-apiv3-core, where I think only grpc and protobuf will be included.
And an extra wrapper etcd-cpp-apiv3 will includes the async client and depends on cpprestsdk.
Here etcd-cpp-apiv3-core and etcd-cpp-apiv3 are referred to CMake build targets.
What I'm saying is that cpprestsdk should be removed from the library and left to the consumer to decide whether to use it. That said, there are sufficient differences in approach that I will need to move forward with a separate implementation.
Yes. In my previous comment, where the
cpprestsdkwill be removed from theetcd-cpp-apiv3-core, where I think only grpc and protobuf will be included.And an extra wrapper
etcd-cpp-apiv3will includes the async client and depends on cpprestsdk.Here
etcd-cpp-apiv3-coreandetcd-cpp-apiv3are referred to CMake build targets.
The decouple of API and thread model is implemented and documented in #130