[
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)
