[
https://issues.apache.org/jira/browse/HBASE-14919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15132267#comment-15132267
]
Eshcar Hillel commented on HBASE-14919:
---------------------------------------
bq. Should we set -1 to when create instance of this class? Define -1 as
NO_SNAPSHOT_ID?
Definitely, yes.
bq. This javadoc seems wrong.
I will fix this.
bq. So, an ImmutableSegment takes a MutableSegment on construction? You can
skip the pupa ImmutableSegmentAdapter? (It is the only implementation of
ImmutableSegment).
Task #4 in the umbrella issue implements another ImmutableSegment
(CellBlocksSrgment), which does not take MutableSegment on construction.
Would you like better the name ImmutableSegmentWrapper?
bq. Reading the MutableSegment looking at the methods it has, why can't they
all be in ImmutableSegment?
We could simplify the design by removing the distinction between mutable and
immutable segments but this would be at the cost of implementing unnecessary
API, possibly not efficiently.
Consider tailSet(firstCell), it is similar for first() and getComparator().
tailSet is used in the methods AbstractMemStore::upsert() and
AbstractMemStore::updateColumnValue() where it is applied only to the active
(mutable) segment, and in methods of MutableCellSetSegment and
MutableCellSetSegmentScanner. It is not needed in any use case for an immutable
segment. However, its implementation in CellBlocksSegment would be inefficient
since it will incur costly traversal over all cell blocks plus allocating many
new objects to be stored in the result sorted set.
It is possible to do it this way. We believe exposing an unnecessary API which
incurs costly implementation is not advisable.
bq. And why this subset of Set methods in MutableSegment?
We believe it is good practice to only expose the minimal necessary
functionality.
bq. In ImmutableSegment, there is this method getScannerForMemStoreSnapshot().
As I read it, I am getting a Scanner on a MemStoreSnapshot...
No, this is incorrect. This method returns a KeyValueScanner that is used *by*
MemStoreSnapshot, but _it scans the cells in the segment who generated it_.
Would you like better the name getKeyValueScanner()? And the other method would
be getSegmentScanner().
bq. What is special about MutableCellSetSegmentScanner? When would the Scanner
on a MutableSegment differ from Scanner on an ImmutableSegment? In other words,
could we just have a SegmentScanner implementation and it work for both Segment
types?
Each scanner depends on the segment it is scanning.
For example, consider the method MutableCellSetSegmentScanner::backwardSeek() -
it seeks forward then go backward
{code}
public boolean backwardSeek(Cell key) throws IOException {
seek(key);
if (peek() == null || segment.compareRows(peek(), key) > 0) {
return seekToPreviousRow(key);
}
return true;
}
{code}
There is no reason for CellBlocksSegment to implement it this way since the
cells are contiguous and therefore allow for much simpler traversal.
> Infrastructure refactoring for In-Memory Flush
> ----------------------------------------------
>
> Key: HBASE-14919
> URL: https://issues.apache.org/jira/browse/HBASE-14919
> Project: HBase
> Issue Type: Sub-task
> Affects Versions: 2.0.0
> Reporter: Eshcar Hillel
> Assignee: Eshcar Hillel
> Attachments: HBASE-14919-V01.patch, HBASE-14919-V01.patch,
> HBASE-14919-V02.patch, HBASE-14919-V03.patch, HBASE-14919-V04.patch,
> HBASE-14919-V04.patch, HBASE-14919-V05.patch, HBASE-14919-V06.patch,
> HBASE-14919-V06.patch, HBASE-14919-V07.patch, HBASE-14919-V08.patch,
> HBASE-14919-V09.patch, HBASE-14919-V10.patch
>
>
> Refactoring the MemStore hierarchy, introducing segment (StoreSegment) as
> first-class citizen and decoupling memstore scanner from the memstore
> implementation.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
