mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anoop Sam John <anoop.hb...@gmail.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 Tue, 27 Sep 2016 17:04:43 GMT


> On Sept. 27, 2016, 3:33 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
line 238
> > <https://reviews.apache.org/r/51785/diff/6/?file=1510150#file1510150line238>
> >
> >     Asked this already. In case of flattening, we just need to call this method
no? But now the code after the switch block will get executed no?  Where is the return? We
had that in before patch
> 
> Anastasia Braginsky wrote:
>     I have actually reshaped this code to get rid off the annoying findbugs issues. But
just to clearify the old version, the return is in the next case of this switch. We intentionally
fall through because there is no break at the end of this case.
>     
>          switch (nextStep){
>           case FLATTEN:    // Youngest Segment in the pipeline is with SkipList index,
make it flat
>             compactingMemStore.flattenOneSegment(versionedList.getVersion());
>           case NOOP:       // intentionally falling through
>             return;                <------------------------------------------------
here is the return
>           case MERGE:      // intentionally fall through
>           case COMPACT:
>             break;
>           default: throw new RuntimeException("Unknown action " + action); // sanity
check
>          }

Can there be a NOOP possible?  If not I would say just go with if else as of now.


> On Sept. 27, 2016, 3:33 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
line 49
> > <https://reviews.apache.org/r/51785/diff/6/?file=1510150#file1510150line49>
> >
> >     One int state is removed now. So we can reduce 2 to 1 now. Ya I can see the
double is removed now.

U mean still we have 2 ints?  I could see one int state got removed.


> On Sept. 27, 2016, 3:33 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
line 48
> > <https://reviews.apache.org/r/51785/diff/6/?file=1510150#file1510150line48>
> >
> >     Why changed from 4 to 2? We have 4 refs no?
> 
> Anastasia Braginsky wrote:
>     We have to references compactingMemStore, versionedList. What are the other references
to count?

One AtomicInt isInterrupted and Type (now it is Action)  Those are also references.


> On Sept. 27, 2016, 3:33 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java,
line 211
> > <https://reviews.apache.org/r/51785/diff/6/?file=1510149#file1510149line211>
> >
> >     Ok so here the succ boolean is true and so useMSLAB is of no relevance. Fine..
 Just add a comment here.
> 
> Anastasia Braginsky wrote:
>     So you say useMSLAB is always true? Why so? We didn't request that compacting memstore
to work only with MSLAB.

I mean in the called place useMSLAB is useful iff succ is false.  So whatever we pass here
make no diff.. Ya whatever u do is just fine.


- Anoop Sam


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


On Sept. 26, 2016, 8:57 p.m., Anastasia Braginsky wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51785/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2016, 8:57 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/MemStoreCompactorSegmentsIterator.java
PRE-CREATION 
>   hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreMergerSegmentsIterator.java
PRE-CREATION 
>   hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSegmentsIterator.java
PRE-CREATION 
>   hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java
510ebbd 
>   hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/VersionedSegmentsList.java
2e8bead 
>   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