Revert "Use a separate gorutine to handle the logic of reconnect"

[wolfstudy]

Reverts #691

[bschofield]

Just to be clear, this commit doesn't actually revert #691 but instead adds a close channel, right? If so it might be good to update the title.

I do have a separate concern with this approach, which might or might not be justified. It looks to me like operations which were previously serialized by virtue of the single goroutine in runEventsLoop() can now occur concurrently, because they are now running in separate goroutines.

For example: suppose the user calls p.Flush() on the producer, which causes a &flushRequest{} to be enqueued to p.eventsChan. The primary goroutine in runEventsLoop() will then call p.internalFlush() which will try to write to p.cnx. Suppose that at the same time, the broker closes the connection. and the new goroutine calls p.reconnectToBroker(), which tries to change p.cnx whilst p.internalFlush() is still using it.

Does it not matter that p.reconnectToBroker() can now be happening at the same time as p.internalSend() / p.internalFlush() / p.internalClose() / p.internalFlushCurrentBatch(), when previously the use of a single goroutine meant that these functions could not be executing simultaneously? Is there some other lock or feature of the code that means this is OK? Or does this introduce a race condition?

Also, does it matter that (because of goroutine scheduling) the order in which these operations execute can now be different to before?

Would really appreciate your thoughts @wolfstudy / @cckellogg. Apologies in advance if you already considered this possibility and there is no issue.

[bschofield]

If the operations occurring simultaneously could be an issue then it would be easy to add a lock to prevent this.

However I don't think that would prevent the order of operations from being changed from the previous behaviour?