[
https://issues.apache.org/jira/browse/HBASE-10648?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13924710#comment-13924710
]
Anoop Sam John commented on HBASE-10648:
----------------------------------------
Thanks Stack for the review and comments.
bq.Should it be CellScanner?
We have to create StoreScanner from the scanner on snapshot. So this can not be
a CellScanner. We will need changes in many upper layers. May be later when
we make changes of KV- Cell in write later, we can think of higher level changes
bq.What happens if a MemStore implementation has N snapshots?
Can it be?
{code}
void snapshot() {
// If snapshot currently has entries, then flusher failed or didn't call
// cleanup. Log a warning.
if (!this.snapshot.isEmpty()) {
LOG.warn("Snapshot called again without clearing previous. " +
"Doing nothing. Another ongoing flush or did we fail last attempt?");
}
{code}
We dont take a new snapshot.
bq.Is the snapshot info a good idea? We need to pass it alongside a scanner?
Should the two be combined; a class that implements CellScanner and from which
you can get snapshot info? You ask SnapshotInfo for the CellScanner when you
need one?
Will do some changes here. Snapshot Scanner "has a" snapshot info ? Or it may
be better we have a Snapshot class which "has a" snapshotScanner and "has" some
info. Info includes cellsCount, size, TimeRangeTracker etc.. The snapshot()
call creates a snapshot instance and there needs to be a getSnapshot() this
returns a Snapshot which has all the things abt the snapshot. Sounds
bq.We need to pass it alongside a scanner?
So we pass the Snapshot. Get scanner or info wherever needed.
bq.And oh, it took me a while... yeah, a class in an Interface as per Andrew is
ahem, 'unorthodox' ... smile. It should be an Interface too?
Fine. I will move out the class from the interface. What I thought was this
SnapshotInfo is relevant only within the MemStore scope and not a top level
thing. This is a POJO and so an interface might not be needed. Now as per the
above comment fix there wont be some thing like SnapshotInfo class at all.
bq.Can this be about Cells rather than KVs since you are making a change:
You mean in TestHRegion.java ? Yes it can be
bq.WHy does this go away? testUpsertMSLAB
This was testing with usage of MemStore#updateColumnValue() This and
HStore#updateColumnValue() were used only in tests and I could see a TODO for
removal. We still need to keep this? If so it can be added to the interface also
bq.Is MSLAB an implementation detail on DefaultMemStore?
Kind of. It might not to be added in MemStore interface. But this MSLAB also
will be extracted to an interface impl way. The default impl will be on heap
MSLAB. May be some one can change to OffHeap MSLAB and plugin to
DefaultMemStore. Andy was also having this same question/suggestion. But
this will not be in this Jira but in a follow on issue :)
bq.MemStore#size is not heapSize or is? FYI, HBASE-10514 adds a method to
MemStore.
For DefaultMemStore both are same. But the size is supposed to return the size
of the memory the MemStore takes. (Whether on heap or off heap) The heapSize
is on heap size alone. After this patch, the heapSize() as such not having
much use. It is used in HRegion#heapSize() and this is not getting used any
where above the line. Oh let me see the patch for HBASE-10514. Any changes
as per that I can do the necessary change in this patch also (Once it goes in)
> Pluggable Memstore
> ------------------
>
> Key: HBASE-10648
> URL: https://issues.apache.org/jira/browse/HBASE-10648
> Project: HBase
> Issue Type: Sub-task
> Reporter: Anoop Sam John
> Assignee: Anoop Sam John
> Attachments: HBASE-10648.patch, HBASE-10648_V2.patch,
> HBASE-10648_V3.patch
>
>
> Make Memstore into an interface implementation. Also make it pluggable by
> configuring the FQCN of the impl.
> This will allow us to have different impl and optimizations in the Memstore
> DataStructure and the upper layers untouched.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
