[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17957: [fix][broker]unify time unit at dropping the backlog on a topic

[GitBox <gi...@apache.org>]

codelipenghui commented on code in PR #17957:
URL: https://github.com/apache/pulsar/pull/17957#discussion_r990888430


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BacklogQuotaManager.java:
##########
@@ -220,7 +220,7 @@ private void dropBacklogForTimeLimit(PersistentTopic persistentTopic, BacklogQuo
                     }
                     // Timestamp only > 0 if ledger has been closed
                     if (ledgerInfo.getTimestamp() > 0
-                            && currentMillis - ledgerInfo.getTimestamp() > quota.getLimitTime()) {
+                            && currentMillis - ledgerInfo.getTimestamp() > quota.getLimitTime() * 1000) {

Review Comment:
   It's a confusing name :)
   
   We should deprecate the old name `limitTime` and introduce a new one `limitTimeInSec`
   Not for this PR, we can start a new discussion in the dev mailing list and make it only happen on the next major release
   



-- 
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