[fix][broker] Fix issue where msgRateExpired may not refresh forever #19759

massakam · 2023-03-09T07:01:05Z

Motivation

Topic stats have a msgRateExpired field. Once this value is greater than 0, it may never return to 0, even though no messages have expired. This is because PersistentMessageExpiryMonitor#updateRates() only runs when PersistentMessageExpiryMonitor#expireMessages() is executed, not periodically.

For example, if the number of messages in the backlog of a subscription is 0, the execution of PersistentMessageExpiryMonitor#expireMessages() will be skipped and msgRateExpired will not be updated.

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java

Lines 1062 to 1071 in cdeef00

    
           public boolean expireMessages(int messageTTLInSeconds) { 
        
               if ((getNumberOfEntriesInBacklog(false) == 0) || (dispatcher != null && dispatcher.isConsumerConnected() 
        
                       && getNumberOfEntriesInBacklog(false) < MINIMUM_BACKLOG_FOR_EXPIRY_CHECK 
        
                       && !topic.isOldestMessageExpired(cursor, messageTTLInSeconds))) { 
        
                   // don't do anything for almost caught-up connected subscriptions 
        
                   return false; 
        
               } 
        
               this.lastExpireTimestamp = System.currentTimeMillis(); 
        
               return expiryMonitor.expireMessages(messageTTLInSeconds); 
        
           }

As a result, the value of msgRateExpired may remain non-zero even though message expiration has not occurred recently.

Modifications

Call PersistentMessageExpiryMonitor#updateRates() in the PersistentTopic#updateRates() method that runs periodically to update the topic stats.

Verifying this change

Make sure that the change passes the CI checks.

Documentation

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

codecov-commenter · 2023-03-09T13:55:49Z

Codecov Report

Merging #19759 (1db15cf) into master (e973388) will increase coverage by 29.95%.
The diff coverage is 29.16%.

@@              Coverage Diff              @@
##             master   #19759       +/-   ##
=============================================
+ Coverage     32.05%   62.01%   +29.95%     
- Complexity     6409    25829    +19420     
=============================================
  Files          1674     1848      +174     
  Lines        126556   135887     +9331     
  Branches      13810    14953     +1143     
=============================================
+ Hits          40570    84264    +43694     
+ Misses        79992    43905    -36087     
- Partials       5994     7718     +1724

Flag	Coverage Δ
inttests	`24.57% <25.00%> (+0.02%)`	⬆️
systests	`25.27% <20.83%> (+<0.01%)`	⬆️
unittests	`59.19% <25.00%> (+41.85%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ker/service/persistent/PersistentSubscription.java	`68.55% <15.78%> (+21.84%)`	⬆️
...sar/broker/service/persistent/PersistentTopic.java	`65.21% <50.00%> (+14.74%)`	⬆️
...roker/service/persistent/PersistentReplicator.java	`39.24% <100.00%> (+6.02%)`	⬆️
...metadata/coordination/impl/LeaderElectionImpl.java	`78.20% <100.00%> (+19.49%)`	⬆️
...e/pulsar/proxy/server/BrokerDiscoveryProvider.java	`76.00% <0.00%> (-8.00%)`	⬇️
...sar/broker/resources/MetadataStoreCacheLoader.java	`73.68% <0.00%> (-5.27%)`	⬇️
...g/apache/pulsar/common/protocol/PulsarHandler.java	`56.89% <0.00%> (-1.73%)`	⬇️
...in/java/org/apache/pulsar/common/api/AuthData.java	`71.42% <0.00%> (ø)`
...ava/org/apache/pulsar/client/api/schema/Field.java	`80.00% <0.00%> (ø)`
...g/apache/pulsar/common/naming/NamespaceBundle.java	`60.71% <0.00%> (ø)`
... and 1205 more

…19759)

…pache#19759) (cherry picked from commit 9a9f005)

Fix issue where msgRateExpired may not refresh forever

1db15cf

massakam added type/bug The PR fixed a bug or issue reported a bug component/stats doc-not-needed Your PR changes do not impact docs ready-to-test labels Mar 9, 2023

massakam added this to the 3.0.0 milestone Mar 9, 2023

massakam self-assigned this Mar 9, 2023

massakam added area/broker and removed component/stats labels Mar 9, 2023

massakam changed the title ~~[fix][stats] Fix issue where msgRateExpired may not refresh forever~~ [fix][broker] Fix issue where msgRateExpired may not refresh forever Mar 9, 2023

poorbarcode approved these changes Mar 9, 2023

View reviewed changes

Technoboy- approved these changes Mar 9, 2023

View reviewed changes

Technoboy- added release/2.11.1 release/2.10.5 labels Mar 9, 2023

Technoboy- merged commit 44993ce into apache:master Mar 12, 2023

Technoboy- pushed a commit that referenced this pull request Mar 12, 2023

[fix][broker] Fix issue where msgRateExpired may not refresh forever (#…

93b83e8

…19759)

Technoboy- added the cherry-picked/branch-2.11 label Mar 12, 2023

massakam deleted the fix-expiration-rate branch March 12, 2023 11:12

Technoboy- pushed a commit that referenced this pull request May 6, 2023

[fix][broker] Fix issue where msgRateExpired may not refresh forever (#…

9a9f005

…19759)

Technoboy- added the cherry-picked/branch-2.10 label May 6, 2023

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 11, 2023

[fix][broker] Fix issue where msgRateExpired may not refresh forever (a…

f9f5356

…pache#19759) (cherry picked from commit 9a9f005)

lhotari mentioned this pull request Jun 20, 2023

Flaky-test: SimpleProducerConsumerStatTest.testMsgRateExpired #20615

Closed

2 tasks

jiazhai mentioned this pull request Apr 28, 2025

[fix][broker] Resolve the issue of frequent updates in message expiration deletion rate #24190

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[fix][broker] Fix issue where msgRateExpired may not refresh forever #19759

[fix][broker] Fix issue where msgRateExpired may not refresh forever #19759

Uh oh!

massakam commented Mar 9, 2023 •

edited

Loading

Uh oh!

codecov-commenter commented Mar 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	public boolean expireMessages(int messageTTLInSeconds) {
	if ((getNumberOfEntriesInBacklog(false) == 0) \|\| (dispatcher != null && dispatcher.isConsumerConnected()
	&& getNumberOfEntriesInBacklog(false) < MINIMUM_BACKLOG_FOR_EXPIRY_CHECK
	&& !topic.isOldestMessageExpired(cursor, messageTTLInSeconds))) {
	// don't do anything for almost caught-up connected subscriptions
	return false;
	}
	this.lastExpireTimestamp = System.currentTimeMillis();
	return expiryMonitor.expireMessages(messageTTLInSeconds);
	}

[fix][broker] Fix issue where msgRateExpired may not refresh forever #19759

[fix][broker] Fix issue where msgRateExpired may not refresh forever #19759

Uh oh!

Conversation

massakam commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Documentation

Uh oh!

codecov-commenter commented Mar 9, 2023

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

massakam commented Mar 9, 2023 •

edited

Loading