[GitHub] [pulsar-client-go] shibd commented on a diff in pull request #991: [feat] Support consumer client memory limit

["shibd (via GitHub)" <gi...@apache.org>]

shibd commented on code in PR #991:
URL: https://github.com/apache/pulsar-client-go/pull/991#discussion_r1141767660


##########
pulsar/consumer_partition.go:
##########
@@ -1752,6 +1762,37 @@ func (pc *partitionConsumer) markScaleIfNeed() {
 	}
 }
 
+func (pc *partitionConsumer) reserveMemory(size int64) {
+	pc.client.memLimit.ForceReserveMemory(size)
+	if pc.client.memLimit.CurrentUsagePercent() >= receiverQueueShrinkMemThreshold {

Review Comment:
   This shrinking strategy is different from java client implementation. 
   
   The java client implementation:
   
   Will try to trigger shrinking memory after each `ReserveMemory`, this means that the producer will also trigger the consumer to reduce the queue when sending data. 
   
   https://github.com/shibd/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MemoryLimitController.java#L65
   
   Your implementation will cause the following scenario where producers cannot produce data.
   - One client instance has `1` producer instance and `3` consumer.
   - If `3` consumers run out of memory, the producer's production data will block.
   
   
   



-- 
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