Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support producer.SendAsync not blocking if queue is full #208

Closed
addisonj opened this issue Apr 3, 2020 · 2 comments · Fixed by #254
Closed

Support producer.SendAsync not blocking if queue is full #208

addisonj opened this issue Apr 3, 2020 · 2 comments · Fixed by #254

Comments

@addisonj
Copy link
Contributor

addisonj commented Apr 3, 2020

Is your feature request related to a problem? Please describe.
Currently, producer.SendAsync can quickly become a blocking call if either:

  1. The queue (set with maxPendingMessages) becomes full
  2. The event channel (for enqueueing, handling disconnects, etc) becomes full (with a hardcoded 10 messages)

This can be really undesirable as it is unexpected that SendAsync becomes a blocking call.

Describe the solution you'd like
The golang producer needs an option equivalent to the java producerBuilder.blockIfQueueFull call. In this mode, any SendAsync call against a full queue (case 1 above) should result in an error. The semaphore implementation around the maxPendingMessages does not have a timeout or tryAcquire, but if it did, it should be easy to make work (perhaps consider switching to golang/x/sync/semaphore package?) and in the event the semaphore can't be acquired, then pop an error.

It seems to also make sense that if the internal eventChannel is also full, we should also error (case 2 above). For example, this can happen when a producer is disconnected and is waiting for a re-connect. The call to re-connect is blocking and (AFAICT) blocks further messages being processed on the eventChannel, which quickly fills up and then results in a blocking behavior on SendAsync. A non-blocking send into the eventChannel would probably work here, though we may need to increase the eventChannel size a bit

Describe alternatives you've considered
None

@wolfstudy
Copy link
Member

Thanks @addisonj feedback, we designed this deliberately, when the sending speed is too fast, it will be blocked instead of returning an error to the user. Maybe we can change the documentation of sendAsync() to give users a clearer explanation.

@ZongrongLi
Copy link

add a counter yourself and comapre the count with MaxPendingMessages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants