mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anastasia Braginsky <anas...@yahoo-inc.com>
Subject Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush
Date Wed, 21 Sep 2016 13:56:43 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message