Support get topic applied policy #9216

codelipenghui · 2021-01-15T08:23:03Z

Is your feature request related to a problem? Please describe.
Provide a way to get the applied policy of the topic. In Pulsar, the topic will apply the topic level policy if present or apply the namespace policy if present or use the broker level configuration. It needs to support get the applied policy for a topic that might use the namespace policy or topic policy or broker level default configuration.

Describe the solution you'd like
Add "--applied" for the get policy cmd. For example:

bin/pulsar-admin topics get-retention --applied

The text was updated successfully, but these errors were encountered:

315157973 · 2021-01-17T02:50:59Z

I will work on it

Master Issue: #9216 ### Motivation Provide a way to get the applied policy of the topic. In Pulsar, the topic will apply the topic level policy if present or apply the namespace policy if present or use the broker level configuration. It needs to support get the applied policy for a topic that might use the namespace policy or topic policy or broker level default configuration. ### Modifications 1 Now if there is no data at the namespace-level, the broker-level data will be returned by default, so I fix it 2 Now if there is no data at the topic-level, the data at the namespace-level will be returned by default, so I fix it 3 Add applied interface 4 Since the initial value becomes null, the corresponding unit test is modified

Master Issue: #9216 ### Modifications 1 Now if there is no data at the namespace-level, the broker-level data will be returned by default, so let it return null 2 add query param `applied` for getInactiveTopic 3 add `applied` API for client ### Verifying this change unit test: InactiveTopicDeleteTest#testInactiveTopicApplied

Anonymitaet · 2021-01-22T02:57:30Z

@315157973 thanks for your contribution.

Could you please add doc once you finish all code work?
Pulsar admin doc and REST API doc are automatically generated from codes, you can add corresponding descriptions in the code files. Thanks again.

315157973 · 2021-01-22T03:31:11Z

@315157973 thanks for your contribution.

Could you please add doc once you finish all code work?
Pulsar admin doc and REST API doc are automatically generated from codes, you can add corresponding descriptions in the code files. Thanks again.

OK

Master Issue: #9216 ### Modifications add applied API ### Verifying this change Test whether the API works: testGetMaxProducerApplied

Master Issue: #9216 ### Modifications add applied API for client ### Verifying this change unit test: adminApiDelayedDelivery.testNamespaceDelayedDeliveryPolicyApi adminApiDelayedDelivery.testDelayedDeliveryApplied

Master Issue: #9216 ### Modifications add applied API ### Verifying this change Test whether the API works: testGetMaxConsumersApplied

Master Issue: #9216 ### Modifications 1. Add applied API 2. Add remove API ### Verifying this change 1. Test whether the priority is correct when policies of different levels exist at the same time 2. Test applied API works

Master Issue: #9216 ### Modifications Add applied API for offloadPolicies

Anonymitaet · 2021-02-25T03:00:09Z

@315157973 @codelipenghui will these code changes apply to 2.7.1?

Master Issue: #9216 ### Modifications 1) The api name of topic-level is consistent with that of namespace-level 2) Fix the problem that the namespace-level policy cannot be removed 3) Fix the problem that topic-level does not take effect when multiple levels of policy are set at the same time 4) Added applied API ### Verifying this change Verify that the new API is correct Verify that the applied API is correct Verify that policies of different levels exist at the same time and whether the priority is correct

codelipenghui · 2021-03-02T04:12:13Z

@Anonymitaet No, this will available in 2.8.0

…9694) Master Issue: #9216 ### Modifications 1）Add applied API 2）Add tests of different levels of policy priority. Therefore, the namespace-level policy needs to add delete API

315157973 · 2021-03-07T17:20:34Z

@codelipenghui @315157973 I'd suggest fixing #9711 before making more changes to policy handling. WDYT?

I will fix thread safety issues in a unified way，will not affect this issue

lhotari · 2021-03-07T17:33:54Z

I will fix thread safety issues in a unified way

@315157973 that would be very good. How?

#9290) Master Issue: #9216 ### Modifications 1) Fix the unackedMessagesExceededOnSubscription at the namespace level cannot be disabled 2) Added applied interface ### Verifying this change test applied api : testMaxUnackedMessagesOnSubApplied test priority : testMaxUnackedMessagesOnSubscriptionPriority

codelipenghui · 2021-03-08T00:20:17Z

@lhotari This task is to support get applied policy for a topic, I don't understand why the new feature needs to wait for #9711. The topic policy does not persistent to zookeeper.

Master Issue: #9216 ### Modifications Add applied API and fix default value in unit test ### Verifying this change Verify the applied API and CMD Verify the default value in namespace-level

Master Issue: #9216 ### Modifications 1. Add applied API for topic policies 2. Add remove API for namespace policies ### Verifying this change testGetSubDispatchRateApplied

lhotari · 2021-03-08T06:21:48Z

This task is to support get applied policy for a topic, I don't understand why the new feature needs to wait for #9711. The topic policy does not persistent to zookeeper.

I have added a comment here: https://github.com/apache/pulsar/pull/9832/files#r589192556

@codelipenghui The thread safety issue isn't related to Zookeeper. It's mainly about unsafe mutation and access of HashMaps. The reason why I think #9711 should be addressed first is that more thread safety issues are introduced in the code base in the PRs related to this issue. The above example is such.
Fixing #9711 would most likely introduce a new way to mutate the various in-memory policy caches in a thread safe way.
Does this make sense?

Master Issue: #9216 ### Modifications 1. Add applied API for ReplicatorDispatchRate in topic-level 2. Add remove for ReplicatorDispatchRate in namespace-level ### Verifying this change Verify the applied API and CMD

codelipenghui · 2021-03-08T07:15:34Z

@lhotari Thanks for the clarification. I don't think it should block the new feature implement. I understand your concern, if we merge more PRs related to the policies, we need to do more changes when fixing #9711. But for community cooperation, we should not block the new features. #9216 is planned to release in 2.8.0. So we can confirm before release 2.8.0, #9711 can be fixed, is this works for you?

lhotari · 2021-03-08T07:28:58Z

@lhotari Thanks for the clarification. I don't think it should block the new feature implement. I understand your concern, if we merge more PRs related to the policies, we need to do more changes when fixing #9711. But for community cooperation, we should not block the new features. #9216 is planned to release in 2.8.0. So we can confirm before release 2.8.0, #9711 can be fixed, is this works for you?

@codelipenghui Makes sense.

Fixing #9711 might have broader impacts and dealing with it asap would be important. I hope it gets prioritized since it seems that the issue is cross-cutting and the current unsafe pattern is copied over to new feature development like we have seen in this case.
There might be simple fixes to address some of the issues, for example replacing plain HashMaps with thread safe wrappers (Collections.synchronizedMap) or ConcurrentHashMaps, but that could leave some data consistency issues.

Master Issue: #9216 ### Modifications Add applied API for topic-level Add remove API for namespace-level ### Verifying this change Verify the applied API and CMD

Master Issue: #9216 ### Modifications 1. Add applied API for topic-level 2. Add remove for namespace-level ### Verifying this change Verify applied API and CMD

Master Issue: #9216 ### Modifications 1. Add applied API for topic policies 2. Add remove API for namespace policies

Anonymitaet · 2021-04-09T08:05:37Z

@315157973 thanks for your great contribution!

Since you've finished all code work, could u pls add docs for them? Then users know your changes and how to use it.

I think the descriptions should be added to pulsar-admin, REST API, or Java admin API if needed.

Below is an example.

Feel free to ping me if you need doc review.

After doc is added, we can close this issue.

315157973 · 2021-04-09T09:40:49Z

There are descriptions in my code, and we already have automatic document generation tools, so I don’t need to add them manually, right? @Anonymitaet

315157973 · 2021-04-09T09:41:49Z

Looks like it has been generated

Anonymitaet · 2021-04-09T09:52:52Z

@315157973 yes, the "docs" are added in the code files and they will be shown on websites automatically.

One thing should pay attention to is that if your changes apply to pulsar-admin, REST API, and Java admin API, make sure the "docs" are added there. Do your changes apply to these 3 websites?

315157973 · 2021-04-09T10:10:28Z

"docs" are added in pulsar-admin, REST API.
The Java admin API does not seem to be automatically generated yet?
Many new APIs do not exist, not just those I added

Anonymitaet · 2021-04-09T12:51:07Z

@315157973 Java admin API is the same as what you did for pulsar-admin and REST API.
If you want to add docs for Java admin API, you may need to add descriptions to these files.

315157973 · 2021-04-10T04:33:38Z

All of my new APIs have descriptions. However, my new APIs and many old APIs with descriptions are not displayed in Java admin API. Please help to check the tool. @Anonymitaet
For example:

pulsar/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java

Lines 1660 to 1674 in 56bad04

    
               /** 
        
                * Set message TTL for a topic. 
        
                * 
        
                * @param topic 
        
                *          Topic name 
        
                * @param messageTTLInSecond 
        
                *          Message TTL in second. 
        
                * @throws NotAuthorizedException 
        
                *             Don't have admin permission 
        
                * @throws NotFoundException 
        
                *             Topic does not exist 
        
                * @throws PulsarAdminException 
        
                *             Unexpected error 
        
                */ 
        
               void setMessageTTL(String topic, int messageTTLInSecond) throws PulsarAdminException;

This API has been around for a long time, but it does not appear in the Java admin API

Anonymitaet · 2021-04-12T09:33:16Z

@315157973

setMessageTTL(String topic, int messageTTLInSecond) is shown on https://pulsar.apache.org/api/admin/2.7.0-SNAPSHOT/ but not shown on https://pulsar.apache.org/api/admin/.

I've checked all commands and documented details here. I found that all commands (except setMessageTTL(String topic, int messageTTLInSecond) are not shown on any version of the Java Admin API website.

Anonymitaet · 2021-04-12T09:38:39Z

Hi @tuteng

@315157973 has added various commands and docs correspondingly in their source code files.

Docs for pulsar-admin and REST API are shown on websites but docs for Java Admin API are not shown on any version of the Java Admin API website (https://pulsar.apache.org/api/admin/2.7.0-SNAPSHOT/ or https://pulsar.apache.org/api/admin/).

Do you have any clues? Many thanks.

Anonymitaet · 2021-04-13T09:03:20Z

Hi @tuteng could u pls give us some instructions? Many thanks.

Anonymitaet · 2021-04-15T04:26:34Z

@315157973 the docs you added for Java Admin API should be shown on https://pulsar.apache.org/api/admin/2.8.0-SNAPSHOT/, however, currently https://pulsar.apache.org/api/admin/2.8.0-SNAPSHOT/ is not generated yet and it should be generated after 2.7.1 is released. I've discussed w/ @tuteng and opened this issue to track. He will handle it.

Since code work has been finished and all docs have been added to source code files (pulsar-admin, REST API, and Java admin API), I'll close this issue and keep an eye for the commands that should be shown on https://pulsar.apache.org/api/admin/2.8.0-SNAPSHOT/ once https://pulsar.apache.org/api/admin/2.8.0-SNAPSHOT/ is generated.

Master Issue: apache#9216 Provide a way to get the applied policy of the topic. In Pulsar, the topic will apply the topic level policy if present or apply the namespace policy if present or use the broker level configuration. It needs to support get the applied policy for a topic that might use the namespace policy or topic policy or broker level default configuration. 1 Now if there is no data at the namespace-level, the broker-level data will be returned by default, so I fix it 2 Now if there is no data at the topic-level, the data at the namespace-level will be returned by default, so I fix it 3 Add applied interface 4 Since the initial value becomes null, the corresponding unit test is modified (cherry picked from commit 54b083b)

codelipenghui added the type/feature The PR added a new feature or issue requested a new feature label Jan 15, 2021

codelipenghui added this to the 2.8.0 milestone Jan 15, 2021

sijie mentioned this issue Jan 15, 2021

ISSUE-9216: Support get topic applied policy streamnative/pulsar-archived#2019

Closed

14 tasks

315157973 mentioned this issue Jan 17, 2021

Support get topic applied policy for message TTL #9225

Merged

codelipenghui assigned 315157973 Jan 17, 2021

Jennifer88huang-zz added doc-required Your PR changes impact docs and you will update later. triage/week-3 labels Jan 18, 2021

315157973 mentioned this issue Jan 18, 2021

Support get topic applied policy for InactiveTopic #9230

Merged

315157973 mentioned this issue Jan 20, 2021

Support get topic applied policy for DelayedDeliveryPolicies #9245

Merged

This was referenced Jan 23, 2021

Support get topic applied policy for MaxUnackedMessagesPerSubscription #9290

Merged

Support get topic applied policy for MaxProducer #9293

Merged

Support get topic applied policy for MaxConsumers #9296

Merged

sijie pushed a commit that referenced this issue Jan 25, 2021

Support get topic applied policy for MaxProducer (#9293)

a0cf37b

Master Issue: #9216 ### Modifications add applied API ### Verifying this change Test whether the API works: testGetMaxProducerApplied

This was referenced Jan 27, 2021

Support get topic applied policy for DeduplicationStatus #9339

Merged

Support get topic applied policy for Retention #9362

Merged

sijie pushed a commit that referenced this issue Feb 1, 2021

Support get topic applied policy for MaxConsumers (#9296)

d40fb58

Master Issue: #9216 ### Modifications add applied API ### Verifying this change Test whether the API works: testGetMaxConsumersApplied

315157973 mentioned this issue Feb 6, 2021

Support get topic applied policy for offloader #9505

Merged

codelipenghui pushed a commit that referenced this issue Feb 7, 2021

Support get topic applied policy for offloader (#9505)

a22eb82

Master Issue: #9216 ### Modifications Add applied API for offloadPolicies

315157973 mentioned this issue Feb 24, 2021

Support get topic applied policy for MaxUnackedMessagePerConsumers #9694

Merged

codelipenghui pushed a commit that referenced this issue Mar 10, 2021

Support get applied PersistencePolicies (#9831)

59e0187

Master Issue: #9216 ### Modifications Add applied API for topic-level Add remove API for namespace-level ### Verifying this change Verify the applied API and CMD

codelipenghui pushed a commit that referenced this issue Mar 10, 2021

Support get applied clusterSubscribeRate (#9832)

76883d4

Master Issue: #9216 ### Modifications 1. Add applied API for topic-level 2. Add remove for namespace-level ### Verifying this change Verify applied API and CMD

codelipenghui pushed a commit that referenced this issue Mar 11, 2021

Support get applied dispatchRate (#9824)

96c1402

Master Issue: #9216 ### Modifications 1. Add applied API for topic policies 2. Add remove API for namespace policies

315157973 mentioned this issue Mar 25, 2021

Support get applied compaction threshold #10038

Merged

Anonymitaet closed this as completed Apr 15, 2021

Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-required Your PR changes impact docs and you will update later. labels Apr 15, 2021

Technoboy- mentioned this issue Jun 13, 2022

[admin] Inconsistent response in GET namespace deduplication REST API endpoint #16024

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support get topic applied policy #9216

Support get topic applied policy #9216

codelipenghui commented Jan 15, 2021 •

edited by 315157973

Loading

315157973 commented Jan 17, 2021

Anonymitaet commented Jan 22, 2021

315157973 commented Jan 22, 2021

Anonymitaet commented Feb 25, 2021

codelipenghui commented Mar 2, 2021

315157973 commented Mar 7, 2021

lhotari commented Mar 7, 2021

codelipenghui commented Mar 8, 2021

lhotari commented Mar 8, 2021 •

edited

Loading

codelipenghui commented Mar 8, 2021

lhotari commented Mar 8, 2021

Anonymitaet commented Apr 9, 2021 •

edited

Loading

315157973 commented Apr 9, 2021

315157973 commented Apr 9, 2021

Anonymitaet commented Apr 9, 2021

315157973 commented Apr 9, 2021

Anonymitaet commented Apr 9, 2021 •

edited

Loading

315157973 commented Apr 10, 2021

Anonymitaet commented Apr 12, 2021

Anonymitaet commented Apr 12, 2021

Anonymitaet commented Apr 13, 2021

Anonymitaet commented Apr 15, 2021

Support get topic applied policy #9216

Support get topic applied policy #9216

Comments

codelipenghui commented Jan 15, 2021 • edited by 315157973 Loading

315157973 commented Jan 17, 2021

Anonymitaet commented Jan 22, 2021

315157973 commented Jan 22, 2021

Anonymitaet commented Feb 25, 2021

codelipenghui commented Mar 2, 2021

315157973 commented Mar 7, 2021

lhotari commented Mar 7, 2021

codelipenghui commented Mar 8, 2021

lhotari commented Mar 8, 2021 • edited Loading

codelipenghui commented Mar 8, 2021

lhotari commented Mar 8, 2021

Anonymitaet commented Apr 9, 2021 • edited Loading

315157973 commented Apr 9, 2021

315157973 commented Apr 9, 2021

Anonymitaet commented Apr 9, 2021

315157973 commented Apr 9, 2021

Anonymitaet commented Apr 9, 2021 • edited Loading

315157973 commented Apr 10, 2021

Anonymitaet commented Apr 12, 2021

Anonymitaet commented Apr 12, 2021

Anonymitaet commented Apr 13, 2021

Anonymitaet commented Apr 15, 2021

codelipenghui commented Jan 15, 2021 •

edited by 315157973

Loading

lhotari commented Mar 8, 2021 •

edited

Loading

Anonymitaet commented Apr 9, 2021 •

edited

Loading

Anonymitaet commented Apr 9, 2021 •

edited

Loading