Himanshu: I attached my latest implementation to the JIRA. I also blogged about my thoughts. Most of your comments have been addressed.
It's now up to user to choose type T which in our case would be Double. For item c, after code review, we can separate LongColumnInterpreter into its own file. We can also add DoubleColumnInterpreter, etc. Cheers On Thu, Mar 31, 2011 at 11:49 PM, Himanshu Vashishtha < [email protected]> wrote: > I think with the introduction of having a generic interface it is moving in > a right direction, but I have few questions/suggestions: > > a) Considering the use case of having MeasureWritable in hbase (I assume > one cell has one such object): Are you happy to have a long return type. Or > you should have a MeasureWritable (MW) object as the return type (like the > MW object that is maximum etc). In later case, we can have a generic method > like: > > AggregateCpProtocol Interface: > public <T> T getMax(byte[] colFamily, byte[] colQualifier, byte[] > startRow, > byte[] endRow, ColumnInterpreter<T> ci) throws IOException; > > AggregationClient: > > public static class MWColumnInterpreter implements ColumnInterpreter<MW> { > ///// impl of getValue.. > } > and its usage will be: > final ColumnInterpreter<MW> ci = new MWColumnInterpreter(); > > public Long call(AggregateCpProtocol instance) throws IOException { > return instance.getMax(colFamily, colQualifier, startKey, > endKey, ci); > } > > In AggregateProtocolImpl, one can use the above getValue (like you are > using in your current file). Btw, you didn't remove the previous > Bytes.toLong call and the validations stuff, so it is still overwriting your > changes. > > b) Having said that, there should be methods to compare two such objects > while computing min/max. Should the Interpreter implement Comparable. > Similarly, it should also support the add method (for computing the avg, > etc). Interface for standard deviation needs to be changed as its value is > always double. That can come later I guess. > > c) How the client side interface looks like. Previously it was > AggregationClient, but now, one can define his own Interpreter leaving some > work to be done by the end user. Or will it be like LongInterpreter becomes > the default interpreter. > > > Thanks, > Himanshu > > > > On Thu, Mar 31, 2011 at 12:37 PM, Ted Yu <[email protected]> wrote: > >> Renaming the subject to better reflect the nature of further discussion. >> >> There're two considerations for my current implementation attached to >> HBASE-1512. >> 1. User shouldn't modify HbaseObjectWritable directly for the new class >> which is to be executed on region server. >> 2. The reason for introducing interpreter is that we (plan to) store >> objects of MeasureWritable, a relatively complex class, in hbase. Using >> interpreter would give us flexibility in computing aggregates. >> >> Cheers >> >> On Thu, Mar 31, 2011 at 10:01 AM, Himanshu Vashishtha < >> [email protected]> wrote: >> >>> Hello Ted, >>> Did you add a new class: LongColumnInterpreter. Is this the new argument >>> type you want to define to pass along rpcs. For all such "new" argument >>> types, they should be supported/backed up with in the HbaseObjectWritable >>> class to read/write it on wire. Do we really need it, just wondering. >>> >>> Himanshu >>> >>> On Thu, Mar 31, 2011 at 10:52 AM, Ted Yu <[email protected]> wrote: >>> >>> > Hi, >>> > When I experimented with HBASE-1512, I got the following from >>> > HbaseObjectWritable: >>> > java.lang.UnsupportedOperationException: No code for unexpected class >>> > >>> > >>> org.apache.hadoop.hbase.client.coprocessor.AggregationClient$1LongColumnInterpreter >>> > >>> > I think there was initiative to support dynamic class registration in >>> > HbaseObjectWritable >>> > >>> > If someone can enlighten me on the above, that would be great. >>> > >>> >> >> >
