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 2022/03/01 07:32:25 UTC
[GitHub] [pulsar] Technoboy- opened a new pull request #14501: Add log to track negtive unacked msg.
Technoboy- opened a new pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501
### Motivation
Now we merged https://github.com/apache/pulsar/pull/14288, but https://github.com/apache/pulsar/pull/14246 has removed the log. So we add back to help find if there are other bugs.
### Documentation
- [x] `no-need-doc`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a change in pull request #14501: Add log to track negtive unacked msg.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501#discussion_r816519346
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
##########
@@ -947,6 +949,10 @@ private int addAndGetUnAckedMsgs(Consumer consumer, int ackedMessages) {
subscription.addUnAckedMessages(ackedMessages);
unackedMsgs = UNACKED_MESSAGES_UPDATER.addAndGet(consumer, ackedMessages);
}
+ if (unackedMsgs < 0 && ++negtiveUnackedMsgsCount % 10000 == 0) {
Review comment:
Would it be better to use the current system time to limit the number of log prints than an incremental variable?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #14501: Add log to track negtive unacked msg.
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501#discussion_r817026915
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
##########
@@ -947,6 +949,10 @@ private int addAndGetUnAckedMsgs(Consumer consumer, int ackedMessages) {
subscription.addUnAckedMessages(ackedMessages);
unackedMsgs = UNACKED_MESSAGES_UPDATER.addAndGet(consumer, ackedMessages);
}
+ if (unackedMsgs < 0 && System.currentTimeMillis() - negtiveUnackedMsgsTimestamp >= 10_000) {
Review comment:
@Technoboy- can you please provide motivation for this new logic in your PR description? This was not in the removed conditional here: https://github.com/apache/pulsar/pull/14246/files
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #14501: Add log to track negtive unacked msg.
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #14501: Add log to track negtive unacked msg.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501#issuecomment-1060227244
Hi @michaeljmarshall @lhotari @merlimat @codelipenghui Could you help review this ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a change in pull request #14501: Add log to track negtive unacked msg.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #14501:
URL: https://github.com/apache/pulsar/pull/14501#discussion_r817292622
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
##########
@@ -947,6 +949,10 @@ private int addAndGetUnAckedMsgs(Consumer consumer, int ackedMessages) {
subscription.addUnAckedMessages(ackedMessages);
unackedMsgs = UNACKED_MESSAGES_UPDATER.addAndGet(consumer, ackedMessages);
}
+ if (unackedMsgs < 0 && System.currentTimeMillis() - negtiveUnackedMsgsTimestamp >= 10_000) {
Review comment:
yes, in #14246, if there is a bug, the unackedMsgs is negative, there will print a lot of logs, this may confuse users.
So it's better to print at a period of 10s.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org