You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "TakaHiR07 (via GitHub)" <gi...@apache.org> on 2023/10/13 08:58:40 UTC
[PR] [fix][broker] Fix heartbeat namespace create event topic and cannot delete heartbeat topic [pulsar]
TakaHiR07 opened a new pull request, #21360:
URL: https://github.com/apache/pulsar/pull/21360
### 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.
2. heartbeat topic can not be deleted by admin api.
```
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
```
### 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
- [x] Make sure that the change passes the CI checks.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: https://github.com/TakaHiR07/pulsar/pull/17
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] Fix heartbeat namespace create event topic and cannot delete heartbeat topic [pulsar]
Posted by "TakaHiR07 (via GitHub)" <gi...@apache.org>.
TakaHiR07 commented on code in PR #21360:
URL: https://github.com/apache/pulsar/pull/21360#discussion_r1364788432
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java:
##########
@@ -95,20 +95,23 @@ public SystemTopicBasedTopicPoliciesService(PulsarService pulsarService) {
@Override
public CompletableFuture<Void> deleteTopicPoliciesAsync(TopicName topicName) {
+ if (NamespaceService.isHeartbeatNamespace(topicName.getNamespaceObject())) {
+ return CompletableFuture.completedFuture(null);
+ }
return sendTopicPolicyEvent(topicName, ActionType.DELETE, null);
}
@Override
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"));
Review Comment:
have done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] Fix heartbeat namespace create event topic and cannot delete heartbeat topic [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #21360:
URL: https://github.com/apache/pulsar/pull/21360#issuecomment-1769860228
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#21360](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (bec574e) into [master](https://app.codecov.io/gh/apache/pulsar/commit/b1bca5609d254734ccca63b616eba33ce3a8b70b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b1bca56) will **decrease** coverage by `0.02%`.
> Report is 4 commits behind head on master.
> The diff coverage is `100.00%`.
[](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ 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](https://app.codecov.io/gh/apache/pulsar/pull/21360/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/21360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.23% <12.50%> (+0.07%)` | :arrow_up: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/21360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.69% <0.00%> (-0.03%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/21360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.56% <100.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../service/SystemTopicBasedTopicPoliciesService.java](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1N5c3RlbVRvcGljQmFzZWRUb3BpY1BvbGljaWVzU2VydmljZS5qYXZh) | `73.68% <100.00%> (+0.29%)` | :arrow_up: |
| [...sar/broker/service/persistent/PersistentTopic.java](https://app.codecov.io/gh/apache/pulsar/pull/21360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL3BlcnNpc3RlbnQvUGVyc2lzdGVudFRvcGljLmphdmE=) | `79.49% <100.00%> (+0.20%)` | :arrow_up: |
... and [66 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/21360/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] Fix heartbeat namespace create event topic and cannot delete heartbeat topic [pulsar]
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui commented on code in PR #21360:
URL: https://github.com/apache/pulsar/pull/21360#discussion_r1363931812
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java:
##########
@@ -95,20 +95,23 @@ public SystemTopicBasedTopicPoliciesService(PulsarService pulsarService) {
@Override
public CompletableFuture<Void> deleteTopicPoliciesAsync(TopicName topicName) {
+ if (NamespaceService.isHeartbeatNamespace(topicName.getNamespaceObject())) {
+ return CompletableFuture.completedFuture(null);
+ }
return sendTopicPolicyEvent(topicName, ActionType.DELETE, null);
}
@Override
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"));
Review Comment:
```suggestion
"Not allowed to update topic policy for the heartbeat topic"));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] Fix heartbeat namespace create event topic and cannot delete heartbeat topic [pulsar]
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- merged PR #21360:
URL: https://github.com/apache/pulsar/pull/21360
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org