michaeljmarshall commented on a change in pull request #13073:
URL: https://github.com/apache/pulsar/pull/13073#discussion_r774321373
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PulsarStats.java
##########
@@ -139,6 +139,8 @@ public synchronized void updateStats(
// this task: helps to activate
inactive-backlog-cursors which have caught up and
// connected, also deactivate
active-backlog-cursors which has backlog
topic.checkBackloggedCursors();
+ // check if topic is inactive and require
ledger rollover
+ ((PersistentTopic)
topic).checkInactiveLedgers();
Review comment:
Nit: I can see that `topic.checkBackloaggedCursors()` here too, but I
wonder if this implementation tightly couples two distinct features. I doubt
that someone would turn off the stats feature, but if they did, they'd lose
this feature.
We already have an event loop used to delete inactive topics. The broker
service initializes several inactivity related loops here:
https://github.com/apache/pulsar/blob/f97d6e5a50355e1de628569c6de68cd7210094f0/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L522-L545
What is your perspective on adding this check to that loop? If we don't move
the loop, I think we should document somewhere that the frequency of this
method is governed by `statsUpdateFrequencyInSecs`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
