Skip to content

Conversation

fxbing
Copy link
Contributor

@fxbing fxbing commented Jul 25, 2019

fix #4755
Retention policy is used in Ledger GC and Topic GC.
Default retention policy is not uploaded to zookeeper, but it is getted from zookeeper when it is used.
So, if zookeeper doesn't have retention policy, we should load it from default config file.
In Ledger GC configuration, it's OK.

RetentionPolicies retentionPolicies = policies.map(p -> p.retention_policies).orElseGet(
() -> new RetentionPolicies(serviceConfig.getDefaultRetentionTimeInMinutes(),
serviceConfig.getDefaultRetentionSizeInMB())
);

But in Topic GC, do nothing.

@sijie
Copy link
Member

sijie commented Jul 25, 2019

@fxbing can you add a test for this?

@fxbing
Copy link
Contributor Author

fxbing commented Jul 26, 2019

run integration tests

@sijie sijie added area/admin area/test type/bug The PR fixed a bug or issue reported a bug labels Jul 27, 2019
@sijie sijie added this to the 2.4.1 milestone Jul 27, 2019
@sijie sijie merged commit 1dcac8c into apache:master Jul 27, 2019
jiazhai pushed a commit that referenced this pull request Aug 28, 2019
fix #4755
Retention policy is used in Ledger GC and Topic GC.
Default retention policy is not uploaded to zookeeper, but it is getted from zookeeper when it is used.
So, if zookeeper doesn't have retention policy, we should load it from default config file.
In Ledger GC configuration, it's OK.
https://github.com/apache/pulsar/blob/075f28b71c8fd9259ce8e136dbb81c0629c3f271/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L739-L742
But in Topic GC, do nothing.
(cherry picked from commit 1dcac8c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin area/test type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

defaultRetentionTimeInMinutes from conf/standalone.conf doesn't do nothing
2 participants