jchen21 commented on pull request #5689:
URL: https://github.com/apache/geode/pull/5689#issuecomment-744970105


   @dschneider-pivotal So far, I still consider the current fix is the best 
option we can have. We need a convenient and reliable way to tell that the 
Oplog is offline compacted. The persisted `0` value of `totalCount` in krf 
serves the purpose. And during persistence recovery, if `totalCount` is `0`, 
then it is updated with the value of `totalLiveCount`, which is the most 
accurate value of the number of live entries in the Oplog.
   
   I have been thinking about whether we should use the local variable 
`krfEntryCount`, instead of `totalLiveCount`. At the end of `readKrf()`, the 
value of `totalLiveCount` and `krfEntryCount` are the same. Because at the 
beginning of `readKrf()`, `totalLiveCount` and `krfEntryCount` are both 0. And 
in the `while` loop inside `readKrf()`, both variables are incremented 
together: 
   
   `totalLiveCount` is incremented here: 
https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java#L1765
   
   `krfEntryCount` is incremented here: 
https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java#L1768
   
   I'd prefer using `totalLiveCount` instead of `krfEntryCount` which is a 
local 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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to