[improve][broker]Only create extended partitions when updating partition number #17349
Conversation
AnonHxy
changed the title
AnonHxy
changed the title
|
/pulsarbot run-failure-checks |
AnonHxy
force-pushed
the
opt_update_partition
branch
from
6c9b348 to
d3ae3e6
Compare
|
/pulsarbot run-failure-checks |
| for (int i = oldNumPartitions; i < numPartitions; i++) { | ||
| futures.add(tryCreatePartitionAsync(i, null)); | ||
| } |
There was a problem hiding this 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.
There was a problem hiding this comment.
We need to discuss this in ML. Thanks a lot for you point it out.
AnonHxy
force-pushed
the
opt_update_partition
branch
from
d3ae3e6 to
7a99303
Compare
|
/pulsarbot run-failure-checks |
2 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
@AnonHxy re-running failed builds won't make this PR mergeable. Please read the mailing list thread about this. https://lists.apache.org/thread/lv358906mnroq7tkk72xtsxfyjpbyvor |
Motivation
Modifications
Describe the modifications you've done.
Verifying this change
PersistentTopicsTest#testUpdatePartitionedTopicDocumentation
Check the box below or label this PR directly.
Need to update docs?
doc-required(Your PR needs to update docs and you will update later)
doc-not-needed(Please explain why)
doc(Your PR contains doc changes)
doc-complete(Docs have been already added)