[improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ #23133

jcass8695 · 2024-08-07T07:35:21Z

Motivation

If a consumer consumes a message and fails during the processing of that message in a terminal way, we as the client in
certain scenarios may not want to retry that message later, we may want to send it directly to the DLQ. Currently, if
the client wanted a persistent record of the message, they would have to do one of:

NACK the message and retry it up to the configured max retries, when it would then land in the DLQ.
ACK the message and explicity produce it to the DLQ.
ACK the message and persist it in some other system, e.g. a database table, a log file.

I propose adding an interface that allows a consumer to terminally NACK a message, sending it directly to the DLQ instead of retrying.

Modifications

No modification yet.

Verifying this change

Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Documentation only

Does this pull request potentially affect one of the following parts:

Documentation

doc
doc-required
doc-not-needed
doc-complete

Matching PR in forked repository

PR in forked repository:

github-actions · 2024-08-07T07:35:46Z

@jcass8695 Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

crossoverJie · 2024-08-07T11:29:52Z

@jcass8695 This is a great feature. I have added the relevant methods for Java.

BTW, the PIP number is conflicting with #23061.

crossoverJie · 2024-08-07T11:35:47Z

pip/pip-367.md

+### Metrics
+
+If the client would like to track the number of calls to the `Term` method, we could add a new metric
+`pulsar_client_consumer_terms`. This metric should in theory track exactly with the


pulsar_client_consumer_message_terms
It is recommended to keep the naming conventions consistent.

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java

Lines 429 to 430 in d4bbf10

consumerDlqMessagesCounter = ip.newCounter("pulsar.client.consumer.message.dlq", Unit.Messages,

"The number of messages sent to DLQ", topic, attrs);

They are all defined with underscores in the Go client library.

https://github.com/apache/pulsar-client-go/blob/master/pulsar/internal/metrics.go#L266-L270

The Java client is currently using OpenTelemetry's naming conventions. I suggest aligning it with the existing naming style.

I think that moving the Go client to be OTel compliant is outside the scope of this change. I think I would rather the Go client be consistent with itself. Changing all of the metrics to be OTel compliant can be done in a single non-backwards compatible change.

Yes, I meant that the Java client should follow the current naming conventions. It would be best to also add a description for Java metrics.

jcass8695 · 2024-08-08T12:26:02Z

@jcass8695 This is a great feature. I have added the relevant methods for Java.

BTW, the PIP number is conflicting with #23061.

@crossoverJie I'll change the PIP number.
I sent an email to [email protected] but it seems like there hasn't been any discussion thread created that I can see on the mailing lists forum - does it need to be manually approved or something?
Do you have a link to those methods in the Java library? I will try to make them the same so that there's parity.

crossoverJie · 2024-08-09T07:52:49Z

I sent an email to [email protected] but it seems like there hasn't been any discussion thread created that I can see on the mailing lists forum - does it need to be manually approved or something?

I didn't see the related discussion on the mailing list. Please double-check if the email has been sent.

Do you have a link to those methods in the Java library? I will try to make them the same so that there's parity.

https://github.com/apache/pulsar/blob/master/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Consumer.java

jcass8695 · 2024-08-09T08:33:20Z

I sent an email to [email protected] but it seems like there hasn't been any discussion thread created that I can see on the mailing lists forum - does it need to be manually approved or something?

I didn't see the related discussion on the mailing list. Please double-check if the email has been sent.

Do you have a link to those methods in the Java library? I will try to make them the same so that there's parity.

https://github.com/apache/pulsar/blob/master/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Consumer.java

I've resent my email now, with the updated PIP number.

jcass8695 · 2024-08-11T15:16:07Z

@crossoverJie still no thread on the mailing list 😞

jcass8695 · 2024-08-13T08:14:52Z

@crossoverJie still no thread on the mailing list 😞

Below is the email that I've sent. Am I doing something wrong? This feels like it should be the easy step...

crossoverJie · 2024-08-14T02:27:26Z

@jcass8695 However, I did not see the email you send on the mailing list🤔.

poorbarcode · 2024-08-21T13:31:27Z

pip/pip-372.md

+```
+
+```java
+public interface Consumer<T> {


Please move the definition of the new API into the section Public API

poorbarcode · 2024-08-21T13:32:29Z

pip/pip-372.md

+
+# Links
+
+* Mailing List discussion thread:


Please send an email for a discussion of this PIP

My email is not making it to the mailing lists. Please see above comments.

jcass8695 and others added 2 commits August 6, 2024 15:45

add: first draft of PIP-367

d8d7b64

Merge branch 'apache:master' into add/pip-367

0e01053

github-actions bot added the PIP label Aug 7, 2024

github-actions bot added the doc-label-missing label Aug 7, 2024

jcass8695 changed the title ~~[improve][client] PIP-367: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ~~ [improve][pip] PIP-367: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ Aug 7, 2024

github-actions bot added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-label-missing labels Aug 7, 2024

add: first draft of PIP-367

a0f4775

jcass8695 mentioned this pull request Aug 7, 2024

Ability for a Consumer to bypass retries and DLQ a terminally failing message apache/pulsar-client-go#1167

Open

crossoverJie assigned jcass8695 Aug 7, 2024

Supplement java client

ecb1b1b

crossoverJie reviewed Aug 7, 2024

View reviewed changes

refactor: pip number from 367 to 372 to avoid clash

b909a16

jcass8695 changed the title ~~[improve][pip] PIP-367: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ~~ [improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ Aug 9, 2024

poorbarcode reviewed Aug 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ #23133

[improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ #23133

jcass8695 commented Aug 7, 2024 •

edited

Loading

github-actions bot commented Aug 7, 2024

crossoverJie commented Aug 7, 2024

crossoverJie Aug 7, 2024

jcass8695 Aug 8, 2024 •

edited

Loading

crossoverJie Aug 9, 2024

jcass8695 Aug 9, 2024

crossoverJie Aug 9, 2024

jcass8695 commented Aug 8, 2024 •

edited

Loading

crossoverJie commented Aug 9, 2024

jcass8695 commented Aug 9, 2024

jcass8695 commented Aug 11, 2024

jcass8695 commented Aug 13, 2024

crossoverJie commented Aug 14, 2024

poorbarcode Aug 21, 2024

poorbarcode Aug 21, 2024

jcass8695 Aug 22, 2024

	consumerDlqMessagesCounter = ip.newCounter("pulsar.client.consumer.message.dlq", Unit.Messages,
	"The number of messages sent to DLQ", topic, attrs);

[improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ #23133

Are you sure you want to change the base?

[improve][pip] PIP-372: Allow consumers to terminally NACK a message, bypassing retries and routing directly to the DLQ #23133

Conversation

jcass8695 commented Aug 7, 2024 • edited Loading

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Matching PR in forked repository

github-actions bot commented Aug 7, 2024

crossoverJie commented Aug 7, 2024

crossoverJie Aug 7, 2024

Choose a reason for hiding this comment

jcass8695 Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

crossoverJie Aug 9, 2024

Choose a reason for hiding this comment

jcass8695 Aug 9, 2024

Choose a reason for hiding this comment

crossoverJie Aug 9, 2024

Choose a reason for hiding this comment

jcass8695 commented Aug 8, 2024 • edited Loading

crossoverJie commented Aug 9, 2024

jcass8695 commented Aug 9, 2024

jcass8695 commented Aug 11, 2024

jcass8695 commented Aug 13, 2024

crossoverJie commented Aug 14, 2024

poorbarcode Aug 21, 2024

Choose a reason for hiding this comment

poorbarcode Aug 21, 2024

Choose a reason for hiding this comment

jcass8695 Aug 22, 2024

Choose a reason for hiding this comment

jcass8695 commented Aug 7, 2024 •

edited

Loading

jcass8695 Aug 8, 2024 •

edited

Loading

jcass8695 commented Aug 8, 2024 •

edited

Loading