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

Reply via email to