You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/10/23 04:03:12 UTC

[GitHub] [pulsar] lhotari commented on issue #8345: [Documentation] Retention policy documentation is misleading and not accurate

lhotari commented on issue #8345:
URL: https://github.com/apache/pulsar/issues/8345#issuecomment-714895284


   I now checked the Javadoc for ManagedLedgerConfig.setRetentionTime .
   
   "A retention time of 0 (the default), will to have no time based retention.
   Specifying a negative retention time will make the data to be retained indefinitely, based on the
   setRetentionSizeInMB value."
   
   https://github.com/apache/pulsar/blob/22b8923a251c3356401090dacb8ff97fd37d14c1/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java#L386-L388
   
   **This is surprising since the value 0 seems to have a different meaning for retention time compared to the meaning of value 0 for retention size.**
   
   "A retention size of 0, will make data to be deleted immediately. 
   A retention size of -1, means to have an unlimited retention size."
   
   https://github.com/apache/pulsar/blob/22b8923a251c3356401090dacb8ff97fd37d14c1/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java#L415-L417 
   
   The Javadoc for ManagedLedgerConfig.setRetentionTime is wrong.
   
   This can be verified in code:
   https://github.com/apache/pulsar/blob/5df23b592f055416fbdf4b0a665e01153a4a237c/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L1994-L2002
   
   In this code, retention time of 0 will make data to be deleted immediately.
   
   The javadoc specifies the meaning of zero value as "no time based retention". This would have no practical difference to how the javadoc describes the meaning of a negative value: "Specifying a negative retention time will make the data to be retained indefinitely, based on the setRetentionSizeInMB value". This is also "no time based retention".
   **The javadoc for ManagedLedgerConfig.setRetentionTime should also be fixed.** Something like this would be explicit:
   "A retention time of 0, will make data to be deleted immediately. 
   A retention time of -1, means to have an unlimited retention time. In this case, the overall retention condition will be based on retention size."


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