hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeff Hammerbacher" <jeff.hammerbac...@gmail.com>
Subject Re: Review Request: HBASE-2400: new connector for Avro RPC access to HBase cluster
Date Thu, 10 Jun 2010 02:56:33 GMT


> On 2010-06-09 19:24:25, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.avpr, line 1
> > <http://review.hbase.org/r/128/diff/3/?file=1190#file1190line1>
> >
> >     if this is generated from the genavro, is it possible to get a maven rule to
generate this?  Or is that not ready yet?

Yes, this should definitely be done during the build. See https://issues.apache.org/jira/browse/AVRO-572.


> On 2010-06-09 19:24:25, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 155
> > <http://review.hbase.org/r/128/diff/3/?file=1191#file1191line155>
> >
> >     The Java API gets its speed by essentially taking a Result which is an array
of KeyValue, which are just byte arrays and serializing it all as one large array.  On the
client side, the client reads the entire array then builds the KeyValues that provide a view
onto this one array. 
> >     
> >     I don't know how this performance improvement could be done in this avro interface,
but I thought I'd bring it up for reference.

My comment here is not for performance considerations, it's for concision and related to your
previous comment (on line 140): AColumn, AResultEntry, and AColumnValue all do approximately
the same thing. I could make the fields nullable and use one Avro record for each. Pro: I
have less generated classes. Con: the generated class I have is less task-specific. To be
honest, since there are not a lot of Avro services out there, it's hard to say which is the
best practice. I'm happy to take feedback but decided that being more verbose with my number
of objects was better.


> On 2010-06-09 19:24:25, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 156
> > <http://review.hbase.org/r/128/diff/3/?file=1191#file1191line156>
> >
> >     it would be nice to collapse AResultEntry and AColumnValue, they seem to be
almost the same thing.

(see above comment)


> On 2010-06-09 19:24:25, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 268
> > <http://review.hbase.org/r/128/diff/3/?file=1191#file1191line268>
> >
> >     these apis are good, but i'm wondering if you'd be open to a new experimental
scanner API we have been interested in for the base RPC...
> >     
> >     essentially right now you need 3 RPC calls even to retrieve a small amount of
data.  What would an API look like that lets you open, get rows and have implicit closes if
you hit the end of the scan in your 'number of records' parameter?  We'd still have explicit
closes for premature client-driven scan-terminations, but if your goal is to scan to the end,
then why do an explicit close?  Also why not have the 'open' also start to return data?  The
returned value would probably have to be a struct..
> >     
> >     This is more of an optional exercise, so if you dont feel the need, it's fine.

Yeah that would be nice; you could return (int scannerId, bytes[] row, resultScanner result).
In the Python client, I don't expose open/close; the Python clients just scan.


> On 2010-06-09 19:24:25, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 230
> > <http://review.hbase.org/r/128/diff/3/?file=1191#file1191line230>
> >
> >     technically speaking getRowOrBefore() isnt a 'public' method, it is supposed
to be mostly used for META support, and I think we are trending to 'dont use for general purpose'.

Noted. I will remove the comment.


- Jeff


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


On 2010-06-09 18:22:12, Jeff Hammerbacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/128/
> -----------------------------------------------------------
> 
> (Updated 2010-06-09 18:22:12)
> 
> 
> 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 953193 
>   trunk/pom.xml 953193 
>   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