You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "poorbarcode (via GitHub)" <gi...@apache.org> on 2023/11/20 09:37:41 UTC
[PR] [fix] [broker] Delete topic timeout due to NPE [pulsar]
poorbarcode opened a new pull request, #21595:
URL: https://github.com/apache/pulsar/pull/21595
### Motivation
**Background:**
- `BucketDelayedDeliveryTrackerFactory` was added with [PIP-195: New bucket based delayed message tracker](https://github.com/apache/pulsar/issues/16763)
- `BucketDelayedDeliveryTrackerFactory` will be initialized if setting config `setDelayedDeliveryTrackerFactoryClassName` to `BucketDelayedDeliveryTrackerFactory`
- Pulsar will delete the delay message indexes bucket when a subscription is unsubscribing.
**Issue:**
There is an NPE that causes the Future of Delay message indexes bucket deletion to be no longer complete, which leads to the topic deletion timeout.
**Issue logs:**
```
2023-11-16T17:53:00,154+0000 [delayer-53-1] WARN org.apache.pulsar.client.admin.internal.BaseResource - [http://event-platform-pulsar-1-sn-broker-0.event-platform-pulsar-1-sn-broker-headless.pulsar.svc.cluster.local:8080/admin/v2/persistent/climate/ep_integration_tests/sn_test_topic-partition-0?force=true&deleteSchema=true] Failed to perform http delete request: java.util.concurrent.CompletionException: org.apache.pulsar.common.util.FutureUtil$LowOverheadTimeoutException: Read timeout
2023-11-16T17:53:00,154+0000 [delayer-53-1] ERROR org.apache.pulsar.broker.admin.impl.PersistentTopicsBase - [11c16270-bb0f-42bf-9e53-f4a94c6e98c3] Failed to delete partition persistent://climate/ep_integration_tests/sn_test_topic-partition-0
org.apache.pulsar.client.admin.PulsarAdminException: java.util.concurrent.CompletionException: org.apache.pulsar.common.util.FutureUtil$LowOverheadTimeoutException: Read timeout
at org.apache.pulsar.client.admin.internal.BaseResource.getApiException(BaseResource.java:300) ~[org.apache.pulsar-pulsar-client-admin-original-3.0.1.jar:3.0.1]
at org.apache.pulsar.client.admin.internal.BaseResource$4.failed(BaseResource.java:237) ~[org.apache.pulsar-pulsar-client-admin-original-3.0.1.jar:3.0.1]
at org.glassfish.jersey.client.JerseyInvocation$1.failed(JerseyInvocation.java:882) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.glassfish.jersey.client.ClientRuntime.processFailure(ClientRuntime.java:247) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.glassfish.jersey.client.ClientRuntime.processFailure(ClientRuntime.java:242) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.glassfish.jersey.client.ClientRuntime.access$100(ClientRuntime.java:62) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.glassfish.jersey.client.ClientRuntime$2.lambda$failure$1(ClientRuntime.java:178) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.internal.Errors.process(Errors.java:292) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.internal.Errors.process(Errors.java:274) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.internal.Errors.process(Errors.java:244) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:288) ~[org.glassfish.jersey.core-jersey-common-2.34.jar:?]
at org.glassfish.jersey.client.ClientRuntime$2.failure(ClientRuntime.java:178) ~[org.glassfish.jersey.core-jersey-client-2.34.jar:?]
at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.lambda$apply$1(AsyncHttpConnector.java:227) ~[org.apache.pulsar-pulsar-client-admin-original-3.0.1.jar:3.0.1]
at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863) ~[?:?]
at java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841) ~[?:?]
at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510) ~[?:?]
at java.util.concurrent.CompletableFuture.completeExceptionally(CompletableFuture.java:2162) ~[?:?]
at org.apache.pulsar.common.util.FutureUtil.lambda$addTimeoutHandling$9(FutureUtil.java:236) ~[org.apache.pulsar-pulsar-common-3.0.1.jar:3.0.1]
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) ~[io.netty-netty-common-4.1.93.Final.jar:4.1.93.Final]
at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: java.util.concurrent.CompletionException: org.apache.pulsar.common.util.FutureUtil$LowOverheadTimeoutException: Read timeout
at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332) ~[?:?]
at java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:347) ~[?:?]
at java.util.concurrent.CompletableFuture$OrApply.tryFire(CompletableFuture.java:1576) ~[?:?]
at java.util.concurrent.CompletableFuture$CoCompletion.tryFire(CompletableFuture.java:1219) ~[?:?]
... 10 more
Caused by: org.apache.pulsar.common.util.FutureUtil$LowOverheadTimeoutException: Read timeout
at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.retryOrTimeout(...)(Unknown Source) ~[org.apache.pulsar-pulsar-client-admin-original-3.0.1.jar:3.0.1]
2023-11-16T17:53:00,160+0000 [configuration-metadata-store-13-1] INFO org.eclipse.jetty.server.RequestLog - 172.16.167.53 - - [16/Nov/2023:17:52:29 +0000] "DELETE /admin/v2/persistent/climate/ep_integration_tests/sn_test_topic/partitions?force=true HTTP/1.1" 500 2 "-" "PostmanRuntime/7.34.0" 30256
```
### Modifications
Fix the NPE.
### 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: x
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode merged PR #21595:
URL: https://github.com/apache/pulsar/pull/21595
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode closed pull request #21595: [fix] [broker] Delete topic timeout due to NPE
URL: https://github.com/apache/pulsar/pull/21595
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on code in PR #21595:
URL: https://github.com/apache/pulsar/pull/21595#discussion_r1399947803
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/BucketDelayedDeliveryTrackerFactory.java:
##########
@@ -35,6 +37,7 @@
import org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers;
import org.apache.pulsar.common.util.FutureUtil;
+@Slf4j
Review Comment:
Not necessary change.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/BucketDelayedDeliveryTrackerFactory.java:
##########
@@ -85,6 +88,9 @@ public DelayedDeliveryTracker newTracker(PersistentDispatcherMultipleConsumers d
*/
public CompletableFuture<Void> cleanResidualSnapshots(ManagedCursor cursor) {
Map<String, String> cursorProperties = cursor.getCursorProperties();
+ if (MapUtils.isEmpty(cursorProperties)) {
Review Comment:
I wonder which place introduce the `null` cursorProperties. And if so, should all the place check `cursorProperties` is empty or not:)?
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #21595:
URL: https://github.com/apache/pulsar/pull/21595#discussion_r1399975738
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/BucketDelayedDeliveryTrackerFactory.java:
##########
@@ -85,6 +88,9 @@ public DelayedDeliveryTracker newTracker(PersistentDispatcherMultipleConsumers d
*/
public CompletableFuture<Void> cleanResidualSnapshots(ManagedCursor cursor) {
Map<String, String> cursorProperties = cursor.getCursorProperties();
+ if (MapUtils.isEmpty(cursorProperties)) {
Review Comment:
> I wonder which place introduce the null cursorProperties. And if so, should all the place check cursorProperties is empty or not:)?
Good suggestion. Fixed
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #21595:
URL: https://github.com/apache/pulsar/pull/21595#discussion_r1399974979
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/BucketDelayedDeliveryTrackerFactory.java:
##########
@@ -35,6 +37,7 @@
import org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers;
import org.apache.pulsar.common.util.FutureUtil;
+@Slf4j
Review Comment:
Removed
--
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] Delete topic timeout due to NPE [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #21595:
URL: https://github.com/apache/pulsar/pull/21595#issuecomment-1820517177
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#21595](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e1071be) into [master](https://app.codecov.io/gh/apache/pulsar/commit/cbdb19c2fd7f5ad70bbd8ca9b3c7740b2f98f4e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (cbdb19c) will **decrease** coverage by `0.02%`.
> Report is 4 commits behind head on master.
> The diff coverage is `90.32%`.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #21595 +/- ##
============================================
- Coverage 73.27% 73.26% -0.02%
- Complexity 32694 32703 +9
============================================
Files 1892 1892
Lines 140707 140682 -25
Branches 15483 15493 +10
============================================
- Hits 103098 103065 -33
- Misses 29496 29508 +12
+ Partials 8113 8109 -4
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/21595/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/21595/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.09% <12.90%> (-0.05%)` | :arrow_down: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/21595/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.66% <12.90%> (+0.01%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/21595/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.59% <90.32%> (+0.02%)` | :arrow_up: |
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/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...che/bookkeeper/mledger/impl/ManagedCursorImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRDdXJzb3JJbXBsLmphdmE=) | `79.07% <100.00%> (-0.12%)` | :arrow_down: |
| [...org/apache/pulsar/broker/ServiceConfiguration.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvU2VydmljZUNvbmZpZ3VyYXRpb24uamF2YQ==) | `99.38% <100.00%> (+<0.01%)` | :arrow_up: |
| [...r/delayed/BucketDelayedDeliveryTrackerFactory.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9kZWxheWVkL0J1Y2tldERlbGF5ZWREZWxpdmVyeVRyYWNrZXJGYWN0b3J5LmphdmE=) | `91.42% <100.00%> (+0.51%)` | :arrow_up: |
| [...r/delayed/bucket/BucketDelayedDeliveryTracker.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9kZWxheWVkL2J1Y2tldC9CdWNrZXREZWxheWVkRGVsaXZlcnlUcmFja2VyLmphdmE=) | `83.62% <100.00%> (-0.88%)` | :arrow_down: |
| [...g/apache/pulsar/client/impl/RawBatchConverter.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Jhd0JhdGNoQ29udmVydGVyLmphdmE=) | `93.75% <87.50%> (+0.24%)` | :arrow_up: |
| [...rg/apache/pulsar/compaction/TwoPhaseCompactor.java](https://app.codecov.io/gh/apache/pulsar/pull/21595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbXBhY3Rpb24vVHdvUGhhc2VDb21wYWN0b3IuamF2YQ==) | `73.83% <85.71%> (+0.15%)` | :arrow_up: |
... and [71 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/21595/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
--
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