You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/08/30 11:55:30 UTC
[GitHub] [pulsar] AnonHxy opened a new pull request, #17349: [improve][broker]Only create extended partitions when updating partition number
AnonHxy opened a new pull request, #17349:
URL: https://github.com/apache/pulsar/pull/17349
### Motivation
*Only create extended partitions when update partion number. It' useful for topic having huge partitions when updating partitions
### Modifications
*Describe the modifications you've done.*
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Documentation
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)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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
[GitHub] [pulsar] lhotari commented on a diff in pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by 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
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#discussion_r964932039
##########
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:
We need to discuss this in ML. Thanks a lot for you point it out.
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1233915749
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] AnonHxy merged pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy merged PR #17349:
URL: https://github.com/apache/pulsar/pull/17349
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1232362453
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1237615516
@Jason918 @Technoboy- @codelipenghui PTAL~
--
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
[GitHub] [pulsar] lhotari commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1241602815
@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
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1241513472
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1240438430
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] AnonHxy commented on pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17349:
URL: https://github.com/apache/pulsar/pull/17349#issuecomment-1240265951
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] AnonHxy closed pull request #17349: [improve][broker]Only create extended partitions when updating partition number
Posted by GitBox <gi...@apache.org>.
AnonHxy closed pull request #17349: [improve][broker]Only create extended partitions when updating partition number
URL: https://github.com/apache/pulsar/pull/17349
--
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