[GitHub] [pulsar] addisonj commented on issue #8220: managedLedgerOffloadDeletionLagMs and managedLedgerOffloadAutoTriggerSizeThresholdBytes are not used from config file

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

addisonj commented on issue #8220:
URL: https://github.com/apache/pulsar/issues/8220#issuecomment-712572579


   @gaoran10 We should make sure we support the original value names (`managedLedgerOffloadAutoTriggerSizeThresholdBytes`) as well as the new value names (`managedLedgerOffloadThresholdInBytes`) from the broker.conf, I think we should fix this by either:
   
   A) just going back to use the constructor here:https://github.com/apache/pulsar/blob/9f687d3f67d7534cb9ee77ddd2c7d0f24d34d0b4/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java#L79 and override the getter for getManagedLedgerOffloadAutoTriggerSizeThresholdBytes()` to also look at the other properties OR
   B) add a method to `ServiceConfiguration` that builds up all the relevant properties. Instead of using the `.getProperties()` method we could build `.getOffloadPolicyProperties()` which would normalize the values. 
   
   Personally, I think I prefer B.
   
   Also, just an an FYI, I talked with @milos-matijasevic and one improvement we could make is that when we construct a new namespace policy with the `pulsar-admin namespaces set-offload-policies` that you  wouldn't need to set all the properties and instead if you don't specify an argument, it will just merge it from the cluster default policy, but we should consider  that a separate improvement 
   


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