[GitHub] [pulsar] fmiguelez commented on pull request #8205: [Issue 8204][bookkeeper] Estimate backlog size as total if no consumer is present

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

fmiguelez commented on pull request #8205:
URL: https://github.com/apache/pulsar/pull/8205#issuecomment-704068552


   Hi @sijie thanks for your comments.
   
   Well if a ledger has no subscriptions but has alredy messages published to it you suggest that its size is zero but from the first moment someone connects (a subscription is created) then zero goes to whatever size ledger has. But if there is at least one subscription that has not consumed anything yet then it is ok to return the total size of the ledger even if rest of subscriptions have already consumed all of it. 
   
   I do not see that "bad" returning the total size of ledger if there are no durable subscriptions at all (take into consideration that `NonDurable` subscriptions do not count in this case) instead of zero size if there is actually data published to it. Indeed that is the backlog size all subscriptions will have that start out fresh from that moment on.
   
   In any case if you do not feel confortable with the change or you think it could break other things it is perfectly fine for me but I think that original issue #8204 must be tackled other way then. 
   
   The main problem is that if a topic receives data but no persistent subscription has been registered yet because consumers are still starting/stopped or because those consumers only use `NonDurable` subscriptions (such as with the `Reader` interface) automatic compaction will never be triggered. However if we trigger it manually it does work as expected. There is obviously a BUG there. The troubled logic is inside `PersistentTopic.checkCompaction()` method:
   
   ```
                   if (compactionSub != null) {
                       backlogEstimate = compactionSub.estimateBacklogSize();
                   } else {
                       // compaction has never run, so take full backlog size
                       // We enter here because "compaction" sub does not exist unless
                       // it has manually been triggered. This returns 0 as no other "durable" 
                       // subscriptions are available but ledger does has data
                       backlogEstimate = ledger.getEstimatedBacklogSize();  
                   }
   
                  // compactionThreshold must be configured over 0 si this can work 
                  // but backlogExtimate will always be 0
                  if (backlogEstimate > compactionThreshold) {
                  }
   ```
   
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org