KAFKA-10899: Producer's BufferPool closing check#9817
KAFKA-10899: Producer's BufferPool closing check#9817unlizhao wants to merge 1 commit intoapache:trunkfrom unlizhao:fix_producer-bufferpool-support-closeing
Conversation
According to the idea of #7967, it made BufferPool support closing Now, there are two places in the BufferPool#allocate() method to judge the 'closed' flag. One is when 'lock' is acquired, and the other is when 'condition' is awakened However, if the memory is allocated outside the lock code block after freeup, Therefore, it is possible for another thread to modify the 'closed' flag, causing this part of memory to be allocated in vain So two modifications have been made, One is to add the 'volatile' modifier before 'closed', One is to determine whether to 'closed' before allocating memory
unlizhao
changed the title
|
hi，@chia7712 @rajinisivaram PTAL，thanks |
| private ByteBuffer safeAllocateByteBuffer(int size) { | ||
| boolean error = true; | ||
| try { | ||
| if (this.closed) { |
There was a problem hiding this comment.
It is not holding the lock so this pool can be closed anytime. How we know the "best" place to check the close flag?
There was a problem hiding this comment.
@chia7712 thanks for your reviewed. This is what I thought when I wrote this code
In the PR of #3053, for efficiency, the operation of allocating memory is moved out of the locked code block.
Subsequent modifications should be based on this consensus, and do some time-consuming operations without locking.
So what we can do is to try our best to check whether the memory can be allocated before allocating, and reduce the operation of allocating memory, just like the 'fail-fast' mechanism.
In this scenario, both BufferPool#allocate() and BufferPool#close() compete for locks.
Here we can think that lock blocking is more likely to occur in concurrent cases.
Finally, even if the memory is allocated in the close state, there is a plan to release the memory. Therefore, this COMMIT is like an optimization rather than bug-fix
|
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
|
This PR has been closed since it has not had any activity in 120 days. If you feel like this |
2 participants
According to the idea of #7967, it made BufferPool support closing.
Now, there are two places in the BufferPool#allocate() method to judge the 'closed' flag. One is when 'lock' is acquired, and the other is when 'condition' is awakened.
However, if the memory is allocated outside the 'lock' code block after 'freeup',
Therefore, it is possible for another thread to modify the 'closed' flag, causing this part of memory to be allocated in vain.
So two modifications have been made,
One is to add the 'volatile' modifier before 'closed',
One is to determine whether to 'closed' before allocating memory.