You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/11/08 19:49:51 UTC

[GitHub] [pulsar] tomas-c opened a new issue #8484: ackTimeoutMillis is set to 30000 when ConsumerBuilder.deadLetterPolicy is called

tomas-c opened a new issue #8484:
URL: https://github.com/apache/pulsar/issues/8484


   Hey everyone,
   
   I'm a bit confused about the usage of Dead Letter Queue with NACKs.
   
   Initially I had a shared consumer like so (apologies for my Kotlin) and was using NACKs:
   
   ```
           val jobConsumer = pulsarClient.newConsumer(JSONSchema.of(jobDefinition))
                   .topic("<removed>")
                   .subscriptionName("<removed>")
                   .subscriptionType(SubscriptionType.Shared)
                   .receiverQueueSize(0)
                   .subscribeAsync()
                   .await()
   ```
   
   Later I decided it would be good to make sure failed messages don't get redelivered too many times so I added a dead letter policy:
   
   
   ```
           val jobConsumer = pulsarClient.newConsumer(JSONSchema.of(jobDefinition))
                   .topic("<removed>")
                   .subscriptionName("<removed>")
                   .subscriptionType(SubscriptionType.Shared)
                   .receiverQueueSize(0)
                   .deadLetterPolicy(DeadLetterPolicy.builder()
                           .maxRedeliverCount(2)
                           .build())
                   .subscribeAsync()
                   .await()
   ```
   
   #### Expected behavior
   
   I expected this to start forwarding messages after 2 deliveries to the DLQ and nothing else to change.
   
   #### Actual behavior
   
   What happened was that the ackTimeoutMillis got set to 30s behind the scenes (this is documented behavior but I didn't expect it...) and then all my messages started timing out because each was originally taking more than 30s to process.
   
   I see this default setting was introduced by #3014
   
   #### System configuration
   **Pulsar version**: 2.6.1
   
   
   Is it safe to override the ackTimeoutMillis timeout that was set by the deadLetterPolicy() call and just set it to 0 again? Is there any reason why DLQ would not work when ack timeouts are disabled and NACKs are used instead?
   
   Thanks!
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui edited a comment on issue #8484: ackTimeoutMillis is set to 30000 when ConsumerBuilder.deadLetterPolicy is called

Posted by GitBox <gi...@apache.org>.
codelipenghui edited a comment on issue #8484:
URL: https://github.com/apache/pulsar/issues/8484#issuecomment-726057120


   > I was just surprised by deadLetterPolicy() auto-setting ackTImeoutMillis to 30s and breaking my time-consuming message processing. And also confused by changes in #3014 implying that ackTImeoutMillis was necessary (but I now see that NACKs were implemented after the DLQ feature so makes sense why it was the default at that time).
   
   Oh, I see. The #3014 is merged before the NACK support. I also have no idea how to avoid incompatibility. Maybe we can add more documents for the dead letter policy to describe the default 30s ack timeout will remove from the subsequent release.
   
   I think the 30s ack timeout that the dead letter topic introduced overwrite the ack timeout of the consumer. So it's better to add a flag to indicate the ack timeout is set by user, the dead letter policy should not overwrite it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on issue #8484: ackTimeoutMillis is set to 30000 when ConsumerBuilder.deadLetterPolicy is called

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #8484:
URL: https://github.com/apache/pulsar/issues/8484#issuecomment-726057120


   > I was just surprised by deadLetterPolicy() auto-setting ackTImeoutMillis to 30s and breaking my time-consuming message processing. And also confused by changes in #3014 implying that ackTImeoutMillis was necessary (but I now see that NACKs were implemented after the DLQ feature so makes sense why it was the default at that time).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] tomas-c commented on issue #8484: ackTimeoutMillis is set to 30000 when ConsumerBuilder.deadLetterPolicy is called

Posted by GitBox <gi...@apache.org>.
tomas-c commented on issue #8484:
URL: https://github.com/apache/pulsar/issues/8484#issuecomment-725399618


   Hey @codelipenghui ! 
   
   If it's something small, I could have a try but just to make it clear - I haven't observed DLQ not working with NACKs. 
   
   I was just surprised by deadLetterPolicy() auto-setting ackTImeoutMillis to 30s and breaking my time-consuming message processing. And also confused by changes in #3014 implying that ackTImeoutMillis was necessary (but I now see that NACKs were implemented after the DLQ feature so makes sense why it was the default at that time).
   
   If ackTimeoutMillis is no longer required for DLQ to work, would it make sense to change this default? Or would people be worried about this being a backwards-incompatible change as it would disable ack timeout for people who (possibly accidentally) enabled it by enabling the DLQ?
   
   Besides my confusion, there's another possible one that changing this default would prevent:
   
   ```
   consumerBuilder
       .ackTimeout(0, TimeUnit.MILLISECONDS)
       .deadLetterPolicy(DeadLetterPolicy.builder().build())
       .subscribe()
   ```
   
   results in ack timeout set to 30s but
   
   ```
   consumerBuilder
       .deadLetterPolicy(DeadLetterPolicy.builder().build())
       .ackTimeout(0, TimeUnit.MILLISECONDS)
       .subscribe()
   ```
   
   results in ack timeout disabled.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on issue #8484: ackTimeoutMillis is set to 30000 when ConsumerBuilder.deadLetterPolicy is called

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #8484:
URL: https://github.com/apache/pulsar/issues/8484#issuecomment-725380305


   @tomas-c The DLQ should work with NACKs, if it does not work, it should be fixed. Are you interested in fix it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org