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/04/19 09:50:01 UTC
[GitHub] [pulsar] Jason918 opened a new pull request, #15216: [PIP-74] Support consumer client memory limit
Jason918 opened a new pull request, #15216:
URL: https://github.com/apache/pulsar/pull/15216
### Motivation
This is the final part of PIP-74.
https://github.com/apache/pulsar/wiki/PIP-74%3A-Pulsar-client-memory-limits
### Modifications
1. This feature does not take effect by default for now. Consumer memory usage take account only if `autoScaledReceiverQueueSizeEnabled` is enabled. We can change this to default when this feature is verified by more production usage.
2. Block consumer receiver queue size expansion if memory usage > 75%
3. Trigger consumer receiver queue size shrinking if memory usage > 95%
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change added tests and can be verified as follows:
- ConsumerMemoryLimitTest
- MemoryLimitControllerTest
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [x] `no-need-doc`
--
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] Jason918 merged pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
Jason918 merged PR #15216:
URL: https://github.com/apache/pulsar/pull/15216
--
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] Jason918 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1103360869
/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] gaozhangmin commented on a diff in pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#discussion_r866663655
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/MemoryLimitController.java:
##########
@@ -45,11 +60,18 @@ public boolean tryReserveMemory(long size) {
}
if (currentUsage.compareAndSet(current, newUsage)) {
+ checkTrigger(current, newUsage);
return true;
}
}
}
+ private void checkTrigger(long prevUsage, long newUsage) {
+ if (newUsage >= triggerThreshold && prevUsage < triggerThreshold && trigger != null) {
Review Comment:
Would this be better?
`if (trigger != null && newUsage >= triggerThreshold && prevUsage < triggerThreshold) `
--
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] Gleiphir2769 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by "Gleiphir2769 (via GitHub)" <gi...@apache.org>.
Gleiphir2769 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1446360203
> > Do we need to introduce a publish/consume memory ratio? In most cases, users are consuming messages from the consumer and publish new messages, and ack the consumed messages. I'm worried about if the consumer can use up the memory, the producer not able to publish, so that the consumer queue can't be cleanup, it looks like a deadlock.
>
> Yes, good point. Sharing memory quota does have this kind of isolation problem. Not only between producers and consumers, but also between different consumers. E.g. one consumer use up the memory, other consumers may not be able to receive any message any more.
>
> Currently, I suggest to **enable this feature within a new client instance.**
>
> I am not sure it's the right time to just split publish/consume memory quota, by ratio or other ways. One alternative is to reduce local message buffer directly and make broker resend message later. This may require some change in protocols. Just a brief thought yet. Let's dig deeper in anther PIP(like solving isolation problem when sharing producers and consumers in one client instance).
>
> @codelipenghui
Hi @Jason918, I find this discussion from https://github.com/apache/pulsar-client-go/issues/927. Just some question I want to figure out.
Why not let the producer/consumer have their own `MemoryLimitController`? And split the `memoryLimit` setting into `consumerMemoryLimit` and `producerMemoryLimit`. Are there any other considerations?
--
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] Jason918 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by "Jason918 (via GitHub)" <gi...@apache.org>.
Jason918 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1465585402
> Why not let the producer/consumer have their own `MemoryLimitController`? And split the `memoryLimit` setting into `consumerMemoryLimit` and `producerMemoryLimit`. Are there any other considerations?
@Gleiphir2769 I have replied in the mail list. :)
--
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] Jason918 commented on a diff in pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#discussion_r867287493
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/MemoryLimitController.java:
##########
@@ -45,11 +60,18 @@ public boolean tryReserveMemory(long size) {
}
if (currentUsage.compareAndSet(current, newUsage)) {
+ checkTrigger(current, newUsage);
return true;
}
}
}
+ private void checkTrigger(long prevUsage, long newUsage) {
+ if (newUsage >= triggerThreshold && prevUsage < triggerThreshold && trigger != null) {
Review Comment:
I put this at the last on purpose. `trigger` is null only in unit test.
--
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] Jason918 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1104628231
/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 #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1118110787
LGTM
--
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] Gleiphir2769 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by "Gleiphir2769 (via GitHub)" <gi...@apache.org>.
Gleiphir2769 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1447456967
> Great question @Gleiphir2769. If you don't get an answer, it might help to post this on the pulsar dev mailing list.
Good suggestion @michaeljmarshall ! I will start a discussion on the dev mailing list.
--
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] michaeljmarshall commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1446810043
Great question @Gleiphir2769. If you don't get an answer, it might help to post this on the pulsar dev mailing list.
--
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] Jason918 commented on pull request #15216: [PIP-74] Support consumer client memory limit
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #15216:
URL: https://github.com/apache/pulsar/pull/15216#issuecomment-1108005626
> Do we need to introduce a publish/consume memory ratio? In most cases, users are consuming messages from the consumer and publish new messages, and ack the consumed messages. I'm worried about if the consumer can use up the memory, the producer not able to publish, so that the consumer queue can't be cleanup, it looks like a deadlock.
Yes, good point. Sharing memory quota does have this kind of isolation problem. Not only between producers and consumers, but also between different consumers. E.g. one consumer use up the memory, other consumers may not be able to receive any message any more.
Currently, I suggest to **enable this feature within a new client instance.**
I am not sure it's the right time to just split publish/consume memory quota, by ratio or other ways. One alternative is to reduce local message buffer directly and make broker resend message later. This may require some change in protocols. Just a brief thought yet. Let's dig deeper in anther PIP(like solving isolation problem when sharing producers and consumers in one client instance).
@codelipenghui
--
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