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 Tue, 27 Sep 2016 14:27:47 GMT


> On Sept. 27, 2016, 10:03 a.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

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
     }


> On Sept. 27, 2016, 10:03 a.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorSegmentsIterator.java,
line 61
> > <https://reviews.apache.org/r/51785/diff/6/?file=1510152#file1510152line61>
> >
> >     It can so happen that a compaction of N segments results in 0 cells. Ya all
cells are say TTL expired. We use the SQM based scanner here. So kvs will be empty here and
so kvsIterator will continue to be null. Below methods of hasNext() and next() dont have null
check and so can cause NPE.   More general Q. When this kind of a scenario comes, what we
should do? The result Segment is empty. So we can just remove the suffix segments?

First, added null check in hasNext() and next() methods. Second, if as result from compaction
we create empty segment, then this empty segment will replace all old segments in the pipeline.
If new cells will be added to the memstore, the empty segment is going to be replaced with
not empty one on next compaction. If not then empty segment will remain there forever, but
why should we care?


- Anastasia


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


On Sept. 26, 2016, 3:27 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, 3:27 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