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

Feature / Interceptor for negative ack redelivery #3962

Merged

Conversation

lovelle
Copy link
Contributor

@lovelle lovelle commented Apr 2, 2019

Motivation

In some scenarios is it helpful to be able to set interceptor for redeliveries
being happening due to negative acknowledge.

Modifications

  • Add onNegativeAcksSend() method in ConsumerInterceptor interface.
  • Add handler for onNegativeAcksSend() interceptor in ConsumerBase.
  • Favor forEach on ConsumerInterceptor instead of classic for loop by index.
  • Add call method to onNegativeAcksSend() from NegativeAcksTracker.

Verifying this change

  • Make sure that the change passes the CI checks.

@sijie sijie added area/client type/feature The PR added a new feature or issue requested a new feature labels Apr 2, 2019
@sijie sijie added this to the 2.4.0 milestone Apr 2, 2019
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up. I really didn't think about interceptors when adding the negative acks stuff!

@sijie
Copy link
Member

sijie commented Apr 8, 2019

@merlimat can you check @lovelle 's latest comment?

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over all LGTM except the method name.

@lovelle lovelle force-pushed the feature/add_negative_ack_redelivery_interceptor branch from c184be5 to 7a8a532 Compare April 19, 2019 15:01
@lovelle
Copy link
Contributor Author

lovelle commented Apr 19, 2019

run integration tests

lovelle added 2 commits April 19, 2019 17:04
*Motivation*

In some scenarios is it helpful to be able to set interceptor for redeliveries
being happening due to negative acknowledge.

*Modifications*

  - Add onNegativeAcksSend() method in ConsumerInterceptor interface.
  - Add handler for onNegativeAcksSend() interceptor in ConsumerBase.
  - Favor forEach on ConsumerInterceptor instead of classic for loop by index.
  - Optimization for each by index to avoid compute size() every iteration.
  - Add call method to onNegativeAckRedelivery() from NegativeAcksTracker.
@lovelle lovelle force-pushed the feature/add_negative_ack_redelivery_interceptor branch from 7a8a532 to e140df9 Compare April 19, 2019 20:05
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@lovelle
Copy link
Contributor Author

lovelle commented Apr 20, 2019

run java8 tests

1 similar comment
@lovelle
Copy link
Contributor Author

lovelle commented Apr 20, 2019

run java8 tests

@merlimat merlimat merged commit 7890750 into apache:master Apr 22, 2019
@lovelle lovelle deleted the feature/add_negative_ack_redelivery_interceptor branch April 23, 2019 12:36
RobertIndie added a commit to apache/pulsar-client-cpp that referenced this pull request Mar 21, 2023
### Motivation

This PR is the C++ implementation of apache/pulsar#3962

### Modifications

* Add `onNegativeAcksSend` to the consumer interceptor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants