hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anastasia Braginsky (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-16438) Create a cell type so that chunk id is embedded in it
Date Tue, 14 Mar 2017 11:21:41 GMT

    [ https://issues.apache.org/jira/browse/HBASE-16438?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15924009#comment-15924009
] 

Anastasia Braginsky commented on HBASE-16438:
---------------------------------------------

Hi [~ram_krish]!

Thank you very much for the updated patch!
I see that you added the tests which is very good!
Are you going to address my other comments that I posted on the RB? (on your previous version
of the patch)

I want to raise again the issue of the design and responsibilities between MemStoreChunkPool
and MemStoreChunkCreator.
It is much clearer, understandable and easier to maintain when a class (e.g. MemStoreChunkCreator)
has a clear responsibility (aka total allocation of the chunk including 2MB memory) and it
is not spread over other (more than one) classes.
If something in this path is going to be changed in the future we have single point of maintenance.
If there is a bug, it has only single appearance...
If we are already restructuring those relationships, let us make it in a better way than it
was.

I suggest to make chunk.init() a part of the allocation in the MemStoreChunkCreator. So the
chunk will leave the MemStoreChunkCreator already with the 2MB attached to it (whether from
pool or from JVM).
The MemStoreChunkPool should also refer to MemStoreChunkCreator also when pre-allocating the
chunks ahead of time.
More than that, in MemStoreLABImpl the code of getOrMakeChunk() is quite awkward. Chunk is
allocated via pool or may be not, and then initiated 10 lines below after multiple if-statements.

I understand that there can be multi-threaded concurrency, but we can deal with that. This
is what exactly makes it interesting to rewrite this code creatively! :)

So if we are already talking about chunk allocation let's make it all much smoother...
I suggest to make the MemStoreChunkCreator the single entity responsible for chunk allocation
AND initialization.
So the entire code-path of chunk allocation is going to be refactored and better looking...
What do you think?

Thanks,
Anastasia

> Create a cell type so that chunk id is embedded in it
> -----------------------------------------------------
>
>                 Key: HBASE-16438
>                 URL: https://issues.apache.org/jira/browse/HBASE-16438
>             Project: HBase
>          Issue Type: Sub-task
>    Affects Versions: 2.0.0
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: ramkrishna.s.vasudevan
>         Attachments: HBASE-16438_1.patch, HBASE-16438.patch, MemstoreChunkCell_memstoreChunkCreator_oldversion.patch,
MemstoreChunkCell_trunk.patch
>
>
> For CellChunkMap we may need a cell such that the chunk out of which it was created,
the id of the chunk be embedded in it so that when doing flattening we can use the chunk id
as a meta data. More details will follow once the initial tasks are completed. 
> Why we need to embed the chunkid in the Cell is described by [~anastas] in this remark
over in parent issue https://issues.apache.org/jira/browse/HBASE-14921?focusedCommentId=15244119&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15244119



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message