search

brainlag / JavaNSQClient

Fast Java client for NSQ
MIT License
90 stars 57 forks source link

NSQConsumer - cleaner shutdown #33

Closed esiqveland closed 7 years ago

esiqveland commented 7 years ago

NSQConsumer: shutdown message processing on close

Executor does not have a shutdown method, only ExecutorService.

The close() method now does the following:

  1. Shutdown executor first so no new message processing can start.
  2. Shutdown scheduler
  3. Tell NSQD we are closing down so stop sending messages.

Not 100% sure this is the desired order in a shutdown. Any suggestions would be nice.

A caveat here is that new tasks submitted to executor will be rejected after executor is shutdown. Maybe this is not desirable. At least it will give the opportunity to specifically reject messages queued after shutdown has been started. It also gives us the chance to cleanly finish already processing messages (with a timeout).

brainlag commented 7 years ago

Imho you need to manage the Executor yourself anyway in any real world application. Currently the consumer provides a default Executor but I think this is the real problem here. This is great for the tests but pretty much fails in any real world application. I'm not sure want to change that now.

Stopping the Executor first is the way to go because then backoff will kick in and stops the message flow. Maybe we should provide a method stop the message flow manually.

Shutting down the Executor inside the Consumer will not work because you could share a Executer between several Consumers.

esiqveland commented 7 years ago

Why would you manage the executor separately?

I am leaning towards requiring an Executor in the NSQConsumer constructor, or initialize with a fixedThreadPool(1). I think this would put the library more in line with the Go client that by default runs one goroutine per handler.

What do you think about this? @brainlag