> On Sept. 19, 2016, 9:48 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java,
> >  lines 95-98
> > <https://reviews.apache.org/r/51785/diff/4/?file=1501087#file1501087line95>
> >
> >     Sorry what is this? Why?
> >     It tries to put back some chunks into pool. This is a merge op or 
> > index-compact. All the segments getting merged will continue to use the old 
> > segments.  Why it is being put back?

This is not putting back. The pooledChunkQueue is the set of the chunks taken 
from the MemStoreChunkPool. The pooledChunkQueue is used to keep references of 
the chunks we ever pooled from the MemStoreChunkPool, later this Queue is used 
to put the closed chunks back to pool. In order not to lose the chunks 
previously allocated from the pool and not yet returned to the pool, I merge 
all the queues from the previous MSLAB to the new MSLAB.


> On Sept. 19, 2016, 9:48 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java,
> >  line 114
> > <https://reviews.apache.org/r/51785/diff/4/?file=1501083#file1501083line114>
> >
> >     Pls see the usage of this pooledChunkQueue for put back.. We need to 
> > assure we dont pool above max capacity of the pool. The max capacity of the 
> > pool can get tuned by HeapMmeoryTuner over the runtime.  Down at place 
> > where this is used I asked another Q.. Am note getting why we need this.
> >     
> >     For Merge type, we can not release any of the segment's MSLAB 
> > instances. Those are active.
> >     For Compact type, we are doing a copy into new MSLAB. So old segment's 
> > can be released. That is any way happening via close() of the segments.

This is in order to return the chunks back to pool after they are released. 
This can not cause pooling above the max capacity of the pool, because those 
chunks are already pooled and here separate queues are just merged into one.

I have written another answer to your comment on the similar issue below, and I 
hope it explains. Please let me know if you still do not understand the reason 
for this code...


> On Sept. 19, 2016, 9:48 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java,
> >  line 207
> > <https://reviews.apache.org/r/51785/diff/4/?file=1501084#file1501084line207>
> >
> >     Why this change?

I am not sure about what change you are asking...
But generally we can no longer rely on this "(cells[i] != c)" comparision in 
order to decide whether the MSLAB is used or not. For the merge case it will 
always be true. So changed to be checked directly...


> On Sept. 19, 2016, 9:48 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
> >  line 55
> > <https://reviews.apache.org/r/51785/diff/4/?file=1501085#file1501085line55>
> >
> >     Use constant INDEX_COMPACTION_CONFIG

I don't want this to be boolean. Index-compaction or data-compaction are 
variants of the compacting memstore type. Therefore I prefer the constant to be 
called COMPACTING_MEMSTORE_TYPE which can have multiple values.


> On Sept. 19, 2016, 9:48 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
> >  line 58
> > <https://reviews.apache.org/r/51785/diff/4/?file=1501085#file1501085line58>
> >
> >     index-compaction means Action.MERGE and data-compaction means 
> > Action.COMPACT, correct?
> >     Can we name both places in similar way pls? So many names makes things 
> > diffcult to read and understand.

Not exactly. Index-compaction and data-compaction are user defined configs of 
the memstore type (with default to index-compaction). 

The Action.MERGE/FLATTEN/COMPACT etc. is the internal desicion of the policy 
(to be done), what in each specific case we should do with the compaction 
pipeline and a new-coming segment.
The correlation is not always one to one.

When the policy whould be defined we will clearly see the difference between 
two.


- Anastasia


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51785/#review149373
-----------------------------------------------------------


On Sept. 18, 2016, 1:37 p.m., Anastasia Braginsky wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51785/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2016, 1:37 p.m.)
> 
> 
> Review request for hbase.
> 
> 
> Repository: hbase-git
> 
> 
> Description
> -------
> 
> This is a step toward final compacting memstore that allowes two modes of 
> work: index-compaction and data-compaction. 
> 
> The index-compaction means that when the new segment is pushed into the 
> pipeline, it is flattened and probably merged with old segments in the 
> pipeline. The new merge "feature" induces no data-copy-compaction and no 
> speculative SQM scan. 
> The compacting memstore of the data-compaction type means the usage of the 
> data-copy-compaction.
> 
> 
> Diffs
> -----
> 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java
>  177f222 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java
>  6a13f43 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java
>  3ca4b0c 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java
>  12b7916 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java
>  714ffe3 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorIterator.java
>  2eafb42 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java
>  510ebbd 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingMemStore.java
>  211a6d8 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingToCellArrayMapMemStore.java
>  fefe2c1 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPerColumnFamilyFlush.java
>  6bfaa59 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWalAndCompactingMemStoreFlush.java
>  74826b0 
> 
> Diff: https://reviews.apache.org/r/51785/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anastasia Braginsky
> 
>

Reply via email to