[
https://issues.apache.org/jira/browse/HBASE-12295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14583173#comment-14583173
]
ramkrishna.s.vasudevan commented on HBASE-12295:
------------------------------------------------
Had some internal discussions on some of the questions raised
Thanks for the reviews so far over in RB. We discussed some of your comments
and thought of finalizing things and also thought it would help to clarify
certain aspects as why it was done.
-> CellBlock creation
The first area is the cellblock creation. As per the previous mail discussion
now we will have now create the cellBlock in the RsRpcServices layer both for
scan and gets.
The scans already were creating the cellScanner in the RsRpcServices layer -
now that we have added the cellBlock creation here.
But in order to do this we are creating a new getScanner() API in HRegion -
(note that it is not in Region.java) so it is not exposed to CPs.
This new API would indicate if the client is java based or not. For java based
client we will allow the cellblock creation in the RsRpcServices layer and then
the blocks are returned back (ref count decremented).
But for the non-java cases we will go with the old getScanner() API only but
internally it would set a state in the RegionScanner saying that the results
are to be copied then and there as there is no cellblock creation for it. (We
don't create cellblock for non-java cases as you may be already knowing it).
Once the copying of the results are done we are sure that we can return the
blocks back.
This new state in the REgionScanner also helps us with the case of CPs wrapping
the scanner where though the scan is from java -client the partial results
could be used in the CPs and we may not know how the CPs will use the result.
Hence it is better to copy those results like we do for non-java client cases.
In case of scan() after every next() we make a call to return the blocks so
that we don't hold them up till the scanners are done.
Now coming to get(), we have now tweaked the logic of get() in RsrpcServices
such that what ever was being done in Region.get() is now moved to
RsRpcServices included CP hooks and once the result is formed we create the
cellblock in RsRpcServices.
For the non-java case the old Region.get() API is still used. Just like in
scan the results would be copied once it is formed.
Remember we need to use it for CP when it needs to do a get() on a region
directly.
In case of gets() the scanner.close() woudl ensure that the block ref count
decrement happens.
-> MemType and CacheType
First of all we can be sure that none of the blocks coming from L1 cache needs
to go through all these shared memory or copying stuff. Because they are just
objects referring to the blocks created from HDFS and nothing is serialized and
there is no problem of eviction.
We need to do this only for Bucketcache. BucketCache again has 3 things
-> RamQueue - which is like initially the blocks are added to this queue and
then moved to the actual buckets. So there are chances the reads are served
from these queue itself. Those cases there is no need for any copying or
sharing of memory. (this is similar to L1).
-> ByteBufferIOEngine
This is the actual case where we need to solve. Here the block is for sure
from L2 and also the memory is shared. So if you see in our patch we would
have set this L2 and shared_memory only at this layer.
-> fileIOEngine
Here though it is a bucketcache impl there is no need for having a shared_mem
because already there is copy from the file.
But who is responsible for setting the memory type-? It should be the engine.
The engine knows what type of memory it is creating. The engines just return a
BB and we don't know the type of it. so better the engine set the type of the
memory also.
-> ReturnBlock in between code in HFileREaders
This is important. As I earlier commented in the RB, the index or bloom blocks
from L1 cache can get demoted to L2 cache. Our code is such that in some case
we try to read a block and we iterate till we reach a data block. Those blocks
could have been from L1 or L2 - we are not sure.
So we have analysed the code and ensured that where ever we read blocks and
just throw it away till we end up in a data block- all such blocks have to be
returned back. If the block is from L1 it is going to be a noop.
I verified it once again and to be safe added some try/finally around it. I
can explain more on this if you have some queries.
->IPCUtil singleton
IPCUtil singleton was mainly for ease of use. The IPCutil had the code to
create Cellblock. So we wanted to reuse that code rather than duplicating it.
We could create IPCUTil inside Payload also. But for doing that we need the
'conf' object. Most of the cases Payload is created from RPcControllerfactory
where already we have 'conf'. But in case of AbstractRPcClient,
AbstractRpcCaller etc we need to get the access to these conf object so that we
can pass a 'conf' to Payload and use that conf to create the IPCUtil object.
So in order to avoid this in the latest will pass the IPCtuil to the Payload
using a helper class which will also have all the other ingredients needed to
create a cellblock. This wil avoid all the singleton related changes to
IPCUtil.
To which Stack had some comments
Hmm.
RsRpcServices is a weird class. It has not character. It is just something
Jimmy made to move a bunch of HRegionServer to its own class.
So, will Get be the only thing in Region that cannot be intercepted by CPs?
Other methods in Region can be intercepted by CPs? Having Get be an exception
will cause confusion.
Or is it that you are saying that the old Get can be used by CPs to
intercept... and in this case there will be the unavoidable copy. If so, that
sounds ok (where can I see current state of code?)
For the MemType and CacheType his reply was
bq.
Thats fine. I just don't like labeling each individual block. High-level, the
engine knows where the block came from, right? It can stamp the return with
where it came from rather than do it per block?
> Prevent block eviction under us if reads are in progress from the BBs
> ---------------------------------------------------------------------
>
> Key: HBASE-12295
> URL: https://issues.apache.org/jira/browse/HBASE-12295
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver, Scanners
> Reporter: ramkrishna.s.vasudevan
> Assignee: ramkrishna.s.vasudevan
> Fix For: 2.0.0
>
> Attachments: HBASE-12295.pdf, HBASE-12295_1.patch, HBASE-12295_1.pdf,
> HBASE-12295_2.patch, HBASE-12295_4.patch, HBASE-12295_trunk.patch
>
>
> While we try to serve the reads from the BBs directly from the block cache,
> we need to ensure that the blocks does not get evicted under us while
> reading. This JIRA is to discuss and implement a strategy for the same.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
