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

Producer respects Context passed to Send() and SendAsync() when appying backpressure #534

Merged

Conversation

dferstay
Copy link
Contributor

@dferstay dferstay commented Jun 7, 2021

Previously, the Producer ignored the context passed to Send() and
SendAsync().

Now, the Producer respects the context in the case where the
ProducerOptions.MaxPendingMessages limit is breached. In this case, the
producer will block until a permit for sending the message is available or
until the context expires, whichever comes first.

Failures to send messages due to context expiration are communicated to
callers via the existing TimeoutError error code.

Signed-off-by: Daniel Ferstay dferstay@splunk.com

Motivation

When the Producer is applying backpressure to the caller it will block attempting to acquire a send permit.

Before this change change, the producer would block indefinitely as a send permit must be acquired before the send timeout can be checked by the internal runEventsLoop go-routine.

With this change, callers can control the duration they are willing to wait for a send permit by using the context passed to the Send() and SendAsync() methods.

Modifications

  • Modify Semaphore.Acquire() to accept a context and block until acquisition is complete or the passed context is expired, whichever comes first
  • Modify partitionProducer.internalSendAsync() to propagate the calling context when attempting to acquire the publish semaphore, and fail the request if the acquire is unsuccessful.

Verifying this change

  • additional unit tests for the Sempahore
  • additional producer_test that verifies publish on context expiration while applying backpressure

@dferstay dferstay force-pushed the producer-respects-ctx-under-backpressure branch from 3d97416 to 3444609 Compare June 7, 2021 20:07
@merlimat merlimat added this to the 0.6.0 milestone Jun 7, 2021
@dferstay dferstay force-pushed the producer-respects-ctx-under-backpressure branch 5 times, most recently from b409f8f to 9d50186 Compare June 7, 2021 22:59
@cckellogg
Copy link
Contributor

/pulsarbot run-failure-checks

…ing backpressure

Previously, the Producer ignored the context passed to Send() and
SendAsync().

Now, the Producer respects the context in the case where the
ProducerOptions.MaxPendingMessages limit is reached.  In this case, the
producer will block until a permit for sending the message is available or
until the context expires, whichever comes first.

Failures to send messages due to context expiration are communicated to
callers via the existing TimeoutError error code.

Signed-off-by: Daniel Ferstay <dferstay@splunk.com>
@dferstay dferstay force-pushed the producer-respects-ctx-under-backpressure branch from 9d50186 to 4b2bfbb Compare June 24, 2021 20:42
@cckellogg cckellogg merged commit 0f3e504 into apache:master Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants