Best-effort drain before shutdown #25
Conversation
| final T notification = this.pushManager.getQueue().poll(timeout, timeUnit); | ||
|
|
||
| final T notification; | ||
| synchronized(pushManager.getQueue()) { |
There was a problem hiding this comment.
I'm confused; why do you think we need to synchronize on a BlockingQueue?
There was a problem hiding this comment.
to do a notify() / wait() over an object, synchronized() is needed.
There was a problem hiding this comment.
Well, right ;) I was more just wondering why we're doing a wait/notify at all here.
I think I get it now, though, but I'm not totally sure I agree with the timing here. What if the notifyAll happened in the completion listener for the ChannelFuture when we actually write the notification instead?
There was a problem hiding this comment.
I've moved it just at the top of the operationComplete callback. It makes more sense, you are right.
|
Any chance of this being merged? or, at least, #20 ? |
|
@flozano Just wrapping up the v0.2 release now and will be starting in on v0.3 features soon. |
Allow to drain the queue and wait until it's empty before shutdown is actually performed. It's a best-effort - shutdown still can return failed notifications, but less likely than with plain shutdown.