[
https://issues.apache.org/jira/browse/HBASE-13387?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486742#comment-14486742
]
stack commented on HBASE-13387:
-------------------------------
I went through the patch again. Here is a more thorough review.
Needs rebasing.
The Result changes are fine; allow a subclass that internally could be
ServerCell.
In BigDecimalColumnInterpreter, we add an overload that adds a getValue that
takes a ServerCell rather than Cell. Will this even work (given ServerCell is
sub interface of Cell)? In the end, all we are doing is taking Cell value and
making a BigDecimal of it. This seems like a function that would be easy to
encapsulate in a CellUtil.getBigDecimal(Cell) -- no need for this class to know
anything of ServerCell. This method seems like it needs severe cleanup anyways
since it takes byte array colFamily and colQualifier but then ignores these
params to use the passed Cell.
Ditto for DoubleColumnInterpreter and LongColumnInterpreter
The change to the ColumnInterpreter Interface would not be needed if you are
good w/ the above.
In ColumnCountGetFilter, the ServerCell is not used at all in filterKeyValue
(which should be filterCell?)... this change seems to be just ripple from
change to filter Interface.
In ColumnPaginationFilter, we have ServerCell in two places. The first time,
the Cell/ServerCell is used to compare qualifier. This seems easy enough to
hideaway in a CellComparator or CellUtil call. The second usage is a
createFirstOnRow, using KVU. If it was CellUtil.createFirstOnRow(Cell), the
fact that we were passing a ServerCell could be opaque until down deep in
CellUtil or CellComparator.
Side note, we have hasArray everywhere currently. For same cost we could change
this to instanceof ServerCell...
ColumnPrefixFilter seems same as above. These server package Filters so far
look like they do not need to know anything about ServerCell AND they will not
pay in perf for not knowing this.
ColumnRangeFilter looks similar to above... compare on qualifier and
createFirstOnRow (these filters look to share a bunch of code)
DependentColumnFilter makes no use of the passed Cell in filterKeyValue.
In filterRowCells, it iterates the Cells but just to look at timestamps
(something available in Cell Interface I believe -- no need of ServerCell).
FamilyFilter is looking at family. Could just pull it out of the Cell or have a
Filter.doCompare that takes a Cell.
The change in the Filter Interface to take ServerCell everywhere so far seems
unwarranted. Need to review more. Could as well take Cell going by above.
I will do more review in the morning.
> Add ServerCell an extension to Cell
> -----------------------------------
>
> Key: HBASE-13387
> URL: https://issues.apache.org/jira/browse/HBASE-13387
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver, Scanners
> Reporter: Anoop Sam John
> Assignee: Anoop Sam John
> Attachments: WIP_ServerCell.patch
>
>
> This came in btw the discussion abt the parent Jira and recently Stack added
> as a comment on the E2E patch on the parent Jira.
> The idea is to add a new Interface 'ServerCell' in which we can add new
> buffer based getter APIs, hasArray API etc. We will keep this interface
> @InterfaceAudience.Private
> Also we have to change the timestamp and seqId on Cells in server side. We
> have added new interfaces SettableSequenceId, SettableTimestamp for this. Now
> if we can add a ServerCell we can add the setter APIs there.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
