hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Purtell" <apurt...@apache.org>
Subject Re: Review Request: HBASE-2400: new connector for Avro RPC access to HBase cluster
Date Sun, 06 Jun 2010 17:23:33 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/128/#review138
-----------------------------------------------------------


Looks good Jeff. Some missing functionality IMHO. See my individual comments.


trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment726>

    That would be good.



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment727>

    No split()?



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment728>

    Export exists() to the client.



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment729>

    Really appreciate this, which will help out with Avro encoding support in the REST interface.
We should find a way for all of the connectors to share common Avro, Thrift, and Protobuf
packages. I will refactor the protobuf stuff soon.



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment730>

    Yes. And all of the connectors should drop such requests, throw exceptions, and warn upon
similar events.



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment731>

    Or just handle this generically as a string?



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment732>

    Or just handle this generically as a string?



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment733>

    0-length array.



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment734>

    See comment on AvroServer in this regard.



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment735>

    How do you deal with user attributes? A column can have an arbitrary set of them. Coprocessors
will use this facility. Not necessary to support attribute access via fields of the descriptors
if there are RPC methods available to read or write them.  



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment736>

    Likewise, no support for user attributes here.



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment737>

    No support for filters. For REST, the model classes recursively walk the filter structure
and build a JSON representation that is then passed as a string.
    
    No support for setBatch()
    
    Consider support for setCacheBlocks() (regionserver level caching) and setCaching() (connector
level caching)?



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment738>

    Missing split()



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment739>

    Missing attribute get and set, if not supporting read/write access via descriptor structs.



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment740>

    Missing attribute get and set, if not supporting read/write access via descriptor structs.



trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro
<http://review.hbase.org/r/128/#comment741>

    Missing exists()


- Andrew


On 2010-06-05 23:16:10, Jeff Hammerbacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/128/
> -----------------------------------------------------------
> 
> (Updated 2010-06-05 23:16:10)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Initial patch; some javadoc and tests missing, but I wanted to get some initial feedback
on the approach. My apologies for sticking a patch on the JIRA before the review. I should
have read further on the HowToContribute JIRA.
> 
> 
> This addresses bug HBASE-2400.
> 
> 
> Diffs
> -----
> 
>   trunk/bin/hbase 951826 
>   trunk/pom.xml 951826 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AAlreadyExists.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AClusterStatus.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumn.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnFamilyDescriptor.java
PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnValue.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ACompressionAlgorithm.java
PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ADelete.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AFamilyDescriptor.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AGet.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIOError.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIllegalArgument.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AMasterNotRunning.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/APut.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ARegionLoad.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResult.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResultEntry.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AScan.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerAddress.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerInfo.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerLoad.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableDescriptor.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableExists.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATimeRange.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/HBase.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/IOError.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/TCell.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.avpr PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/package.html PRE-CREATION 
>   trunk/src/test/java/org/apache/hadoop/hbase/avro/TestAvroServer.java PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/128/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jeff
> 
>


Mime
View raw message