KAFKA-4011 - fix issues and beef up tests around ByteBoundedBlockingQueue #1714
Conversation
| extends Iterable[E] { | ||
| if (sizeFunction == null || sizeFunction.isEmpty) { | ||
| throw new IllegalArgumentException("size function must be provided") | ||
| } |
There was a problem hiding this comment.
If we want to make it mandatory, then we should just change sizeFunction to be E => Long. Option only makes sense if we want it to be optional.
There was a problem hiding this comment.
true, fixed (and force pushed)
radai-rosenblatt
force-pushed
the
ByteBoundedBlockingQueue-work
branch
from
d5a63e5
to
5feba7b
Compare
| currentByteSize.addAndGet(sizeFunction.get(e)) | ||
| if (currentByteSize.get() < queueByteCapacity && leftToWait > 0) { | ||
| val success = queue.offer(e, leftToWait, TimeUnit.NANOSECONDS) //wait to clear numElements req. | ||
| val sizeBytesAfter = if (success) { |
There was a problem hiding this comment.
Minor: In Kafka we don't put brackets for one line code.
…ueue, as discussed under KIP-72
radai-rosenblatt
force-pushed
the
ByteBoundedBlockingQueue-work
branch
from
5feba7b
to
f997ffd
Compare
|
@becketqin Are you happy with this now or should someone else take a pass at review? |
|
@ewencp This PR is actually for KIP-72, which has been changed quite a bit since its initial proposal. So this PR seems not applicable any more. @radai-rosenblatt Could you close the PR and link the KIP to the right PR? |
|
this patch contains fixes to ByteBoundedBlockingQueue. KIP-72 no longer intends to use this class and, given nothing else uses it either, it should probably be deleted. |
as discussed under KIP-72