sodonnel commented on a change in pull request #2929:
URL: https://github.com/apache/ozone/pull/2929#discussion_r772685089
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
##########
@@ -259,7 +259,7 @@ private StripeWriteStatus rewriteStripeToNewBlockGroup(int
chunkSize,
blockOutputStreamEntryPool.getCurrentStreamEntry();
newBlockGroupStreamEntry
.updateBlockGroupToAckedPosition(failedStripeDataSize);
- ecChunkBufferCache.clear(chunkSize);
Review comment:
There seems to be two places in the code where we
"allocateParityBuffers". When finishing a stripe and when finishing the file.
When finishing the stripe, we always ask for a ecChunkSize buffers, but when
calling close, we ask for buffers for the desired size. I think we should
always ask for the full chunksize, and then set the limits accordingly.
However looking at the `allocateBuffers()` code:
```
private void allocateBuffers(ByteBuffer[] buffers, int bufferSize) {
for (int i = 0; i < buffers.length; i++) {
buffers[i] = byteBufferPool.getBuffer(false, cellSize);
buffers[i].limit(bufferSize);
}
}
```
We always allocate the cellSize, but then set the limit, so I think we
always ask for ecChunkSize buffers.
Looking into this code a bit more, I see we always call
`allocateParityBuffers()` in `writeParityCells()`. Is this the correct thing to
do? Should we not re-use the parity buffers for the life of the block, and just
clear them between stripes?
Also, do we release the parity buffers, before allocating them again, if we
allocate them on each stripe write?
From the above code snippet, if we call `allocateParityBuffers()`:
```
public void allocateParityBuffers(int size){
allocateBuffers(parityBuffers, size);
}
```
It will just overwrite the slots in the parityBuffer array, which will leak
buffers in the pool.
Am I reading the code correctly, or am I missing something?
Prior to this PR, the pool is a static variable - if I am correct about the
above, then I think the leak exists even without this change?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
