danieljoos / libkafka-asio

C++ Kafka Client Library using Boost Asio
MIT License
76 stars 40 forks source link

Feature connection queue: added mutex #9

Closed DEvil0000 closed 9 years ago

danieljoos commented 9 years ago

Looking at the code of csi-kafka the better approach seems to be using the io_service::post instead of an exclusive lock. It already provides thread safety. Here's a link to the code: https://github.com/bitbouncer/csi-kafka/blob/master/csi_kafka/lowlevel_client.cpp#L437

DEvil0000 commented 9 years ago

I am new to asio so I might be totally wrong but as far as I understand http://www.boost.org/doc/libs/1_58_0/doc/html/boost_asio/tutorial/tuttimer5.html post is not thread save. It is save as long as you only use one thread per io_service. To make it save again you may use strand or a mutex. And I think a mutex is faster then strand in this case. Do I misunderstand asio here? I was thinking about more then one thread per io_service because you can give it a service. Do you mean post with strand?

skarlsson commented 9 years ago

csi-kafka assumes one thread handling the sockets, That said, I normally use busy-wait for locking VERY short operations that themselves cannot block or do callbacks.I believe this is MUCH faster than using a os mutex. The locking I need is restricted to when other threads calls into the objects handled by the single thread.

typical usecases: 1) main thread does a blocking call. 2) other threads calls the kafka lib

https://github.com/bitbouncer/csi-kafka/blob/master/csi_kafka/internal/spinlock.h

When you thinking about several threads handling the kafka libs - give the ordering of messages a thought - I thing the java implementation is a bit messed up here - at least they used to loose the ordering of messages when retransmitting data. In some use cases this is very bad...

2015-08-03 12:11 GMT+02:00 A. Binzxxxxxx notifications@github.com:

I am new to asio so I might be totally wrong but as far as I understand http://www.boost.org/doc/libs/1_58_0/doc/html/boost_asio/tutorial/tuttimer5.html post is not thread save. It is save as long as you only use one thread per io_service. To make it save again you may use strand or a mutex. And I think a mutex is faster then strand in this case. Do I misunderstand asio here? I was thinking about more then one thread per io_service because you can give it a service.

— Reply to this email directly or view it on GitHub https://github.com/danieljoos/libkafka-asio/pull/9#issuecomment-127183837 .

DEvil0000 commented 9 years ago

spinlocks/busy-wait might be faster (depending on a lot other things) - but they waste a lot of CPU cycles which is not acceptable in multi process systems (at least in my eyes). Instead of writing a lot I link here: http://stackoverflow.com/questions/5869825/when-should-one-use-a-spinlock-instead-of-mutex

When you thinking about several threads handling the kafka libs - give the ordering of messages a thought

I think this is a retry issue. If one fails we just need to retry this and if it fails again we need to cancel/stop all other requests in the queue.

skarlsson commented 9 years ago

Thats a good discussion and I fully agree, but for the case where you only wait on pushing or getting to/from a non thread-safe queue a spinlock outperforms by far a (os)mutex.

If we go down this path then you will notice that a spinlock will be "volatile" which trashes the cache on multicore / or multiprocessor machines. This is why single threaded apps can have a nice performance boost.As with everything - it depends on your usecase.

In recent years I've built quite a few things based on single single threaded, non-blocking (using boost asio). If io bound- they perform great. Lately I've started using mesos to run a lot of those single threaded things on a cluster. Also a great match.

finally I should change my implementation to http://www.boost.org/doc/libs/1_58_0/doc/html/atomic.html

my two cent's: use a deque containing a shared_ptr object and protect the deque with a boost::atomic

2015-08-03 16:06 GMT+02:00 A. Binzxxxxxx notifications@github.com:

spinlocks/busy-wait might be faster (depending on a lot other things) - but they waste a lot of CPU cycles which is not acceptable in multi process systems (at least in my eyes). Instead of writing a lot I link here: http://stackoverflow.com/questions/5869825/when-should-one-use-a-spinlock-instead-of-mutex

When you thinking about several threads handling the kafka libs - give the ordering of messages a thought

I think this is a retry issue. If one fails we just need to retry this and if it fails again we need to cancel/stop all other requests in the queue.

— Reply to this email directly or view it on GitHub https://github.com/danieljoos/libkafka-asio/pull/9#issuecomment-127252949 .

skarlsson commented 9 years ago

I think this is a retry issue

agreed, but give the matter some thought if you allow several writes at the same time (using different sockets - I suppose, if not I'm not sure you benefit from several threads at all....)

2015-08-03 17:19 GMT+02:00 Svante Karlsson svante.karlsson@csi.se:

Thats a good discussion and I fully agree, but for the case where you only wait on pushing or getting to/from a non thread-safe queue a spinlock outperforms by far a (os)mutex.

If we go down this path then you will notice that a spinlock will be "volatile" which trashes the cache on multicore / or multiprocessor machines. This is why single threaded apps can have a nice performance boost.As with everything - it depends on your usecase.

In recent years I've built quite a few things based on single single threaded, non-blocking (using boost asio). If io bound- they perform great. Lately I've started using mesos to run a lot of those single threaded things on a cluster. Also a great match.

finally I should change my implementation to http://www.boost.org/doc/libs/1_58_0/doc/html/atomic.html

my two cent's: use a deque containing a shared_ptr object and protect the deque with a boost::atomic

2015-08-03 16:06 GMT+02:00 A. Binzxxxxxx notifications@github.com:

spinlocks/busy-wait might be faster (depending on a lot other things) - but they waste a lot of CPU cycles which is not acceptable in multi process systems (at least in my eyes). Instead of writing a lot I link here: http://stackoverflow.com/questions/5869825/when-should-one-use-a-spinlock-instead-of-mutex

When you thinking about several threads handling the kafka libs - give the ordering of messages a thought

I think this is a retry issue. If one fails we just need to retry this and if it fails again we need to cancel/stop all other requests in the queue.

— Reply to this email directly or view it on GitHub https://github.com/danieljoos/libkafka-asio/pull/9#issuecomment-127252949 .

DEvil0000 commented 9 years ago

My feeling is you would even benefit from more threads when using one socket. (may hardly depend on your network speeds and CPU speeds) As long as you are limited to the speed of your network connection it does not make much sense to use more threads. But if you run the software on a system with a very fast network and/or slow CPU it should make a difference. Just imagine you could do this in different threads:

skarlsson commented 9 years ago

serialize and deserialize can be solved by having the kafka lib consuming and returning unserialized (possibly shared_ptr) buffers that you consume outside the callbacks. (asio::post from callbacks)

io wait should be low using async operations.

To be honest I've not used slow cpu's with performance sensitive applications in a while.