[GitHub] [pulsar] lhotari commented on a diff in pull request #17349: [improve][broker]Only create extended partitions when updating partition number

[GitBox <gi...@apache.org>]

lhotari commented on code in PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#discussion_r964399426


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java:
##########
@@ -162,6 +162,21 @@ protected CompletableFuture<Void> tryCreatePartitionsAsync(int numPartitions) {
         return FutureUtil.waitForAll(futures);
     }
 
+    protected CompletableFuture<Void> tryCreateExtendedPartitionsAsync(int oldNumPartitions, int numPartitions) {
+        if (!topicName.isPersistent()) {
+            return CompletableFuture.completedFuture(null);
+        }
+        if (numPartitions <= oldNumPartitions) {
+            return CompletableFuture.failedFuture(new RestException(Status.NOT_ACCEPTABLE,
+                    "Number of new partitions must be greater than existing number of partitions"));
+        }
+        List<CompletableFuture<Void>> futures = new ArrayList<>(numPartitions - oldNumPartitions);
+        for (int i = oldNumPartitions; i < numPartitions; i++) {
+            futures.add(tryCreatePartitionAsync(i, null));
+        }

Review Comment:
   @mattisonchao @Technoboy- This is a good example of the challenges with asynchronous code. When using loops that trigger async calls, all operations will be started in parallel. This simply doesn't scale. Let's say that you would create 50000 partitions this way. All operations will be sent off at once. 
   There should be better ways to control the level of concurrency. When using threads and executors, we don't have this problem since threads will be the way to handle the level of concurrency. Obviously that has a tradeoff since threads are idling when they are waiting for responses. When using the asynchronous programming model, there should be ways to implement flow control ("back pressure") by limiting the number of operations in flight. Otherwise it will lead to a lot of requests piling up in memory when the system gets overloaded. That happens eventually if there is no way for flow control and there are a lot of operations and there are peaks in usage where the utilization of the system goes over it's throughput capacity and queues start to grow.
   This particular detail was never addressed in the sync->async transformation and it will be necessary to address it to make the solution reliable.
   
   
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org