dnadolny commented on code in PR #21065:
URL: https://github.com/apache/kafka/pull/21065#discussion_r2674448044
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -1044,6 +1044,9 @@ public void deallocate(ProducerBatch batch) {
if (batch.isBufferDeallocated()) {
log.warn("Skipping deallocating a batch that has already been
deallocated. Batch is {}, created time is {}", batch, batch.createdMs);
} else {
+ if (batch.isInflight()) {
+ throw new IllegalStateException("Attempting to deallocate
a batch that is not inflight. Batch is " + batch);
Review Comment:
If we just log a warning I worry it won't get caught in tests if there's a
regression. One alternative is using an assert like
https://github.com/apache/kafka/pull/21065#discussion_r2674253553, though how
do you feel about deallocating a fresh buffer followed by throwing an
exception? That way there's no memory leak, we fail tests if there's a
regression, and get a stack trace in the logs?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
