hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-13387) Add ServerCell an extension to Cell
Date Thu, 09 Apr 2015 05:38:12 GMT

    [ 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)

Mime
View raw message