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 01:03:00 GMT


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 111
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line111>
> >
> >     do we need to make these fields nullable?  usually they are true/false in the
java code.  
> >     
> >     Is this some semi-mechanical translation from a java api?

I use the same Avro record for table creation and modification as well as description. For
create table, I want the fields to be nullable because the user should not have to specify
a value.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 94
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line94>
> >
> >     the compression can never be null, because the "NONE" is the catch all here.

Same as below: I use the same record for family creation, modification, and description. Avro
currently doesn't have default values on write, so making this field nullable means we can
do smart things if the user doesn't specify a compression algorithm during Family creation.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 78
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line78>
> >
> >     same as deadServerNames.

Yeah I should make these 0-length arrays.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 73
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line73>
> >
> >     couldnt you use a empty string if there are no dead server names?  im not sure
if arrays can be 0 length in avro :-)

Will make a 0-length array


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 66
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line66>
> >
> >     technically the serverName is the serverAddress + startCode... in the Java code
is isnt fully exposed.  Not sure what we want to do here, but this is probably fine as is.

Yeah since Avro records don't have methods, you can think of this field as a materialization
of the Java logic.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 34
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line34>
> >
> >     you can probably just use 'hostname' and 'port'.  There was a recent patch in
trunk that is attempting to get rid of IP addresses (they cause issues when they dont align
with DNS names, etc) and generally move us to a DNS name world.

Let me know what you want me to do here. I was just copying the fields directly from the Java
objects.


- Jeff


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


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