bschofield opened a new pull request #678: URL: https://github.com/apache/pulsar-client-go/pull/678
Fixes issue https://github.com/apache/pulsar-client-go/issues/513. This issue was previously partially addressed by @zzzming in PR https://github.com/apache/pulsar-client-go/pull/528. ### Motivation Following PR https://github.com/apache/pulsar-client-go/pull/528, the logic for the `hasSpace()` function, which is used to determine whether a batch container has enough space for a new message, reads: ``` func (bc *keyBasedBatchContainer) hasSpace(payload []byte) bool { msgSize := uint32(len(payload)) return bc.numMessages+1 < bc.maxMessages || (bc.buffer.ReadableBytes()+msgSize) < uint32(bc.maxBatchSize) } ``` Currently, then, the batch container will be considered to have space (be non-full) until **both** the max-messages and the max-size limits are hit. I think that is incorrect. Nearby in the code, the batch container is considered to be full if **either** of the conditions are hit: ``` // IsFull check if the size in the current batch exceeds the maximum size allowed by the batch func (bc *batchContainer) IsFull() bool { return bc.numMessages >= bc.maxMessages || bc.buffer.ReadableBytes() > uint32(bc.maxBatchSize) } ``` Because the logical rule is `!(A || B) = (!A) && (!B)`, the `||` in `hasSpace()` should be an `&&`. This issue was causing me trouble because I set `maxMessages = math.MaxUint32`, with the intention that batching be driven only by message size. With the current logic, that meant that `hasSpace()` always returned true, so I was creating very large message batches which caused system problems. ### Modifications Change the `||` in `hasSpace()` to an `&&`, so that the container will be considered full if either limit is hit. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. ### Documentation - Does this pull request introduce a new feature? no -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
