[
https://issues.apache.org/jira/browse/HBASE-15788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15648111#comment-15648111
]
stack commented on HBASE-15788:
-------------------------------
Are failures related?
On the patch:
I'm trying to follow what is going on in CellBlockBuilder. We seem to make an
InputStream out of a byte array to copy decompressed bytes into a ByteBuffer
and then we make a ByteBufferInputStream of it. It looks ok just a bit
convoluted (maybe it has to be like this).
This is a new type, ShareableMemoryOffheapKeyValue. In KeyValueCodec, we know
it is safe to make these? How many Cell/KeyValue types do we have now?
On ByteBufferSupportOutputStream as a marker interface, could we refactor?
Could we make the ByteBufferOutputStream an abstract class or an Interface?
Then have implementations implement it rather than afterward come along and
mark BBOS as ByteBufferSupportOS which is kinda stating the obvious. The
ByteArrayOutputStream is our own too so we can do it anyway we want... having
it implement BBOS instead of OS and then marking it with BBSupportOS. Looks
like it would have to be an Interface if we want to subclass DOS as we do in
BBSDOS. ByteBufferSupportOutputStreamWrapper could then just be BBOSWrapper.
When would we be passed an OS that doesn't support
ByteBufferSupportOutputStream? Is this a just-in-case?
Thanks for adding this:
// TODO to have another name. This can easily get confused with netty's
ByteBuf
Lets make the change before 2.0.
I like this: private final int minSizeForReservoirUse;
Is there a missing assign in the below?
1810 // Allocate ByteBuffer(s) and assign to 'data'
1811 allocateByteBuffToReadInto(dataLength);
Better to return data and assign to this.data in the caller... doing as
side-effect can confuse....
ByteBuffByteInput is from netty? I see what it is. So, its passing netty
ByteBuff to pb? Needs class comment.
Nice unit tests.
There is great stuff going on in here. Just a bit tough to follow. Can we do
anything about the proliferation of KV types. Can we do some cleanup around
ByteBufferSupport*
Thanks.
> Use Offheap ByteBuffers from BufferPool to read RPC requests.
> -------------------------------------------------------------
>
> Key: HBASE-15788
> URL: https://issues.apache.org/jira/browse/HBASE-15788
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
> Reporter: ramkrishna.s.vasudevan
> Assignee: Anoop Sam John
> Fix For: 2.0.0
>
> Attachments: HBASE-15788.patch, HBASE-15788_V4.patch,
> HBASE-15788_V5.patch
>
>
> Right now, when an RPC request reaches RpcServer, we read the request into an
> on demand created byte[]. When it is write request and including many
> mutations, the request size will be some what larger and we end up creating
> many temp on heap byte[] and causing more GCs.
> We have a ByteBufferPool of fixed sized off heap BBs. This is used at
> RpcServer while sending read response only. We can make use of the same while
> reading reqs also. Instead of reading whole of the request bytes into a
> single BB, we can read into N BBs (based on the req size). When BB is not
> available from pool, we will fall back to old way of on demand on heap byte[]
> creation.
> Remember these are off heap BBs. We read many proto objects from this read
> request bytes (like header, Mutation protos etc). Thanks to PB 3 and our
> shading work as it supports off heap BB now. Also the payload cells are also
> in these DBBs now. The codec decoder can work on these and create off heap
> BBs. Whole of our write path work with Cells now. At the time of addition to
> memstore, these cells are by default copied to MSLAB ( off heap based pooled
> MSLAB issue to follow this one). If MSLAB copy is not possible, we will do a
> copy to on heap byte[].
> One possible down side of this is :
> Before adding to Memstore, we do write to WAL. So the Cells created out of
> the offheap BBs (Codec#Decoder) will be used to write to WAL. The default
> FSHLog works with an OS obtained from DFSClient. This will have only standard
> OS write APIs which is byte[] based. So just to write to WAL, we will end up
> in temp on heap copy for each of the Cell. The other WAL imp (ie. AsynWAL)
> supports writing offheap Cells directly. We have work in progress to make
> AsycnWAL as default. Also we can raise HDFS req to support BB based write
> APIs in their client OS? Until then, will try for a temp workaround solution.
> Patch to say more on this.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
