Skip to content

Conversation

@TakaHiR07
Copy link
Contributor

@TakaHiR07 TakaHiR07 commented Oct 13, 2023

Motivation

  1. Heartbeat namespace should not create __change_events topics. But if we call admin.topics.getRetention(persistent://pulsar/localhost:65213/healthcheck) , or other admin topic api, event topic still be created. we'd better avoid this case, although in most time we would not call admin api for healthcheck topic.
2023-10-12T11:52:59,499+0800 [pulsar-io-8-3] INFO  org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [12/10月/2023:11:52:59 +0800] "GET /admin/v2/brokers/health?topicVersion=V2 HTTP/1.1" 200 2 "-" "Pulsar-Java-v3.2.0-SNAPSHOT" 426
2023-10-12T11:52:59,508+0800 [bookkeeper-ml-scheduler-OrderedScheduler-6-0] INFO  org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - Opening managed ledger pulsar/localhost:54480/persistent/__change_events-partition-1
2023-10-12T11:52:59,513+0800 [bookkeeper-ml-scheduler-OrderedScheduler-6-0] INFO  org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [pulsar/localhost:54480/persistent/__change_events-partition-1] Closing managed ledger
2023-10-12T11:52:59,531+0800 [pulsar-7-4] INFO  org.apache.pulsar.broker.systopic.NamespaceEventsSystemTopicFactory - Create topic policies system topic client persistent://pulsar/localhost:54480/__change_events
2023-10-12T11:52:59,533+0800 [pulsar-7-4] WARN  org.apache.pulsar.client.util.RetryUtil - Execution with retry fail, because of Topic policies cache have not init., will retry in 500 ms
2023-10-12T11:52:59,561+0800 [configuration-metadata-store-4-1] INFO  org.apache.pulsar.broker.service.BrokerService - partitioned metadata successfully created for persistent://pulsar/localhost:54480/__change_events
2023-10-12T11:52:59,574+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.client.impl.ConsumerImpl - [persistent://pulsar/localhost:54480/__change_events-partition-0][multiTopicsReader-12e15916e3] Subscribing to topic on cnx [id: 0x3d7925a7, L:/127.0.0.1:54525 - R:localhost/127.0.0.1:54440], consumerId 1
2023-10-12T11:52:59,575+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.client.impl.ConsumerImpl - [persistent://pulsar/localhost:54480/__change_events-partition-1][multiTopicsReader-12e15916e3] Subscribing to topic on cnx [id: 0x3d7925a7, L:/127.0.0.1:54525 - R:localhost/127.0.0.1:54440], consumerId 2
2023-10-12T11:52:59,575+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.client.impl.ConsumerImpl - [persistent://pulsar/localhost:54480/__change_events-partition-2][multiTopicsReader-12e15916e3] Subscribing to topic on cnx [id: 0x3d7925a7, L:/127.0.0.1:54525 - R:localhost/127.0.0.1:54440], consumerId 3
2023-10-12T11:52:59,575+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.client.impl.ConsumerImpl - [persistent://pulsar/localhost:54480/__change_events-partition-3][multiTopicsReader-12e15916e3] Subscribing to topic on cnx [id: 0x3d7925a7, L:/127.0.0.1:54525 - R:localhost/127.0.0.1:54440], consumerId 4
2023-10-12T11:52:59,575+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.client.impl.ConsumerImpl - [persistent://pulsar/localhost:54480/__change_events-partition-4][multiTopicsReader-12e15916e3] Subscribing to topic on cnx [id: 0x3d7925a7, L:/127.0.0.1:54525 - R:localhost/127.0.0.1:54440], consumerId 5
  1. heartbeat topic can not be deleted by admin api
2023-10-13T16:59:42,425+0800 [pulsar-io-8-3] INFO  org.apache.pulsar.broker.admin.impl.BrokersBase - [null] Successfully run health check.
2023-10-13T16:59:42,428+0800 [pulsar-io-8-3] INFO  org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [13/10月/2023:16:59:41 +0800] "GET /admin/v2/brokers/health?topicVersion=V2 HTTP/1.1" 200 2 "-" "Pulsar-Java-v3.2.0-SNAPSHOT" 448
2023-10-13T16:59:42,445+0800 [ForkJoinPool.commonPool-worker-3] INFO  org.apache.pulsar.broker.service.BrokerService - Successfully delete authentication policies for topic persistent://pulsar/localhost:60562/healthcheck
2023-10-13T16:59:42,447+0800 [PulsarTestContext-executor-OrderedExecutor-0-0] INFO  org.apache.pulsar.broker.service.BrokerService - Delete schema storage of id: pulsar/localhost:60562/healthcheck
2023-10-13T16:59:42,453+0800 [metadata-store-2-1] ERROR org.apache.pulsar.broker.service.persistent.PersistentTopic - [persistent://pulsar/localhost:60562/healthcheck] Error deleting topic
java.util.concurrent.CompletionException: org.apache.pulsar.broker.service.BrokerServiceException$NotAllowedException: Not allowed to send event to health check topic
	at java.util.concurrent.CompletableFuture.encodeRelay(CompletableFuture.java:368) ~[?:?]
	at java.util.concurrent.CompletableFuture.completeRelay(CompletableFuture.java:377) ~[?:?]
	at java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1152) ~[?:?]
	at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510) ~[?:?]
	at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147) ~[?:?]
	at org.apache.pulsar.broker.service.schema.BookkeeperSchemaStorage.lambda$deleteSchema$22(BookkeeperSchemaStorage.java:403) ~[classes/:?]
	at java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:787) ~[?:?]
	at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510) ~[?:?]
	at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147) ~[?:?]
	at org.apache.pulsar.metadata.impl.ZKMetadataStore.handleDeleteResult(ZKMetadataStore.java:307) ~[classes/:?]
	at org.apache.pulsar.metadata.impl.ZKMetadataStore.lambda$batchOperation$5(ZKMetadataStore.java:216) ~[classes/:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]
	at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[netty-common-4.1.99.Final.jar:4.1.99.Final]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: org.apache.pulsar.broker.service.BrokerServiceException$NotAllowedException: Not allowed to send event to health check topic
	at org.apache.pulsar.broker.service.SystemTopicBasedTopicPoliciesService.sendTopicPolicyEvent(SystemTopicBasedTopicPoliciesService.java:116) ~[classes/:?]
	at org.apache.pulsar.broker.service.SystemTopicBasedTopicPoliciesService.deleteTopicPoliciesAsync(SystemTopicBasedTopicPoliciesService.java:101) ~[classes/:?]
	at org.apache.pulsar.broker.service.BrokerService.deleteTopicPolicies(BrokerService.java:3468) ~[classes/:?]
	at org.apache.pulsar.broker.service.AbstractTopic.deleteTopicPolicies(AbstractTopic.java:1246) ~[classes/:?]
	at org.apache.pulsar.broker.service.persistent.PersistentTopic.lambda$delete$35(PersistentTopic.java:1360) ~[classes/:?]
	at java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1150) ~[?:?]
	... 15 more
2023-10-13T16:59:42,456+0800 [metadata-store-2-1] ERROR org.apache.pulsar.broker.service.persistent.PersistentTopic - [persistent://pulsar/localhost:60562/healthcheck] Error deleting topic

Modifications

  1. return null topicPolicy if topic belongs to heartbeat namespace.
  2. updateTopicPoliciesAsync of heartbeat topic throw exception, while deleteTopicPoliciesAsync of heartbeat topic should succeed.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: TakaHiR07#17

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 13, 2023
@Technoboy- Technoboy- added this to the 3.2.0 milestone Oct 16, 2023
public CompletableFuture<Void> updateTopicPoliciesAsync(TopicName topicName, TopicPolicies policies) {
if (NamespaceService.isHeartbeatNamespace(topicName.getNamespaceObject())) {
return CompletableFuture.failedFuture(new BrokerServiceException.NotAllowedException(
"Not allowed to send update event to health check topic"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Not allowed to send update event to health check topic"));
"Not allowed to update topic policy for the heartbeat topic"));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have done

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.26%. Comparing base (b1bca56) to head (bec574e).
⚠️ Report is 1717 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21360      +/-   ##
============================================
- Coverage     73.27%   73.26%   -0.02%     
+ Complexity    32581    32576       -5     
============================================
  Files          1888     1888              
  Lines        140282   140283       +1     
  Branches      15415    15417       +2     
============================================
- Hits         102790   102772      -18     
- Misses        29415    29433      +18     
- Partials       8077     8078       +1     
Flag Coverage Δ
inttests 24.23% <0.00%> (+0.07%) ⬆️
systests 24.69% <0.00%> (-0.03%) ⬇️
unittests 72.56% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../service/SystemTopicBasedTopicPoliciesService.java 73.68% <100.00%> (+0.29%) ⬆️

... and 67 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy- Technoboy- merged commit 700a29d into apache:master Oct 19, 2023
Technoboy- added a commit that referenced this pull request Oct 19, 2023
…elete heartbeat topic (#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 22, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
(cherry picked from commit 700a29d)
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 23, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
(cherry picked from commit 700a29d)
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 23, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
(cherry picked from commit 700a29d)
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 24, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
(cherry picked from commit 700a29d)
poorbarcode pushed a commit that referenced this pull request Oct 24, 2023
…elete heartbeat topic (#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
(cherry picked from commit 700a29d)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
…elete heartbeat topic (apache#21360)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwei Guo <technoboy@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants