> On Sept. 27, 2016, 10:03 a.m., Anoop Sam John wrote: > > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSegmentsIterator.java, > > line 36 > > <https://reviews.apache.org/r/51785/diff/6/?file=1510154#file1510154line36> > > > > The 2 impls of this have diff wrt Scanner creation only no? In case of > > merge we dont need SQM and can use max long as read point > > In case of compaction we use SQM and smallest read pnt. > > Other than this, the hasNext() and next() can be same logic no? Only > > we can have createScanner as abstarct method and impl in both places?
The difference between MemStoreMergerSegmentsIterator and MemStoreCompactorSegmentsIterator is that MemStoreCompactorSegmentsIterator implements StoreScanner (compactingScanner) on top of the MemStoreScanner (scanner) and MemStoreMergerSegmentsIterator uses the MemStoreScanner (scanner) directly. The hasNext() and next() of MemStoreCompactorSegmentsIterator need to keep managing of the KVS (as old MemStoreCompactorIterator did) and hasNext() and next() of MemStoreMergerSegmentsIterator just use MemStoreScanner directly. Do you say that MemStoreMergerSegmentsIterator should also build StoreScanner on top of the MemStoreScanner and just not to use SQM? But why that additional code is needed? > On Sept. 27, 2016, 10:03 a.m., Anoop Sam John wrote: > > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSegmentsIterator.java, > > line 57 > > <https://reviews.apache.org/r/51785/diff/6/?file=1510154#file1510154line57> > > > > Are we doing wat the comment says? We use smallestReadPnt and comments > > says max possible. Fixed the comment. Is it bad that also for the merge we are going to return only relevant cells? This doesn't involves SQM anyway... > On Sept. 27, 2016, 10:03 a.m., Anoop Sam John wrote: > > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java, > > line 95 > > <https://reviews.apache.org/r/51785/diff/6/?file=1510155#file1510155line95> > > > > This is kind of merging all the segent's MemstoreLABs into one and then > > pass that to the result segment. Can have a such a method and in the > > ineterface? Interface of what? MSLAB? I think MSLAB interface is going to be refactored significantly with your offHeapMSLAB, let's do it all there... > On Sept. 27, 2016, 10:03 a.m., Anoop Sam John wrote: > > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java, > > line 97 > > <https://reviews.apache.org/r/51785/diff/6/?file=1510155#file1510155line97> > > > > This typecasting is ok as of now. But when we have diff kind of MSLAB > > impl will be an issue. (OffheapMSLAB) I prefer to deal with it all then... - 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 > >
