docker / go-events

Composable event distribution for Go
Apache License 2.0
131 stars 22 forks source link

queue: Close destination before waiting #14

Closed aaronlehmann closed 7 years ago

aaronlehmann commented 8 years ago

The current code tries to flush the queue first and then waits on the condition variable. This can get stuck if there is a write in progress. Instead, close the destination first, and then wait afterwards.

aaronlehmann commented 8 years ago

This change means that a Close without all the events in the queue delivered won't wait for those events to be delivered. It's causing a test to fail.

Is this behavioral change okay? In swarmkit, it's what we want. We have cases where we shut down a broadcaster before consuming all the events. It's not always straightforward to continue consuming events during this shutdown process.

stevvooe commented 8 years ago

@aaronlehmann We may have run into this bug before. Are you sure the sink is being shutdown in the right order?

aaronlehmann commented 8 years ago

Not sure what I should be doing differently. I am simply calling Close on the broadcaster, but it looks like the broadcaster code closes the sinks as part of that, and that's what's hanging. It seems like the problem is that a Queue can't be closed while it's writing without the change in this PR.

stevvooe commented 8 years ago

@aaronlehmann Could you please show me what the actual problem is before we make a bunch of changes here?