search

abrandoned / protobuf-nats

MIT License
8 stars 4 forks source link

updates / thread pool queue / more tests #8

Closed film42 closed 7 years ago

film42 commented 7 years ago

This does the following:

  1. Removes connection pool from project cause we're not using it right now.
  2. Remove the custom thread pool and use a queue of size N where N is the number of threads. It's possible for the thread pool to lag behind, so it depends on this queue for taking messages. With the old pool, we were dropping messages and this caused an issue. I'm still worried that we might pick up work and the client might retry if the pool is full, but this at least gets us closer to ZMQ behavior, where ZMQ uses a frontend queue of size 10. We set the thread pool queue to size 10 but were still seeing instances where 10 was not enough buffer between work coming and and the thread pool pulling off new work, so I made this the same size as current threads. We might end up changing this, but for now this seems like the best working option with concurrent ruby.
  3. Add a test to the client to make sure that messages can be sent in any order (ack, response) and we will still complete successfully.

cc @quixoten @mmmries @abrandoned

film42 commented 7 years ago

Added a new commit that ACKs if the message was enqueued on the server. This is similar to how we process requests with ZMQ. I would like to add some kind of TTL to this, but we may not need it. @quixoten and I have been trying to find a good way to make this work and would your advice. Thanks!

mmmries commented 7 years ago

Looking over this it appears that if we set :threads => 8 that will give us 8 executor threads and allows at most 8 items to be queued up. If the queue is full then we drop the message (no ACK sent back) and the client will get an ack timeout and try again?

If that's correct then I'm :+1: on this PR

film42 commented 7 years ago

@mmmries that is correct. 8 threads creates a max queue of 8, and once the queue is full, it will reject new requests. If we can stick it on the queue, then we will ACK immediately. This is a trade-off, but it seems good enough for now.