[PIP-130] Apply redelivery backoff policy for ack timeout #13707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
doc
|
/pulsarbot run-failure-checks |
1 task
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
Show resolved
Hide resolved
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
Anonymitaet
added
the
doc-complete
…-master handle conflict
…_ack_timeout handle conflict
|
/pulsarbot run-failure-checks |
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageRedeliveryTracker.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageRedeliveryTracker.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnackMessageIdWrapper.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ExponentialRedeliveryBackoff.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiplierRedeliveryBackoff.java
Outdated
Show resolved
Hide resolved
...ar-broker/src/test/java/org/apache/pulsar/client/impl/UnAcknowledgedMessagesTimeoutTest.java
Outdated
Show resolved
Hide resolved
...ar-broker/src/test/java/org/apache/pulsar/client/impl/UnAcknowledgedMessagesTimeoutTest.java
Show resolved
Hide resolved
...ar-broker/src/test/java/org/apache/pulsar/client/impl/UnAcknowledgedMessagesTimeoutTest.java
Outdated
Show resolved
Hide resolved
codelipenghui
requested review from
merlimat,
315157973,
hangc0276 and
Jason918
codelipenghui
approved these changes
Jan 25, 2022
Jason918
reviewed
Jan 25, 2022
pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiplierRedeliveryBackoff.java
Outdated
Show resolved
Hide resolved
| this.maxDelayMs = maxDelayMs; | ||
| this.multiplier = multiplier; | ||
| maxMultiplierPow = multiplier == 1 ? -1 : | ||
| (int) (Math.log((double) maxDelayMs / minDelayMs) / Math.log(multiplier)) + 1; |
There was a problem hiding this comment.
Would this "+ 1" here causing the final result overflow?
There was a problem hiding this comment.
No, this parameter is to reduce invalid POW calculations.
Jason918
reviewed
Jan 26, 2022
| super(client, consumerBase, conf); | ||
| this.ackTimeoutRedeliveryBackoff = conf.getAckTimeoutRedeliveryBackoff(); | ||
| this.ackTimeoutMessages = new HashMap<MessageId, Long>(); | ||
| this.redeliveryMessageIdPartitionMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Does this redeliveryMessageIdPartitionMap need to be ConcurrentHashMap ?
It seems to be wrapped with writeLock
There was a problem hiding this comment.
can remove concurrent var.
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageRedeliveryTracker.java
Show resolved
Hide resolved
Jason918
approved these changes
Jan 26, 2022
|
/pulsarbot run-failure-checks |
2 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
Nicklee007
pushed a commit
to Nicklee007/pulsar
that referenced
this pull request
Apr 20, 2022
### Motivation This pull request is the specific implementation plan of PIP-130 ### Modifications *Apply the message redelivery policy for the ack timeout.* *Alert NegativeAckBackoff interface to RedeliveryBackoff.* *Expose AckTimeoutRedeliveryBackoff in ConsumerBulider.*
Open
2 tasks
2 tasks
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Master Issue: #13528
Motivation
This pull request is the specific implementation plan of PIP-130
Modifications
Apply the message redelivery policy for the ack timeout.
Alert NegativeAckBackoff interface to RedeliveryBackoff.
Expose AckTimeoutRedeliveryBackoff in ConsumerBulider.
Add unit test case.
Currently only the Java client is modified.
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
Need to update docs?
doc