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