Re: [I] Cleanup synchronous call in resources component [pulsar]
lhotari commented on issue #22544: URL: https://github.com/apache/pulsar/issues/22544#issuecomment-2071571572 We have another problem with asynchronous calls and that's about the lack of proper backpressure. Here's a comment about it: https://github.com/apache/pulsar/pull/22541#issuecomment-2071568113 . -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch branch-3.0 updated: Revert "[fix][test][branch-3.0] Fix broken ManagedLedgerTest.testGetNumberOfEntriesInStorage"
This is an automated email from the ASF dual-hosted git repository. zhaocong pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/branch-3.0 by this push: new d6791a8de26 Revert "[fix][test][branch-3.0] Fix broken ManagedLedgerTest.testGetNumberOfEntriesInStorage" d6791a8de26 is described below commit d6791a8de269498cf2f5d82a0ac54baab60f8845 Author: coderzc AuthorDate: Tue Apr 23 15:04:57 2024 +0800 Revert "[fix][test][branch-3.0] Fix broken ManagedLedgerTest.testGetNumberOfEntriesInStorage" This reverts commit e3531e808c172ff588e36499c41746835d06904a. --- .../test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java index c28a32d9ec2..c16c31a7e96 100644 --- a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java +++ b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java @@ -2643,7 +2643,7 @@ public class ManagedLedgerTest extends MockedBookKeeperTestCase { // trigger ledger rollover and wait for the new ledger created Awaitility.await().untilAsserted(() -> { - assertEquals(managedLedger.getState(), ManagedLedgerImpl.State.ClosedLedger); + assertEquals("LedgerOpened", WhiteboxImpl.getInternalState(managedLedger, "state").toString()); }); managedLedger.createLedgerAfterClosed(); Awaitility.await().untilAsserted(() -> {
Re: [PR] [fix][broker] Introduce the last sent position to fix message ordering issues in Key_Shared (PIP-282) [pulsar]
equanz commented on PR #21953: URL: https://github.com/apache/pulsar/pull/21953#issuecomment-2071580220 Rebased to fix conflicts. BTW, can someone review this? I've been trying to fix the issue for a year now... (cf. https://github.com/apache/pulsar/pull/20179) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][broker] Make BrokerSelectionStrategy pluggable [pulsar]
BewareMyPower commented on PR #22553: URL: https://github.com/apache/pulsar/pull/22553#issuecomment-2071585235 The test failed because the broker selection strategy of `CustomExtensibleLoadManager ` chooses the broker with the smallest **HTTP listen port**, while the test uses the `PulsarService` instance with the smallest **binary listen port** as the expected broker. I fixed it now -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][broker] Support lookup options for extensible load manager [pulsar]
BewareMyPower merged PR #22487: URL: https://github.com/apache/pulsar/pull/22487 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [fix][broker] Support lookup options for extensible load manager (#22487)
This is an automated email from the ASF dual-hosted git repository. xyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new 7fe92ac43cf [fix][broker] Support lookup options for extensible load manager (#22487) 7fe92ac43cf is described below commit 7fe92ac43cfd2f2de5576a023498aac8b46c7ac8 Author: Kai Wang AuthorDate: Tue Apr 23 15:22:44 2024 +0800 [fix][broker] Support lookup options for extensible load manager (#22487) --- .../pulsar/broker/loadbalance/LoadManager.java | 3 +- .../extensions/ExtensibleLoadManager.java | 5 +- .../extensions/ExtensibleLoadManagerImpl.java | 53 +- .../extensions/ExtensibleLoadManagerWrapper.java | 15 +++-- .../channel/ServiceUnitStateChannelImpl.java | 4 +- .../extensions/data/BrokerLookupData.java | 17 +- .../pulsar/broker/namespace/NamespaceService.java | 4 +- .../AntiAffinityNamespaceGroupExtensionTest.java | 4 +- .../ExtensibleLoadManagerImplBaseTest.java | 4 ++ .../extensions/ExtensibleLoadManagerImplTest.java | 65 ++ .../channel/ServiceUnitStateChannelTest.java | 14 ++--- .../extensions/data/BrokerLookupDataTest.java | 32 ++- .../loadbalance/ExtensibleLoadManagerTest.java | 3 +- 13 files changed, 162 insertions(+), 61 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LoadManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LoadManager.java index 2cce68b60cb..0dd5d948480 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LoadManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LoadManager.java @@ -31,6 +31,7 @@ import org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerWrap import org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerWrapper; import org.apache.pulsar.broker.loadbalance.impl.SimpleLoadManagerImpl; import org.apache.pulsar.broker.lookup.LookupResult; +import org.apache.pulsar.broker.namespace.LookupOptions; import org.apache.pulsar.common.naming.ServiceUnitId; import org.apache.pulsar.common.stats.Metrics; import org.apache.pulsar.common.util.Reflections; @@ -63,7 +64,7 @@ public interface LoadManager { Optional getLeastLoaded(ServiceUnitId su) throws Exception; default CompletableFuture> findBrokerServiceUrl( -Optional topic, ServiceUnitId bundle) { +Optional topic, ServiceUnitId bundle, LookupOptions options) { throw new UnsupportedOperationException(); } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManager.java index b7da70d1cf1..eabf6005b43 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManager.java @@ -60,9 +60,12 @@ public interface ExtensibleLoadManager extends Closeable { * (e.g. {@link NamespaceService#internalGetWebServiceUrl(NamespaceBundle, LookupOptions)}), * So the topic is optional. * @param serviceUnit service unit (e.g. bundle). + * @param options The lookup options. * @return The broker lookup data. */ -CompletableFuture> assign(Optional topic, ServiceUnitId serviceUnit); +CompletableFuture> assign(Optional topic, + ServiceUnitId serviceUnit, + LookupOptions options); /** * Check the incoming service unit is owned by the current broker. diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java index c8cf1c05756..a20694356b1 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java @@ -88,6 +88,7 @@ import org.apache.pulsar.broker.loadbalance.extensions.strategy.BrokerSelectionS import org.apache.pulsar.broker.loadbalance.extensions.strategy.LeastResourceUsageWithWeight; import org.apache.pulsar.broker.loadbalance.impl.LoadManagerShared; import org.apache.pulsar.broker.loadbalance.impl.SimpleResourceAllocationPolicies; +import org.apache.pulsar.broker.namespace.LookupOptions; import org.apache.pulsar.broker.namespace.NamespaceEphemeralData; import org.apache.pulsar.broker.namespace.NamespaceService;
Re: [PR] [improve][pip] PIP-346: Add a simplified configuration file for Pulsar [pulsar]
lhotari commented on PR #22274: URL: https://github.com/apache/pulsar/pull/22274#issuecomment-2071609387 > Hi, could we move forward with this PIP? It seems that it is not rely on another PIP `Proposal to Replace Pulsar's Homegrown Configuration Framework with Gestalt Config`. @merlimat @BewareMyPower @crossoverJie @codelipenghui @dao-jun @lhotari The current Pulsar configuration solution is currently a mess (example: `apply-config-from-env.py` is needed to override configuration from environment) and instead of just doing changes that don't really address the problems, I think that it's a distraction. That's why I think that we shouldn't proceed on PIP-346. We need a design for revisiting the configuration solution for Pulsar 4.0 and beyond. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch branch-3.0 updated: [fix][test][branch-3.0] Fix test PersistentTopicsTest.testUpdatePartitionedTopic
This is an automated email from the ASF dual-hosted git repository. zhaocong pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/branch-3.0 by this push: new 6e849fcb064 [fix][test][branch-3.0] Fix test PersistentTopicsTest.testUpdatePartitionedTopic 6e849fcb064 is described below commit 6e849fcb06447d767c4f3fa521b5a1c4d14c9aae Author: coderzc AuthorDate: Tue Apr 23 15:27:19 2024 +0800 [fix][test][branch-3.0] Fix test PersistentTopicsTest.testUpdatePartitionedTopic --- .../java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java index 0eddbf1fea1..2095582fd6f 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java @@ -57,9 +57,9 @@ import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.mledger.ManagedCursor; import org.apache.commons.collections4.MapUtils; import org.apache.pulsar.broker.BrokerTestUtil; +import org.apache.pulsar.broker.admin.v2.ExtPersistentTopics; import org.apache.pulsar.broker.admin.v2.NonPersistentTopics; import org.apache.pulsar.broker.admin.v2.PersistentTopics; -import org.apache.pulsar.broker.admin.v2.ExtPersistentTopics; import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest; import org.apache.pulsar.broker.authentication.AuthenticationDataHttps; import org.apache.pulsar.broker.namespace.NamespaceService; @@ -1667,7 +1667,7 @@ public class PersistentTopicsTest extends MockedPulsarServiceBaseTest { true, 5); verify(response, timeout(5000).times(1)).resume(throwableCaptor.capture()); Assert.assertEquals(throwableCaptor.getValue().getMessage(), -"Desired partitions 5 can't be greater than the maximum partitions per topic 4."); +"Expect partitions 5 grater than maximum partitions per topic 4"); response = mock(AsyncResponse.class); metaCaptor = ArgumentCaptor.forClass(PartitionedTopicMetadata.class);
Re: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071621213 Another location without proper backpressure is namespace deletion: https://github.com/apache/pulsar/blob/d7d54522933b63f6a74ec7139c6dedebe8ad9149/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L282-L293 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Argo CD unable to generate template with pulsar chart 3.3.0 [pulsar-helm-chart]
lhotari closed issue #471: Argo CD unable to generate template with pulsar chart 3.3.0 URL: https://github.com/apache/pulsar-helm-chart/issues/471 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] customRuntimeOptions in "Creates a new Pulsar Source in cluster mode" API call in source connector doesn't add nodeselectors & tolerations [pulsar-helm-chart]
lhotari commented on issue #394: URL: https://github.com/apache/pulsar-helm-chart/issues/394#issuecomment-2071647058 > But I want different nodeSelector for different functions which I deploy. @kunjbosamiaFalkonry One possible approach would be to create your own customizer implementation. Currently there's not an easy way to get your custom classes into use. You need a custom overlay docker image with the jar file in /pulsar/lib . There's a feature request to add init container support that could copy the jar file to /pulsar/lib when the pod starts. That's #491 / #240 . This is an open source project and contributions are more than welcome to improve Apache Pulsar Helm chart. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug] broker healthcheck ran into loop after decommissioned a cluster of bookies [pulsar]
hpvd commented on issue #22559: URL: https://github.com/apache/pulsar/issues/22559#issuecomment-2071657216 since 2.10 is not supported anymore, cna you plz check if this also appears in newer versions? For details see - supported versions: https://pulsar.apache.org/contribute/release-policy/ - latest pulsar helm chart: https://github.com/apache/pulsar-helm-chart/releases -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071666071 example of creating partitions: https://github.com/apache/pulsar/blob/50121e7f7be541f45bb6dc976f51e30658b1cb8d/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java#L162-L171 This would need backpressure too. Let's say if you create a 100 partition topic, the broker might open 100 HTTP connections to create the topic partitions concurrently. This is problematic when the brokers are under heavy load. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][ci] Don't allow merging PR without successful result [pulsar]
BewareMyPower merged PR #22563: URL: https://github.com/apache/pulsar/pull/22563 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated (7fe92ac43cf -> 358c7cc6bbb)
This is an automated email from the ASF dual-hosted git repository. xyz pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git from 7fe92ac43cf [fix][broker] Support lookup options for extensible load manager (#22487) add 358c7cc6bbb [fix][ci] Don't allow merging PR without successful result (#22563) No new revisions were added by this update. Summary of changes: .github/workflows/pulsar-ci.yaml | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-)
Re: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071687995 Looks like https://github.com/spotify/completable-futures/blob/master/src/main/java/com/spotify/futures/ConcurrencyReducer.java could be a useful solution to leverage. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [improve][pip] PIP-347: add role field in consumer's stat [pulsar]
thetumbled opened a new pull request, #22564: URL: https://github.com/apache/pulsar/pull/22564 ### Motivation pip-347 ### Modifications ### Verifying this change - [ ] Make sure that the change passes the CI checks. *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (10MB)* - *Extended integration test for recovery after broker failure* ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` - [ ] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] [broker] add role for consumer stat [pulsar]
thetumbled commented on code in PR #22562: URL: https://github.com/apache/pulsar/pull/22562#discussion_r1575843420 ## pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ConsumerStats.java: ## @@ -25,6 +25,9 @@ * Consumer statistics. */ public interface ConsumerStats { +/** the role used for authorization. */ +String getRole(); Review Comment: this is the pip for this pr: https://github.com/apache/pulsar/pull/22564. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][pip] PIP-347: add role field in consumer's stat [pulsar]
thetumbled commented on PR #22564: URL: https://github.com/apache/pulsar/pull/22564#issuecomment-2071713876 PTAL, thanks. @BewareMyPower @lhotari @codelipenghui @poorbarcode @Technoboy- -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071822579 Noticed that there's a solution to run 1-by-1 using https://github.com/apache/pulsar/blob/ed599673c7e60ab5bb02e1fb0615a7ff8e5d6430/pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java#L179-L210 . However, I think that ConcurrencyReducer would be a better solution for most use cases. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [D] Apache Beam support for Pulsar [pulsar]
GitHub user hpvd added a comment to the discussion: Apache Beam support for Pulsar just opened an issue in Beam repository to maintain a good overview on this: https://github.com/apache/beam/issues/31078 GitHub link: https://github.com/apache/pulsar/discussions/18453#discussioncomment-9198904 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [I] [Bug] Client with shared subscription is blocked [pulsar]
MichalKoziorowski-TomTom commented on issue #21104: URL: https://github.com/apache/pulsar/issues/21104#issuecomment-2071870500 @michalcukierman Could you recheck your case? I've checked my case and I can't reproduce with 3.0.4 client while it was easily reproducible with 3.0.1. Test with version that includes https://github.com/apache/pulsar/issues/22352 fix. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] Why are Clusters not displayed? Is special configuration required? [pulsar-manager]
YANGJINJUE opened a new issue, #558: URL: https://github.com/apache/pulsar-manager/issues/558 "Why are Clusters not displayed? Is special configuration required?" ![image](https://github.com/apache/pulsar-manager/assets/32013488/14279828-f26a-4295-adbd-e8585abb3907) -- 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-unsubscr...@pulsar.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071881698 Another challenge is to cancel work that is queued in the system, but not waited by any clients. Newer Jersey clients have support for this. I noticed commit https://github.com/eclipse-ee4j/jersey/commit/96028068b6379ad923cf26ab018f372f3ea040f6 in Jersey. When the system is overloaded, request processing might be very slow so that clients get timeouts and retry requests. This will add more work to the system unless there's a solution that cancels the timed out tasks. That's why addressing this is also important part of the solution. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug] one topic suddenly cannot be consumed,others is ok [pulsar]
MichalKoziorowski-TomTom commented on issue #21082: URL: https://github.com/apache/pulsar/issues/21082#issuecomment-2071880394 @poorbarcode I've checked with 3.0.4 client containing https://github.com/apache/pulsar/issues/22352 fix and I can't reproduce it now. I don't know if this PR fixed it or some other, but I'm almost sure it would appear again with my testing, because ot was very easily reproducible with 3.0.1 version. About original issue question, I don't know. I had no problems with 2.9.2 client, only with >3.0 ones, so maybe that fixed only mine or https://github.com/apache/pulsar/issues/21104 issue. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [D] Apache Beam support for Pulsar [pulsar]
GitHub user hpvd added a comment to the discussion: Apache Beam support for Pulsar opened an issue to add current state of Pulsar support to Beams doc: https://github.com/apache/beam/issues/31079 GitHub link: https://github.com/apache/pulsar/discussions/18453#discussioncomment-9199245 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [PR] [improve] [broker] Create partitioned topics automatically when enable topic level replication [pulsar]
gaoran10 commented on code in PR #22537: URL: https://github.com/apache/pulsar/pull/22537#discussion_r1576003194 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java: ## @@ -621,35 +625,82 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n private void internalCreatePartitionedTopicToReplicatedClustersInBackground(int numPartitions) { getNamespaceReplicatedClustersAsync(namespaceName) -.thenAccept(clusters -> { -for (String cluster : clusters) { -if (!cluster.equals(pulsar().getConfiguration().getClusterName())) { -// this call happens in the background without async composition. completion is logged. -pulsar().getPulsarResources().getClusterResources() -.getClusterAsync(cluster) -.thenCompose(clusterDataOp -> -((TopicsImpl) pulsar().getBrokerService() - .getClusterPulsarAdmin(cluster, - clusterDataOp).topics()) - .createPartitionedTopicAsync( - topicName.getPartitionedTopicName(), -numPartitions, -true, null)) -.whenComplete((__, ex) -> { -if (ex != null) { -log.error( -"[{}] Failed to create partitioned topic {} in cluster {}.", -clientAppId(), topicName, cluster, ex); -} else { -log.info( -"[{}] Successfully created partitioned topic {} in " -+ "cluster {}", -clientAppId(), topicName, cluster); -} -}); -} +.thenAccept(clusters -> { +// this call happens in the background without async composition. completion is logged. + internalCreatePartitionedTopicToReplicatedClustersInBackground(clusters, numPartitions); +}); +} + +protected Map> internalCreatePartitionedTopicToReplicatedClustersInBackground( +Set clusters, int numPartitions) { +final String shortTopicName = topicName.getPartitionedTopicName(); +Map> tasksForAllClusters = new HashMap<>(); +for (String cluster : clusters) { +if (cluster.equals(pulsar().getConfiguration().getClusterName())) { +continue; +} +ClusterResources clusterResources = pulsar().getPulsarResources().getClusterResources(); +CompletableFuture createRemoteTopicFuture = new CompletableFuture<>(); +tasksForAllClusters.put(cluster, createRemoteTopicFuture); + clusterResources.getClusterAsync(cluster).whenComplete((clusterData, ex1) -> { +if (ex1 != null) { +// Unexpected error, such as NPE. Catch all error to avoid the "createRemoteTopicFuture" stuck. +log.error("[{}] An un-expected error occurs when trying to create partitioned topic {} in cluster" ++ " {}.", clientAppId(), topicName, cluster, ex1); +createRemoteTopicFuture.completeExceptionally(new RestException(ex1)); +return; +} +// Get cluster data success. +TopicsImpl topics = +(TopicsImpl) pulsar().getBrokerService().getClusterPulsarAdmin(cluster, clusterData).topics(); +topics.createPartitionedTopicAsync(shortTopicName, numPartitions, true, null) +.whenComplete((ignore, ex2) -> { +if (ex2 == null) { +// Create success. +log.info("[{}] Successfully created partitioned topic {} in cluster {}", +clientAppId(), topicName, cluster); +createRemoteTopicFuture.complete(null); +return; +} +// Create topic on the remote cluster error. +Throwable unwrapEx2 = FutureUtil.unwrapCompletionException(ex2); +// The topic has been created before, check
Re: [PR] [improve][broker] Make BrokerSelectionStrategy pluggable [pulsar]
BewareMyPower merged PR #22553: URL: https://github.com/apache/pulsar/pull/22553 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [improve][broker] Make BrokerSelectionStrategy pluggable (#22553)
This is an automated email from the ASF dual-hosted git repository. xyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new 89b201ed8a4 [improve][broker] Make BrokerSelectionStrategy pluggable (#22553) 89b201ed8a4 is described below commit 89b201ed8a49877e0a7148b060af945b29074b02 Author: Yunze Xu AuthorDate: Tue Apr 23 18:52:49 2024 +0800 [improve][broker] Make BrokerSelectionStrategy pluggable (#22553) --- .../extensions/ExtensibleLoadManagerImpl.java | 11 ++- .../strategy/BrokerSelectionStrategy.java | 2 + ...gy.java => BrokerSelectionStrategyFactory.java} | 24 +- .../CustomBrokerSelectionStrategyTest.java | 86 ++ 4 files changed, 100 insertions(+), 23 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java index a20694356b1..41832fb6007 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java @@ -85,6 +85,7 @@ import org.apache.pulsar.broker.loadbalance.extensions.store.LoadDataStore; import org.apache.pulsar.broker.loadbalance.extensions.store.LoadDataStoreException; import org.apache.pulsar.broker.loadbalance.extensions.store.LoadDataStoreFactory; import org.apache.pulsar.broker.loadbalance.extensions.strategy.BrokerSelectionStrategy; +import org.apache.pulsar.broker.loadbalance.extensions.strategy.BrokerSelectionStrategyFactory; import org.apache.pulsar.broker.loadbalance.extensions.strategy.LeastResourceUsageWithWeight; import org.apache.pulsar.broker.loadbalance.impl.LoadManagerShared; import org.apache.pulsar.broker.loadbalance.impl.SimpleResourceAllocationPolicies; @@ -104,7 +105,7 @@ import org.apache.pulsar.metadata.api.coordination.LeaderElectionState; import org.slf4j.Logger; @Slf4j -public class ExtensibleLoadManagerImpl implements ExtensibleLoadManager { +public class ExtensibleLoadManagerImpl implements ExtensibleLoadManager, BrokerSelectionStrategyFactory { public static final String BROKER_LOAD_DATA_STORE_TOPIC = TopicName.get( TopicDomain.non_persistent.value(), @@ -252,6 +253,11 @@ public class ExtensibleLoadManagerImpl implements ExtensibleLoadManager { return ownedServiceUnits; } +@Override +public BrokerSelectionStrategy createBrokerSelectionStrategy() { +return new LeastResourceUsageWithWeight(); +} + public enum Role { Leader, Follower @@ -267,8 +273,7 @@ public class ExtensibleLoadManagerImpl implements ExtensibleLoadManager { this.brokerFilterPipeline.add(new BrokerLoadManagerClassFilter()); this.brokerFilterPipeline.add(new BrokerMaxTopicCountFilter()); this.brokerFilterPipeline.add(new BrokerVersionFilter()); -// TODO: Make brokerSelectionStrategy configurable. -this.brokerSelectionStrategy = new LeastResourceUsageWithWeight(); +this.brokerSelectionStrategy = createBrokerSelectionStrategy(); } public static boolean isLoadManagerExtensionEnabled(PulsarService pulsar) { diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java index e0a9122383c..b240cb5b5f6 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java @@ -21,11 +21,13 @@ package org.apache.pulsar.broker.loadbalance.extensions.strategy; import java.util.Optional; import java.util.Set; import org.apache.pulsar.broker.loadbalance.extensions.LoadManagerContext; +import org.apache.pulsar.common.classification.InterfaceStability; import org.apache.pulsar.common.naming.ServiceUnitId; /** * The broker selection strategy is designed to select the broker according to different implementations. */ +@InterfaceStability.Evolving public interface BrokerSelectionStrategy { /** diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategyFactory.java similarity index 51% copy from pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/BrokerSelectionStrategy.java copy to pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbala
[PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
BewareMyPower opened a new pull request, #22567: URL: https://github.com/apache/pulsar/pull/22567 ### Motivation In a private fork repository, the precondition workflow of a PR will always fail due to the lack of the permission to fetch the content of that PR. ``` Fetching list of changed files for PR#208 from Github API Invoking listFiles(pull_number: 208, per_page: 100) Error: Resource not accessible by integration ``` ### Modifications Only run the "Detect changed files" step and steps that depend on that step in the Apache repo. ### Verifying this change After applying this patch, the precondition workflows succeeded and the tests were executed. https://github.com/apache/pulsar/assets/18204803/c5388604-c0fe-4f33-8566-412c02c7e2f9";> ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][meta] Log a warning when ZK batch fails with connectionloss [pulsar]
lhotari commented on PR #22566: URL: https://github.com/apache/pulsar/pull/22566#issuecomment-2072024722 I might have encountered this issue recently when working on the /metrics endpoint issue. A few times I got errors such as `java.io.IOException: Packet len 15589885 is out of range!` without any other proper explanation. More details of test setup in #22477 and #22494 comments. It seems that https://github.com/lhotari/pulsar-playground/blob/master/src/main/java/com/github/lhotari/pulsar/playground/TestScenarioCreateLongNamedTopics.java and https://github.com/lhotari/pulsar-playground/blob/master/src/main/java/com/github/lhotari/pulsar/playground/TestScenarioLoadAll.java would reproduce the problem I faced. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
lhotari commented on PR #22567: URL: https://github.com/apache/pulsar/pull/22567#issuecomment-2072051995 Is there a way where explicit job permissions specified with https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs would address the issue? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [D] Pulsar 3.2.2 correct Message to InfluxDBv2 sink connector with Python Producer [pulsar]
GitHub user smuehlmann edited a discussion: Pulsar 3.2.2 correct Message to InfluxDBv2 sink connector with Python Producer Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. With a java test code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [PR] [improve][broker] Optimize topic production rate stats. [pulsar]
hanmz closed pull request #22518: [improve][broker] Optimize topic production rate stats. URL: https://github.com/apache/pulsar/pull/22518 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][broker] Optimize topic production rate stats. [pulsar]
hanmz commented on PR #22518: URL: https://github.com/apache/pulsar/pull/22518#issuecomment-2072064765 > @hanmz This is a long-standing issue and the plan was to address as part of the overall changes of [PIP-264](https://github.com/apache/pulsar/blob/master/pip/pip-264.md). I wouldn't introduce a fix for just 1 metric with one dedicated config option when these metrics are going away soon. @dragosvictor has already started converting the metrics. If the community will solve this problem in the near future, this PR will not be introduced. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [D] Pulsar 3.2.2 Message for InfluxDBv2 sink connector with Python Producer [pulsar]
GitHub user smuehlmann edited a discussion: Pulsar 3.2.2 Message for InfluxDBv2 sink connector with Python Producer Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. With a java test code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [D] Python Producer Message to InfluxDBv2 sink connector with [pulsar]
GitHub user smuehlmann edited a discussion: Python Producer Message to InfluxDBv2 sink connector with Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. With a java test code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [D] Python Producer Message to InfluxDBv2 sink connector with [pulsar]
GitHub user smuehlmann edited a discussion: Python Producer Message to InfluxDBv2 sink connector with Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. Especially the message content / payload in combination with the used producer schema is my problem. The Pulsar Version is 3.2.2. With java code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [D] Python Producer Message to InfluxDBv2 sink connector [pulsar]
GitHub user smuehlmann edited a discussion: Python Producer Message to InfluxDBv2 sink connector Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. Especially the message content / payload in combination with the used producer schema is my problem. The Pulsar Version is 3.2.2. With java code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [D] Python Producer Message to InfluxDB v2 sink connector [pulsar]
GitHub user smuehlmann edited a discussion: Python Producer Message to InfluxDB v2 sink connector Hi, i have problems to send a message with a python producer which should be processed by an InfluxDBv2 sink connector. Especially the message content / payload in combination with the used producer schema is my problem. The Pulsar Version is 3.2.2. With java code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [D] Python Producer Message to InfluxDB v2 sink connector [pulsar]
GitHub user smuehlmann edited a discussion: Python Producer Message to InfluxDB v2 sink connector Hi, i have problems to send a message with a python producer which should be processed by an InfluxDB v2 sink connector. Especially the message content / payload in combination with the used producer schema is my problem. The Pulsar Version is 3.2.2. With java code it is possible. Java class: public class Device { public String measurement; public long timestamp; public Map tags; public Map fields; } Set values: Device device = new Device(); device.measurement = "test5"; device.timestamp = Instant.now().toEpochMilli(); device.tags = Maps.newHashMap(); device.tags.put("foo", "bar"); device.fields = Maps.newHashMap(); device.fields.put("temp", 14.8); Create producer: Producer producer = client.newProducer(JSONSchema.of(Device.class)) .topic("persistent://public/default/foobar") .producerName("test") .create(); Send message: MessageId msgID = producer.newMessage() .key("test") .value(device) .send(); Any suggestions how to do with Python ? Thanks GitHub link: https://github.com/apache/pulsar/discussions/22558 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
BewareMyPower commented on PR #22567: URL: https://github.com/apache/pulsar/pull/22567#issuecomment-2072080615 > The problem with the changes in this PR is that it would make it harder to develop the workflow in a public forked repository. The main affect is the PRs that only modify the documents will still trigger the unit tests. However, as the `pulsar-site` repository was already moved out of the main repo, such PRs are becoming much less than before. So I don't think it actually makes development harder in a public forked repository. I'm also suspicious about if there is a real developer that gets benefits from such savings of GitHub actions resources. On the other hand, without this PR or another solution, the development in a private fork could be very hard. A PR needs to modify the workflow or the master branch needs to include a commit that removes these workflows to have tests executed. > We should first investigate if there's a way to add permissions to the workflow. Technically it's right. But the [`paths-filter` plugin](https://github.com/apache/pulsar-test-infra/commits/master/paths-filter) is hard to maintain for contributors not familiar with JavaScript and GitHub actions. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
lhotari commented on PR #22567: URL: https://github.com/apache/pulsar/pull/22567#issuecomment-2072183428 > Technically it's right. But the [`paths-filter` plugin](https://github.com/apache/pulsar-test-infra/commits/master/paths-filter) is hard to maintain for contributors not familiar with JavaScript and GitHub actions. there's no need to modify the paths-filter action when defining [GitHub Actions workflow or job permissions](https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs). It's possible that private repositories have a setting that makes the default permissions just work without any changes. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
BewareMyPower commented on PR #22567: URL: https://github.com/apache/pulsar/pull/22567#issuecomment-2072194156 I also found another permission issue in the **Semantic Pull Request / Check pull request title** workflow though it does not block the development. ``` Run amannn/action-semantic-pull-request@v5.0.2 Error: Resource not accessible by integration ``` https://github.com/apache/pulsar/assets/18204803/d0cf0774-6a66-48cc-9415-48b650a52ad1";> Mark this PR as drafted first because I don't have much time on it. As a workaround to unblock the development, I will commit this patch in an independent development branch in the private forked repository. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [improve][ci] Add explicit GitHub Actions permissions [pulsar]
lhotari opened a new pull request, #22568: URL: https://github.com/apache/pulsar/pull/22568 ### Motivation This is required when running GitHub Actions in a fork that has been configured with restrictive access. - [GitHub Actions workflow or job permissions](https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs). - [the default permissions](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token) Explained in "[Enforcing a policy for workflow permissions in your enterprise](https://docs.github.com/en/enterprise-cloud@latest/admin/policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-github-actions-in-your-enterprise#enforcing-a-policy-for-workflow-permissions-in-your-enterprise)": > Anyone with write access to a repository can modify the permissions granted to the GITHUB_TOKEN, adding or removing access as required, by editing the permissions key in the workflow file. For more information, see [permissions](https://docs.github.com/en/enterprise-cloud@latest/actions/using-workflows/workflow-syntax-for-github-actions#permissions). ### Modifications Add explicit "permissions" to each workflow. ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
lhotari commented on PR #22567: URL: https://github.com/apache/pulsar/pull/22567#issuecomment-2072261695 experiment in #22568 in draft mode -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][ci] Skip detecting changed files in fork repositories [pulsar]
BewareMyPower closed pull request #22567: [improve][ci] Skip detecting changed files in fork repositories URL: https://github.com/apache/pulsar/pull/22567 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar-dotpulsar) branch master updated: Added traits to tests. Now using Pulsar 3.1.3 for tests
This is an automated email from the ASF dual-hosted git repository. blankensteiner pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar-dotpulsar.git The following commit(s) were added to refs/heads/master by this push: new 4a05f01 Added traits to tests. Now using Pulsar 3.1.3 for tests 4a05f01 is described below commit 4a05f019a0b0cd008d63d3d1533776e50e49770f Author: Daniel Blankensteiner AuthorDate: Tue Apr 23 15:25:08 2024 +0200 Added traits to tests. Now using Pulsar 3.1.3 for tests --- tests/DotPulsar.Tests/IntegrationFixture.cs| 10 +- .../DotPulsar.Tests/Internal/RoundRobinPartitionRouterTests.cs | 3 ++- tests/DotPulsar.Tests/Internal/SinglePartitionRouterTests.cs | 3 ++- tests/DotPulsar.Tests/PulsarClientTests.cs | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/DotPulsar.Tests/IntegrationFixture.cs b/tests/DotPulsar.Tests/IntegrationFixture.cs index 9dec12f..cf882d9 100644 --- a/tests/DotPulsar.Tests/IntegrationFixture.cs +++ b/tests/DotPulsar.Tests/IntegrationFixture.cs @@ -78,7 +78,7 @@ public class IntegrationFixture : IAsyncLifetime .Build(); _pulsarCluster = new ContainerBuilder() -.WithImage("apachepulsar/pulsar:3.1.1") +.WithImage("apachepulsar/pulsar:3.1.3") .WithEnvironment(environmentVariables) .WithHostname("pulsar") .WithNetwork(_network) @@ -153,12 +153,12 @@ public class IntegrationFixture : IAsyncLifetime if (expiryTime != Timeout.InfiniteTimeSpan) arguments += $" --expiry-time {expiryTime.TotalSeconds}s"; -var result = await _pulsarCluster.ExecAsync(new[] { "/bin/bash", "-c", arguments }, cancellationToken); +var result = await _pulsarCluster.ExecAsync(["/bin/bash", "-c", arguments], cancellationToken); if (result.ExitCode != 0) throw new InvalidOperationException($"Could not create the token: {result.Stderr}"); -return result.Stdout; +return result.Stdout.Trim(); } private static string CreateTopicName() => $"persistent://public/default/{Guid.NewGuid():N}"; @@ -174,7 +174,7 @@ public class IntegrationFixture : IAsyncLifetime { var arguments = $"bin/pulsar-admin topics create {topic}"; -var result = await _pulsarCluster.ExecAsync(new[] { "/bin/bash", "-c", arguments }, cancellationToken); +var result = await _pulsarCluster.ExecAsync(["/bin/bash", "-c", arguments], cancellationToken); if (result.ExitCode != 0) throw new Exception($"Could not create the topic: {result.Stderr}"); @@ -191,7 +191,7 @@ public class IntegrationFixture : IAsyncLifetime { var arguments = $"bin/pulsar-admin topics create-partitioned-topic {topic} -p {numberOfPartitions}"; -var result = await _pulsarCluster.ExecAsync(new[] { "/bin/bash", "-c", arguments }, cancellationToken); +var result = await _pulsarCluster.ExecAsync(["/bin/bash", "-c", arguments], cancellationToken); if (result.ExitCode != 0) throw new Exception($"Could not create the partitioned topic: {result.Stderr}"); diff --git a/tests/DotPulsar.Tests/Internal/RoundRobinPartitionRouterTests.cs b/tests/DotPulsar.Tests/Internal/RoundRobinPartitionRouterTests.cs index 681cae0..81181b4 100644 --- a/tests/DotPulsar.Tests/Internal/RoundRobinPartitionRouterTests.cs +++ b/tests/DotPulsar.Tests/Internal/RoundRobinPartitionRouterTests.cs @@ -1,5 +1,6 @@ -namespace DotPulsar.Tests.Internal; +namespace DotPulsar.Tests.Internal; +[Trait("Category", "Unit")] public class RoundRobinPartitionRouterTests { [Fact] diff --git a/tests/DotPulsar.Tests/Internal/SinglePartitionRouterTests.cs b/tests/DotPulsar.Tests/Internal/SinglePartitionRouterTests.cs index 0a86ce1..899b258 100644 --- a/tests/DotPulsar.Tests/Internal/SinglePartitionRouterTests.cs +++ b/tests/DotPulsar.Tests/Internal/SinglePartitionRouterTests.cs @@ -1,5 +1,6 @@ -namespace DotPulsar.Tests.Internal; +namespace DotPulsar.Tests.Internal; +[Trait("Category", "Unit")] public class SinglePartitionRouterTests { [Fact] diff --git a/tests/DotPulsar.Tests/PulsarClientTests.cs b/tests/DotPulsar.Tests/PulsarClientTests.cs index f90e8c8..77b924a 100644 --- a/tests/DotPulsar.Tests/PulsarClientTests.cs +++ b/tests/DotPulsar.Tests/PulsarClientTests.cs @@ -59,7 +59,7 @@ public sealed class PulsarClientTests : IDisposable if (throwException) throw new Exception(); var token = await _fixture.CreateToken(TimeSpan.FromSeconds(10), _cts.Token); -_testOutputHelper.Log($"Received token: {token}"); +_testOutputHelper.Log($"Received token: '{token}'"); return token; }); @@ -110,7 +110,7 @@ public sealed class PulsarClientTests : IDisposable tcs.SetResult();
Re: [I] [Bug] Unload with destinationBroker could fail with 504 with extensible load manager [pulsar]
BewareMyPower closed issue #22136: [Bug] Unload with destinationBroker could fail with 504 with extensible load manager URL: https://github.com/apache/pulsar/issues/22136 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Bug] Broker could take 30+ seconds to close with extensible load manager [pulsar]
BewareMyPower opened a new issue, #22569: URL: https://github.com/apache/pulsar/issues/22569 ### Search before asking - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. ### Read release policy - [X] I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker. ### Version master (89b201ed8a49877e0a7148b060af945b29074b02) ### Minimal reproduce step ```java public class ExtensibleLoadManagerCloseTest extends MultiBrokerBaseTest { @Override protected void startBroker() throws Exception { addCustomConfigs(conf); super.startBroker(); } @Override protected ServiceConfiguration createConfForAdditionalBroker(int additionalBrokerIndex) { return addCustomConfigs(getDefaultConf()); } private static ServiceConfiguration addCustomConfigs(ServiceConfiguration config) { config.setLoadManagerClassName(ExtensibleLoadManagerImpl.class.getName()); config.setDefaultNumberOfNamespaceBundles(16); config.setLoadBalancerAutoBundleSplitEnabled(false); config.setLoadBalancerDebugModeEnabled(true); return config; } @Test public void test() throws Exception { final var topic = "test"; admin.topics().createPartitionedTopic(topic, 20); admin.lookups().lookupPartitionedTopic(topic); @Cleanup final var client = PulsarClient.builder().serviceUrl(pulsar.getBrokerServiceUrl()).build(); @Cleanup final var producer = (PartitionedProducerImpl) client.newProducer().topic(topic).create(); } } ``` ### What did you expect to see? It should not takes too long to complete. ### What did you see instead? It sometimes took about 54 seconds to complete and sometimes 22 seconds. ### Anything else? _No response_ ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- 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-unsubscr...@pulsar.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug] broker healthcheck ran into loop after decommissioned a cluster of bookies [pulsar]
wallacepeng commented on issue #22559: URL: https://github.com/apache/pulsar/issues/22559#issuecomment-2072404304 @hpvd we are downgrading the storage so provisioned the bookkeeper cluster. we will upgrade the cluster a bit later. is there any way to clean the healthcheck topic it looks like the ledger cached old bookies ? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][admin] Fix can't delete tenant for v1 [pulsar]
merlimat merged PR #22550: URL: https://github.com/apache/pulsar/pull/22550 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [fix][admin] Fix can't delete tenant for v1 (#22550)
This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new d5c72312ff4 [fix][admin] Fix can't delete tenant for v1 (#22550) d5c72312ff4 is described below commit d5c72312ff4d03291e1ea2eb37464250c85bf401 Author: Jiwei Guo AuthorDate: Tue Apr 23 22:04:13 2024 +0800 [fix][admin] Fix can't delete tenant for v1 (#22550) --- .../pulsar/broker/resources/TopicResources.java| 2 +- .../pulsar/broker/auth/AuthorizationTest.java | 29 ++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/TopicResources.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/TopicResources.java index 0963f25c3d3..413184764f5 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/TopicResources.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/TopicResources.java @@ -120,7 +120,7 @@ public class TopicResources { return store.exists(path) .thenCompose(exists -> { if (exists) { -return store.delete(path, Optional.empty()); +return store.deleteRecursive(path); } else { return CompletableFuture.completedFuture(null); } diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java index 01bfd03ceb8..f59f9d480b8 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java @@ -33,6 +33,7 @@ import org.apache.pulsar.broker.authorization.AuthorizationService; import org.apache.pulsar.broker.resources.PulsarResources; import org.apache.pulsar.client.admin.PulsarAdmin; import org.apache.pulsar.client.admin.PulsarAdminBuilder; +import org.apache.pulsar.client.api.ClientBuilder; import org.apache.pulsar.common.naming.TopicDomain; import org.apache.pulsar.common.naming.TopicName; import org.apache.pulsar.common.policies.data.AuthAction; @@ -56,12 +57,17 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest { @Override public void setup() throws Exception { conf.setClusterName("c1"); +conf.setSystemTopicEnabled(false); conf.setAuthenticationEnabled(true); +conf.setForceDeleteNamespaceAllowed(true); +conf.setForceDeleteTenantAllowed(true); conf.setAuthenticationProviders( Sets.newHashSet("org.apache.pulsar.broker.auth.MockAuthenticationProvider")); conf.setAuthorizationEnabled(true); conf.setAuthorizationAllowWildcardsMatching(true); conf.setSuperUserRoles(Sets.newHashSet("pulsar.super_user", "pass.pass")); + conf.setBrokerClientAuthenticationPlugin(MockAuthentication.class.getName()); +conf.setBrokerClientAuthenticationParameters("user:pass.pass"); internalSetup(); } @@ -70,6 +76,11 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest { pulsarAdminBuilder.authentication(new MockAuthentication("pass.pass")); } +@Override +protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) { +clientBuilder.authentication(new MockAuthentication("pass.pass")); +} + @AfterClass(alwaysRun = true) @Override public void cleanup() throws Exception { @@ -233,6 +244,24 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest { admin.namespaces().deleteNamespace("p1/c1/ns1"); admin.tenants().deleteTenant("p1"); + +admin.clusters().deleteCluster("c1"); +} + +@Test +public void testDeleteV1Tenant() throws Exception { +admin.clusters().createCluster("c1", ClusterData.builder().build()); +admin.tenants().createTenant("p1", new TenantInfoImpl(Sets.newHashSet("role1"), Sets.newHashSet("c1"))); +waitForChange(); +admin.namespaces().createNamespace("p1/c1/ns1"); +waitForChange(); + + +String topic = "persistent://p1/c1/ns1/ds2"; +admin.topics().createNonPartitionedTopic(topic); + +admin.namespaces().deleteNamespace("p1/c1/ns1", true); +admin.tenants().deleteTenant("p1", true); admin.clusters().deleteCluster("c1"); }
Re: [PR] [improve][meta] Log a warning when ZK batch fails with connectionloss [pulsar]
lhotari commented on PR #22566: URL: https://github.com/apache/pulsar/pull/22566#issuecomment-2072410563 The test https://github.com/apache/pulsar/blob/61296d90d912493113d7a18c18eef23114810ce7/pulsar-metadata/src/test/java/org/apache/pulsar/metadata/MetadataStoreBatchingTest.java#L151-L182 will reproduce the problem. it will log "2024-04-23T17:03:51,416 - WARN - [main-EventThread:ZKMetadataStore@204] - Connection loss while executing batch operation of 40 GET entries of total data size of 910. Retrying individual operations one-by-one." with the changes in this PR. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][meta] Log a warning when ZK batch fails with connectionloss [pulsar]
lhotari commented on PR #22566: URL: https://github.com/apache/pulsar/pull/22566#issuecomment-2072415742 It seems that batching reads could cause more harm than benefit when the returned data exceeds jute.maxbuffer size. This impacts stability a lot so there should be a way to disable batch reads completely. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576335249 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: Sorry I didn't get it. Could you explain it with more details? > if the initial connection attempt fails with the given assigned URL Did you mean `p.grabCnx(assignedBrokerURL)` failed? If so, `reconnectToBroker` would just return and would be called when the next time `ConnectionClosed` is called. How can assigning a local variable (`connectionClosed`) to nil just resume the established reconnection? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
dragosvictor commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576507047 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: There is a for loop wrapping around the reconnection attempts: ```go for maxRetry != 0 { if p.getProducerState() != producerReady { // Producer is already closing p.log.Info("producer state not ready, exit reconnect") return } var assignedBrokerURL string if connectionClosed != nil && connectionClosed.HasURL() { delayReconnectTime = 0 assignedBrokerURL = connectionClosed.assignedBrokerURL connectionClosed = nil // Only attempt once } else if p.options.BackoffPolicy == nil { delayReconnectTime = defaultBackoff.Next() } else { delayReconnectTime = p.options.BackoffPolicy.Next() } ... ``` Setting the variable to nil guarantees we will only attempt connection to the assigned broker once. If that fails, for whatever reason, we revert back to performing topic lookups in order to locate the broker serving the 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
dragosvictor commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576510648 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: My understanding here is that if `grabCnx` fails, the loop handles the error by increasing the reconnect delay and trying up to a configurable number of times, before giving up. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
dragosvictor commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576510648 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: My understanding here is that if `grabCnx` fails, the loop handles the error by increasing the reconnect delay and retrying up to a configurable number of times, before giving up. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576520011 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: Oh I see. I was so blind that I missed the `for maxRetry != 0` loop before. (BTW, the loop is too long) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576526236 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: How about moving the `assignedBrokerURL` out of the for loop? ```go var assignedBrokerURL string if connectionClosed != nil && connectionClosed.HasURL() { assignedBrokerURL = connectionClosed.assignedBrokerURL } for maxRetry != 0 { // ... if p.options.BackoffPolicy == nil { delayReconnectTime = defaultBackoff.Next() } else { delayReconnectTime = p.options.BackoffPolicy.Next() } ``` When `connectionClosed` is nil, `assignedBrokerURL` will be an empty string and `grabCnx(assignedBrokerURL)` will be equivalent with `grabCnx("")`. The code will look more clear and less code changes are needed. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576526236 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: How about moving the `assignedBrokerURL` out of the for loop? ```go var assignedBrokerURL string if connectionClosed != nil && connectionClosed.HasURL() { assignedBrokerURL = connectionClosed.assignedBrokerURL } for maxRetry != 0 { // ... if p.options.BackoffPolicy == nil { delayReconnectTime = defaultBackoff.Next() } else { delayReconnectTime = p.options.BackoffPolicy.Next() } // ... err := p.grabCnx(assignedBrokerURL) ``` When `connectionClosed` is nil, `assignedBrokerURL` will be an empty string and `grabCnx(assignedBrokerURL)` will be equivalent with `grabCnx("")`. The code will look more clear and less code changes are needed. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576526236 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: How about moving the `assignedBrokerURL` out of the for loop? ```go assignedBrokerURL = "" if connectionClosed != nil && connectionClosed.HasURL() { assignedBrokerURL = connectionClosed.assignedBrokerURL } for maxRetry != 0 { // ... if p.options.BackoffPolicy == nil { delayReconnectTime = defaultBackoff.Next() } else { delayReconnectTime = p.options.BackoffPolicy.Next() } // ... err := p.grabCnx(assignedBrokerURL) ``` When `connectionClosed` is nil, `assignedBrokerURL` will be an empty string and `grabCnx(assignedBrokerURL)` will be equivalent with `grabCnx("")`. The code will look more clear and less code changes are needed. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
dragosvictor commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576544492 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: `assignedBrokerUrl` being set to empty is what signals to `grabCnx` to execute the lookup further on ([ref](https://github.com/apache/pulsar-client-go/pull/1208/files#diff-8adb09af2175be5751840343e14df2220a2154ca4bc8de21157ed112be25b6b9R224-R239)), so it cannot be fixed outside of the loop and never changed subsequently. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower commented on code in PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208#discussion_r1576550511 ## pulsar/producer_partition.go: ## @@ -429,12 +453,22 @@ func (p *partitionProducer) reconnectToBroker() { return } - if p.options.BackoffPolicy == nil { + var assignedBrokerURL string + + if connectionClosed != nil && connectionClosed.HasURL() { + delayReconnectTime = 0 + assignedBrokerURL = connectionClosed.assignedBrokerURL + connectionClosed = nil // Only attempt once Review Comment: I see, when the client failed to connect the assigned broker URL, it needs to call `grabCnx("")` for the lookup. It makes sense to me. Thanks! -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] PIP-307: Use assigned broker URL hints during broker reconnection [pulsar-client-go]
BewareMyPower merged PR #1208: URL: https://github.com/apache/pulsar-client-go/pull/1208 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576580042 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack +The total number of message acknowledgments received for this topic. +* Type: Counter +* Unit: `{ack}` + + pulsar.broker.topic.storage.size +The total storage size of the messages in this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.logical.size +The storage size of topics in the namespace owned by the broker without replicas. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.size +The total backlog size of the topics of this topic owned by this broker. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.offloaded.size +The total amount of the data in this topic offloaded to the tiered storage. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.size +The size based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.time +The time based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{second}` + + pulsar.broker.topic.storage.backlog.quota.eviction.count +The number of times a backlog was evicted since it has exceeded its quota. +* Type: Counter +* Unit: `{eviction}` + + pulsar.broker.topic.storage.backlog.age +The age of the oldest unacknowledged message (backlog). +* Type: Gauge +* Unit: `{second}` + + pulsar.broker.topic.storage.outgoing +The total message batches (entries) written to the storage for this topic. +* Type: UpDownCounter +* Unit: `{message batch}` + + pulsar.broker.topic.storage.incoming +The total message batches (entries) read from the storage for this topic. +* Type: UpDownCounter +* Unit: `{message batch}` + + pulsar.broker.topic.compaction.removed.event.count +The total number of removed events of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.succeed.count +The total number of successes of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.failed.count +The total number of failures of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.duration +The total time duration of compaction operations on the topic. +* Type: DoubleUpDownCounter +* Unit: `{second}` + + pulsar.broker.topic.compaction.incoming +The total count of bytes read by the compaction process for this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.compaction.outgoing +The total count of bytes written by the compaction process for this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.compaction.compacted.entries.count +The total number of compacted entries. +* Type: UpDownCounter +* Unit: `{entry}` + + pulsar.broker.topic.compaction.compacted.entries.size +The total size of the compacted entries. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.transaction +The number of transactions on this topic. +* Type: UpDownCounter Review Comment: This metric has attributes for the tx status ([ref](https://github.com/apache/pulsar/pull/22467/files#diff-e3d8386fc3f9292780771f4eae19d9297637d374244f9c4b3f89b6c0e442e2e3R458-R470)): `active`, `committed`, `aborted`. The latter two can be counters, but not the first one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576582036 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack +The total number of message acknowledgments received for this topic. +* Type: Counter +* Unit: `{ack}` + + pulsar.broker.topic.storage.size +The total storage size of the messages in this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.logical.size +The storage size of topics in the namespace owned by the broker without replicas. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.size +The total backlog size of the topics of this topic owned by this broker. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.offloaded.size +The total amount of the data in this topic offloaded to the tiered storage. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.size +The size based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.time +The time based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{second}` + + pulsar.broker.topic.storage.backlog.quota.eviction.count +The number of times a backlog was evicted since it has exceeded its quota. +* Type: Counter +* Unit: `{eviction}` + + pulsar.broker.topic.storage.backlog.age +The age of the oldest unacknowledged message (backlog). +* Type: Gauge +* Unit: `{second}` + + pulsar.broker.topic.storage.outgoing +The total message batches (entries) written to the storage for this topic. +* Type: UpDownCounter +* Unit: `{message batch}` + + pulsar.broker.topic.storage.incoming +The total message batches (entries) read from the storage for this topic. +* Type: UpDownCounter +* Unit: `{message batch}` + + pulsar.broker.topic.compaction.removed.event.count +The total number of removed events of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.succeed.count +The total number of successes of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.failed.count +The total number of failures of the compaction. +* Type: UpDownCounter +* Unit: `{event}` + + pulsar.broker.topic.compaction.duration +The total time duration of compaction operations on the topic. +* Type: DoubleUpDownCounter +* Unit: `{second}` + + pulsar.broker.topic.compaction.incoming +The total count of bytes read by the compaction process for this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.compaction.outgoing +The total count of bytes written by the compaction process for this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.compaction.compacted.entries.count +The total number of compacted entries. +* Type: UpDownCounter +* Unit: `{entry}` + + pulsar.broker.topic.compaction.compacted.entries.size +The total size of the compacted entries. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.transaction +The number of transactions on this topic. +* Type: UpDownCounter +* Unit: `{transaction}` + + pulsar.broker.topic.subscription.delayed +The total message batches (entries) are delayed for dispatching. +* Type: UpDownCounter Review Comment: This number can fluctuate up and down, it does not reflect a running total. -- 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 comm
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576586529 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics Review Comment: The implementation https://github.com/apache/pulsar/pull/22467 uses asynchronous instruments, due to performance and resource efficiency reasons (more details in that PR). This is a detail, however, so it doesn't need to be in the metrics reference documentation. Furthermore, from the users point of view, this is transparent. Instrument types do not impact the way the user needs to collect or interpret these values. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576587699 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack +The total number of message acknowledgments received for this topic. Review Comment: This particular metric is not currently populated by the implementation. I'll remove it from the doc until it is. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576589918 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack Review Comment: Removed, as explained above. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576586529 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics Review Comment: The implementation https://github.com/apache/pulsar/pull/22467 uses asynchronous instruments, due to performance and resource efficiency reasons (more details in that PR). This is a detail, however, so it doesn't need to be in the metrics reference documentation. Furthermore, from the users point of view, this is transparent ([ref](https://opentelemetry.io/docs/specs/semconv/general/metrics/#instrument-types)). Instrument types do not impact the way the user needs to collect or interpret these values. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576606808 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + Review Comment: There's some recommendations: https://opentelemetry.io/docs/specs/semconv/general/metrics/#use-count-instead-of-pluralization-for-updowncounters. I'll add the suffixes where suggested, sure! -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][meta] Log a warning when ZK batch fails with connectionloss [pulsar]
heesung-sn commented on code in PR #22566: URL: https://github.com/apache/pulsar/pull/22566#discussion_r1576672640 ## pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/ZKMetadataStore.java: ## @@ -192,7 +192,20 @@ protected void batchOperation(List ops) { Code code = Code.get(rc); if (code == Code.CONNECTIONLOSS) { // There is the chance that we caused a connection reset by sending or requesting a batch -// that passed the max ZK limit. Retry with the individual operations +// that passed the max ZK limit. + +// Build the log warning message +// summarize the operations by type +String countsByType = ops.stream().collect( + Collectors.groupingBy(MetadataOp::getType, Collectors.summingInt(op -> 1))) +.entrySet().stream().map(e -> e.getValue() + " " + e.getKey().name() + " entries") +.collect(Collectors.joining(", ")); +Long totalSize = ops.stream().collect(Collectors.summingLong(MetadataOp::size)); +log.warn("Connection loss while executing batch operation of {} " Review Comment: Nit: Can we also log the max zk buffer size compared to the data size? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve][meta] Log a warning when ZK batch fails with connectionloss [pulsar]
heesung-sn commented on code in PR #22566: URL: https://github.com/apache/pulsar/pull/22566#discussion_r1576674865 ## pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/ZKMetadataStore.java: ## @@ -192,7 +192,20 @@ protected void batchOperation(List ops) { Code code = Code.get(rc); if (code == Code.CONNECTIONLOSS) { // There is the chance that we caused a connection reset by sending or requesting a batch -// that passed the max ZK limit. Retry with the individual operations +// that passed the max ZK limit. + +// Build the log warning message +// summarize the operations by type +String countsByType = ops.stream().collect( + Collectors.groupingBy(MetadataOp::getType, Collectors.summingInt(op -> 1))) +.entrySet().stream().map(e -> e.getValue() + " " + e.getKey().name() + " entries") +.collect(Collectors.joining(", ")); +Long totalSize = ops.stream().collect(Collectors.summingLong(MetadataOp::size)); +log.warn("Connection loss while executing batch operation of {} " Review Comment: Also, ideally it would be great if we can emit some metics too. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576716453 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack +The total number of message acknowledgments received for this topic. +* Type: Counter +* Unit: `{ack}` + + pulsar.broker.topic.storage.size +The total storage size of the messages in this topic. Review Comment: I don't see offloaded ledgers being subtracted from `ManagedLedgerImpl.totalSize` ([ref](https://github.com/apache/pulsar/blob/d5c72312ff4d03291e1ea2eb37464250c85bf401/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L190)). I suspect the value includes the offloaded numbers too, but I wasn't able to assess for certain. @merlimat @lhotari can you confirm? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat][site] PIP-264: Document topic messaging metrics [pulsar-site]
dragosvictor commented on code in PR #880: URL: https://github.com/apache/pulsar-site/pull/880#discussion_r1576804180 ## docs/reference-metrics-opentelemetry.md: ## @@ -8,6 +8,153 @@ Pulsar exposes the following OpenTelemetry metrics. ## Broker +### Topic Messaging metrics + + pulsar.broker.topic.subscription.count +The number of Pulsar subscriptions of the topic served by this broker. +* Type: UpDownCounter +* Unit: `{subscription}` + + pulsar.broker.topic.producer.count +The number of active producers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{producer}` + + pulsar.broker.topic.consumer.count +The number of active consumers of the topic connected to this broker. +* Type: UpDownCounter +* Unit: `{consumer}` + + pulsar.broker.topic.message.incoming.count +The total number of messages received for this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.outgoing.count +The total number of messages read from this topic. +* Type: Counter +* Unit: `{message}` + + pulsar.broker.topic.message.incoming.size +The total number of messages bytes received for this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.message.outgoing.size +The total number of messages bytes read from this topic. +* Type: Counter +* Unit: `{byte}` + + pulsar.broker.topic.publish.rate.limit +The number of times the publish rate limit is triggered. +* Type: Counter +* Unit: `{event}` + + pulsar.broker.topic.consumer.msg.ack +The total number of message acknowledgments received for this topic. +* Type: Counter +* Unit: `{ack}` + + pulsar.broker.topic.storage.size +The total storage size of the messages in this topic. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.logical.size +The storage size of topics in the namespace owned by the broker without replicas. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.size +The total backlog size of the topics of this topic owned by this broker. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.offloaded.size +The total amount of the data in this topic offloaded to the tiered storage. +* Type: UpDownCounter +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.size +The size based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{byte}` + + pulsar.broker.topic.storage.backlog.quota.limit.time +The time based backlog quota limit for this topic. +* Type: Gauge +* Unit: `{second}` + + pulsar.broker.topic.storage.backlog.quota.eviction.count +The number of times a backlog was evicted since it has exceeded its quota. +* Type: Counter +* Unit: `{eviction}` + Review Comment: No, only the event counts, no information on the messages themselves. Perhaps we can add this as a follow-up 👍 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [fix] Include swagger annotations in shaded client lib [pulsar]
merlimat opened a new pull request, #22570: URL: https://github.com/apache/pulsar/pull/22570 ### Motivation Fixes: #21962 #20700 marked the swagger-annotation with scope `provided` in the `pulsar-client` pom. The side effect is that the library is not getting included when the shading happens, though its symbols are being relocated. That is causing warning with Gradle bulds and errors with Bazel. ### Modifications Make sure the swagger annotations are included in the shaded jar (with the renamed symbols). ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` - [X] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [improve] [broker] Create partitioned topics automatically when enable topic level replication [pulsar]
poorbarcode commented on PR #22537: URL: https://github.com/apache/pulsar/pull/22537#issuecomment-2074019426 Rebase master -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix] Include swagger annotations in shaded client lib [pulsar]
lhotari merged PR #22570: URL: https://github.com/apache/pulsar/pull/22570 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Missing shade io.swagger classes in org.apache.pulsar:pulsar-client jar [pulsar]
lhotari closed issue #21962: Missing shade io.swagger classes in org.apache.pulsar:pulsar-client jar URL: https://github.com/apache/pulsar/issues/21962 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [fix] Include swagger annotations in shaded client lib (#22570)
This is an automated email from the ASF dual-hosted git repository. lhotari pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new 77a99ed287c [fix] Include swagger annotations in shaded client lib (#22570) 77a99ed287c is described below commit 77a99ed287cd2c85590f9734190faceb8a50532c Author: Matteo Merli AuthorDate: Tue Apr 23 22:32:06 2024 -0700 [fix] Include swagger annotations in shaded client lib (#22570) --- distribution/shell/src/assemble/LICENSE.bin.txt | 1 + pulsar-client/pom.xml | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/shell/src/assemble/LICENSE.bin.txt b/distribution/shell/src/assemble/LICENSE.bin.txt index 91d4643d9d4..f76631dbbf2 100644 --- a/distribution/shell/src/assemble/LICENSE.bin.txt +++ b/distribution/shell/src/assemble/LICENSE.bin.txt @@ -333,6 +333,7 @@ The Apache Software License, Version 2.0 - listenablefuture-.0-empty-to-avoid-conflict-with-guava.jar * J2ObjC Annotations -- j2objc-annotations-1.3.jar * Netty Reactive Streams -- netty-reactive-streams-2.0.6.jar + * Swagger -- swagger-annotations-1.6.2.jar * DataSketches - memory-0.8.3.jar - sketches-core-0.8.3.jar diff --git a/pulsar-client/pom.xml b/pulsar-client/pom.xml index 3917e2996e1..f79af79d574 100644 --- a/pulsar-client/pom.xml +++ b/pulsar-client/pom.xml @@ -86,7 +86,6 @@ io.swagger swagger-annotations - provided