[ https://issues.apache.org/jira/browse/HBASE-17655?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15891561#comment-15891561 ]
ramkrishna.s.vasudevan commented on HBASE-17655: ------------------------------------------------ If the comment from Anoop is fine to go and there is one comment in RB from me. If those are addressed/fixed then we can go for commit. Thanks. I can take up the commit to master branch once the above are resolved. > Removing MemStoreScanner and SnapshotScanner > -------------------------------------------- > > Key: HBASE-17655 > URL: https://issues.apache.org/jira/browse/HBASE-17655 > Project: HBase > Issue Type: Improvement > Components: Scanners > Affects Versions: 2.0.0 > Reporter: Eshcar Hillel > Assignee: Eshcar Hillel > Attachments: HBASE-17655-V01.patch, HBASE-17655-V02.patch, > HBASE-17655-V03.patch, HBASE-17655-V04.patch, HBASE-17655-V05.patch, > HBASE-17655-V05.patch, HBASE-17655-V06.patch > > > With CompactingMemstore becoming the new default, a store comprises multiple > memory segments and not just 1-2. MemStoreScanner encapsulates the scanning > of segments in the memory part of the store. SnapshotScanner is used to scan > the snapshot segment upon flush to disk. > Having the logic of scanners scattered in multiple classes (StoreScanner, > SegmentScanner, MemStoreScanner, SnapshotScanner) makes maintainance and > debugging challenging tasks, not always for a good reason. > For example, MemStoreScanner has a KeyValueHeap (KVH). When creating the > store scanner which also has a KVH, this makes a KVH inside a KVH. Reasoning > about the correctness of the methods supported by the scanner (seek, next, > hasNext, peek, etc.) is hard and debugging them is cumbersome. > In addition, by removing the MemStoreScanner layer we allow store scanner to > filter out each one of the memory scanners instead of either taking them all > (in most cases) or discarding them all (rarely). > SnapshotScanner is a simplified version of SegmentScanner as it is used only > in a specific context. However it is an additional implementation of the same > logic with no real advantage of improved performance. > Therefore, I suggest removing both MemStoreScanner and SnapshotScanner. The > code is adjusted to handle the list of segment scanners they encapsulate. > This fits well with the current code since in most cases at some point a list > of scanner is expected, so passing the actual list of segment scanners is > more natural than wrapping a single (high level) scanner with > Collections.singeltonList(...). -- This message was sent by Atlassian JIRA (v6.3.15#6346)