[GitHub] merlimat commented on issue #1123: Configure a max producer/consumer queue size enforced across partitions

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

merlimat commented on issue #1123: Configure a max producer/consumer queue size enforced across partitions
URL: https://github.com/apache/incubator-pulsar/pull/1123#issuecomment-361664821
 
 
   > Code changes look fine in general - except for once comment about using "throw"
   
   We're using "throw" in all other parameters validation already. The reasons why `throw` is generally preferable to `assert()` are:
   
    1. With throw we can put a message to why we did it
    2. Asserts are removed when compiling with `-DNDEBUG` in release binaries (I don't think we use that flag, though is a very common practice)
    3. Asserts are used to validated programmer's assumptions for conditions that should never happen, not for parameter validations
   
   > I was thinking of a different approach - divide the maxPendingMessages and receiveQueueSize by number or partitions with min as 100 (or some other number)
   > i want to avoid adding another two parameters (one in producer and consumer each)
   
   We still need to give a chance to override the limits to accommodate unusual requirements.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services