[
https://issues.apache.org/jira/browse/HBASE-21879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16862244#comment-16862244
]
chenxu commented on HBASE-21879:
--------------------------------
In Compactor#compact, we have the following:
{code:java}
protected List<Path> compact(final CompactionRequest request...
...
try {
...
} finally {
Closeables.close(scanner, true);
if (!finished && writer != null) {
abortWriter(writer);
}
}
assert finished : "We should have exited the method on all error paths";
assert writer != null : "Writer should be non-null if no error";
return commitWriter(writer, fd, request);
}{code}
should we call writer#beforeShipped() before _Closeables.close(scanner, true);_
In order to copy some cell's data out of the ByteBuff before it released, or
_commitWriter_ may be wrong in the following call stack
Compactor#commitWriter
－> HFileWriterImpl#close
－> HFileWriterImpl#writeFileInfo
－> HFileWriterImpl#finishFileInfo
{code:java}
protected void finishFileInfo() throws IOException {
if (lastCell != null) {
// Make a copy. The copy is stuffed into our fileinfo map. Needs a clean
// byte buffer. Won't take a tuple.
byte [] lastKey =
PrivateCellUtil.getCellKeySerializedAsKeyValueKey(this.lastCell);
fileInfo.append(FileInfo.LASTKEY, lastKey, false);
}
...
}{code}
Because the _lastCell_ may refer to a reused ByteByff.
> Read HFile's block to ByteBuffer directly instead of to byte for reducing
> young gc purpose
> ------------------------------------------------------------------------------------------
>
> Key: HBASE-21879
> URL: https://issues.apache.org/jira/browse/HBASE-21879
> Project: HBase
> Issue Type: Improvement
> Reporter: Zheng Hu
> Assignee: Zheng Hu
> Priority: Major
> Fix For: 3.0.0, 2.3.0
>
> Attachments: HBASE-21879.v1.patch, HBASE-21879.v1.patch,
> QPS-latencies-before-HBASE-21879.png, gc-data-before-HBASE-21879.png
>
>
> In HFileBlock#readBlockDataInternal, we have the following:
> {code}
> @VisibleForTesting
> protected HFileBlock readBlockDataInternal(FSDataInputStream is, long offset,
> long onDiskSizeWithHeaderL, boolean pread, boolean verifyChecksum,
> boolean updateMetrics)
> throws IOException {
> // .....
> // TODO: Make this ByteBuffer-based. Will make it easier to go to HDFS with
> BBPool (offheap).
> byte [] onDiskBlock = new byte[onDiskSizeWithHeader + hdrSize];
> int nextBlockOnDiskSize = readAtOffset(is, onDiskBlock, preReadHeaderSize,
> onDiskSizeWithHeader - preReadHeaderSize, true, offset +
> preReadHeaderSize, pread);
> if (headerBuf != null) {
> // ...
> }
> // ...
> }
> {code}
> In the read path, we still read the block from hfile to on-heap byte[], then
> copy the on-heap byte[] to offheap bucket cache asynchronously, and in my
> 100% get performance test, I also observed some frequent young gc, The
> largest memory footprint in the young gen should be the on-heap block byte[].
> In fact, we can read HFile's block to ByteBuffer directly instead of to
> byte[] for reducing young gc purpose. we did not implement this before,
> because no ByteBuffer reading interface in the older HDFS client, but 2.7+
> has supported this now, so we can fix this now. I think.
> Will provide an patch and some perf-comparison for this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
