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: [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: [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
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] [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: [PR] [improve][client] Add maxConnectionsPerHost and connectionMaxIdleSeconds to PulsarAdminBuilder [pulsar]
lhotari commented on PR #22541: URL: https://github.com/apache/pulsar/pull/22541#issuecomment-2071568113 There's a problem with backpressure handling with async requests in the Pulsar code base. Since this PR limits the Pulsar Admin client to 16 connections per host, it now shows up problems. The namespace unloading is a good example: https://github.com/apache/pulsar/blob/d7d54522933b63f6a74ec7139c6dedebe8ad9149/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L829-L841 All bundles in the namespace are unloaded at once without limiting concurrency. There was a dev mailing list discussion about backpressure and Pulsar Admin API implementation in https://lists.apache.org/thread/03w6x9zsgx11mqcp5m4k4n27cyqmp271 . However we didn't come across resolving the problem. The solution for the namespace unloading issue is to have a way to limit the outstanding CompletableFutures that are in progress and use that as a way to "backpressure" the sending of new requests. The current solution of sending out all requests and then waiting for the results is a problematic solution since it doesn't use any sort of feedback from the system to adjust the speed. In other words, there's currently no proper backpressure solution for async Pulsar Admin calls within Pulsar broker. I'll experiment with some ways to add backpressure to cases where a large amount of async calls are triggered and then results are waited. -- 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-2070268832 the `setMaxConnectionsPerHost` in async http client doesn't seem to behave as expected. Will check the errors ``` Caused by: java.util.concurrent.CompletionException: org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector$RetryException: Could not complete the operation. Number of retries has been exhausted. Failed reason: Too many connections: 16 at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332) at java.base/java.util.concurrent.CompletableFuture.uniApplyNow(CompletableFuture.java:674) at java.base/java.util.concurrent.CompletableFuture.orApplyStage(CompletableFuture.java:1601) at java.base/java.util.concurrent.CompletableFuture.applyToEither(CompletableFuture.java:2261) at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.retryOrTimeOut(AsyncHttpConnector.java:275) at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.apply(AsyncHttpConnector.java:234) ``` -- 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 code in PR #22541: URL: https://github.com/apache/pulsar/pull/22541#discussion_r1574241817 ## pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java: ## @@ -336,4 +336,30 @@ PulsarAdminBuilder authentication(String authPluginClassName, Map + * By default, the connection pool maintains up to 16 connections to a single host. This method allows you to + * modify this default behavior and limit the number of connections. + * + * This setting can be useful in scenarios where you want to limit the resources used by the client library, + * or control the level of parallelism for operations so that a single client does not overwhelm + * the Pulsar cluster with too many concurrent connections. + * + * @param connectionsPerHost the maximum number of connections to establish per host. Set to <= 0 to disable + * the limit. + * @return the PulsarAdminBuilder instance, allowing for method chaining + */ +PulsarAdminBuilder connectionsPerHost(int connectionsPerHost); Review Comment: makes sense. renamed -- 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
