KAFKA-5213; Mark a MemoryRecordsBuilder as full as soon as the append stream is closed #3015
Conversation
|
cc @hachikuji @ijuma |
| FutureRecordMetadata result0 = batch.tryAppend(now, null, new byte[10], Record.EMPTY_HEADERS, null, now); | ||
| assertNotNull(result0); | ||
| memoryRecordsBuilder.closeForRecordAppends(); | ||
| assertFalse(memoryRecordsBuilder.hasRoomFor(now, null, new byte[10])); |
There was a problem hiding this comment.
I'd add this same test (but assertTrue) just before closeForRecordAppends to verify that the closeForRecordApends is what causes the change.
There was a problem hiding this comment.
Good suggestion, updated.
…r append, but not after
| @@ -664,7 +664,7 @@ public boolean isClosed() { | |||
| public boolean isFull() { | |||
| // note that the write limit is respected only after the first record is added which ensures we can always | |||
| // create non-empty batches (this is used to disable batching when the producer's batch size is set to 0). | |||
| return isClosed() || (this.numRecords > 0 && this.writeLimit <= estimatedBytesWritten()); | |||
| return appendStreamIsClosed || (this.numRecords > 0 && this.writeLimit <= estimatedBytesWritten()); | |||
There was a problem hiding this comment.
Is it possible that isClosed() is true, but appendStreamIsClosed is false?
There was a problem hiding this comment.
No. MemoryRecordsBuilder.close calls closeForRecordAppends after performing validations.
|
Bonus points for including the test from the start. :) |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
No description provided.