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-14921) Memory optimizations
Date Sat, 02 Apr 2016 18:48:25 GMT

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

Anastasia Braginsky commented on HBASE-14921:
---------------------------------------------

{quote}
CellBlock createCellBlocks(Comparator<? super Cell> comparator, int min, int max, boolean
descending);
This is for making a sub set of original block. Can we have a better name pls?
{quote}

yes, I’ll change to createSubCellMap()

bq. getCellFromIndex(int i) -> getCell (int index) Is this enough?

yes, changed already

bq. getValidIndex -> Name looks diff to understand what it does. Pls add some comments..
Other places wherever necessary

added comments

bq. HeapMemStoreLAB memStoreLAB -> WHy need the impl type here? We have MSLAB interface
now.
 
Unfortunately, Chunk is internal class of HeapMemStoreLAB, therefore interface MemStoreLAB
can not help. The best solution is to move the Chunk outside of the HeapMemStoreLAB, but I
didn’t want to do this refactoring as part of this JIRA.

bq. HeapMemStoreLAB.Chunk[] chunks -> Do we need to keep Chunk refs? byte[] refs is enough?
Or is this for return back to MSLAB pool after the DISK flush?

This point was already discussed, but generally for off-heaping and re-usage.

bq. Why this move of init to here immediately after the new Chunk is made? When multiple threads
are at this point, there is a chance that more than one will do this init call make make byte[].
If it is after compareAndSet, it is sure abt the concurrency solution.

First, this is for the case when ChunkPool is null and actually the entire code (as it is
written now) doesn’t work if  ChunkPool is null. The issue of ensuring that ChunkPool is
never null was already discussed here.
Second, if multiple threads run this code each thread is going to allocate and initiate its
own chunk.

bq. allocateChunk - Why is it needed when we have getOrMakeChunk? Who will guarantee thread
safety here?

allocateChunk() is a method of MemStoreChunkPool class and getOrMakeChunk() is a method of
HeapMemStoreLAB class, so they do not intersect in their responsibilities. allocateChunk()
is safe for multi-threading because it uses AtomicInteger, new-based allocation, ConcurrentHashMap,
and finally each thread is going to initialize its own chunk (it is not shared memory yet).

{quote}
//org.junit.Assert.assertTrue("\n\n inside chunk initialization 3", false);
pls remove commented code.. Some other cases also.
{quote}

surely will remove

{quote}
-reclaimedChunks.add(chunk);	
I can not see where we will be adding it as per the patch? The initial created chunks were
added here.
{quote}

It was my bug, which is already found and already fixed. The call for reclaimedChunks.add(chunk)
on pre-allocation is indeed missing here.

bq. Smaller than 256 bytes? Yes. How'd you get 256 bytes? I didn't follow. Sorry.

Just guessed that number for easier calculation.

> Memory optimizations
> --------------------
>
>                 Key: HBASE-14921
>                 URL: https://issues.apache.org/jira/browse/HBASE-14921
>             Project: HBase
>          Issue Type: Sub-task
>    Affects Versions: 2.0.0
>            Reporter: Eshcar Hillel
>            Assignee: Anastasia Braginsky
>         Attachments: CellBlocksSegmentInMemStore.pdf, CellBlocksSegmentinthecontextofMemStore(1).pdf,
HBASE-14921-V01.patch, HBASE-14921-V02.patch
>
>
> Memory optimizations including compressed format representation and offheap allocations



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message