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

Anastasia Braginsky commented on HBASE-16229:
---------------------------------------------

I have made a review to the current version of the patch, I left the comments 
which need to be relevant regardless the re-base. However, this patch need to 
be significantly changed once 14921 is in and this patch is re-based. I think 
I'll have more comments then. 

After taking an additional look on this patch, I agree that this change is in 
good direction and not far away from the design I have published. I like it. I 
am intrigued to see where it gets after the re-base.

Now let's just wait for HBASE-14921 to be committed and let us return to the 
sizes issues after that once again.

> Cleaning up size and heapSize calculation
> -----------------------------------------
>
>                 Key: HBASE-16229
>                 URL: https://issues.apache.org/jira/browse/HBASE-16229
>             Project: HBase
>          Issue Type: Sub-task
>    Affects Versions: 2.0.0
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>             Fix For: 2.0.0
>
>         Attachments: HBASE-16229.patch, HBASE-16229_V2.patch, 
> HBASE-16229_V3.patch
>
>
> It is bit ugly now. For eg:
> AbstractMemStore
> {code}
> public final static long FIXED_OVERHEAD = ClassSize.align(
>       ClassSize.OBJECT +
>           (4 * ClassSize.REFERENCE) +
>           (2 * Bytes.SIZEOF_LONG));
>   public final static long DEEP_OVERHEAD = ClassSize.align(FIXED_OVERHEAD +
>       (ClassSize.ATOMIC_LONG + ClassSize.TIMERANGE_TRACKER +
>       ClassSize.CELL_SKIPLIST_SET + ClassSize.CONCURRENT_SKIPLISTMAP));
> {code}
> We include the heap overhead of Segment also here. It will be better the 
> Segment contains its overhead part and the Memstore impl uses the heap size 
> of all of its segments to calculate its size.
> Also this
> {code}
> public long heapSize() {
>     return getActive().getSize();
>   }
> {code}
> HeapSize to consider all segment's size not just active's. I am not able to 
> see an override method in CompactingMemstore.
> This jira tries to solve some of these.
> When we create a Segment, we seems pass some initial heap size value to it. 
> Why?  The segment object internally has to know what is its heap size not 
> like some one else dictate it.
> More to add when doing this cleanup



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

Reply via email to