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