[
https://issues.apache.org/jira/browse/HBASE-16229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15381814#comment-15381814
]
ramkrishna.s.vasudevan commented on HBASE-16229:
------------------------------------------------
The DEEP_OVERHEAD_PER_PIPELINE_ITEM is good. We actually don't need to do that.
And the setSnapShot and setSnapShotSize is all fine.
So now Segment.getSize() will not have any DEEP_OVERHEAD and it will always
account only for the actual data in that segment. When we need heapSize or
size() then the DEEP_OVERHEAD will be accounted for.
The Memstore's heapSize() calc is now inside each of the impl
{code}
* Get the entire heap usage for this MemStore not including keys in the
+ * snapshot.
+ */
+ @Override
+ public long heapSize() {
+ return DEEP_OVERHEAD + this.active.heapSize() + this.snapshot.heapSize();
+ }
+
{code}
But now it does include the snapshot also. So should we change the doc ?
Similarly in CompactingMemstore also.
The call to Memstore.size(), Memstore.getFlushableSize() is the one that is
accounted for the flush calculation and this heapSize is only for the HStore
level accounting.
For the ImmutableSegments we have the ClassSize.TimeRange also to be accounted.
Is that missing? Rest looks good to me. The refactoring makes things simple to
reason out the size calculations.
> 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
>
>
> 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)
