Its not clear why org.apache.hadoop.hbase.client.coprocessor.Exec implements
Row interface. It seems the row attribute is not used as such apart from
passing it to server side as a part of Exec object. Is this intentional or I
am missing something.

Though its comparatively older code now, but still would be good to know the
reason.

Thanks,
Himanshu

I took a look.  Could not figure it out.  I think 'reference row'
needs to be explained better (or implementation of Row Interface
removed -- gives impression that CP is row scoped; I don't think that
the case).

St.Ack

On Wed, Apr 27, 2011 at 9:08 PM, Himanshu Vashishtha
<[EMAIL PROTECTED]> wrote:
> Its not clear why org.apache.hadoop.hbase.client.coprocessor.Exec implements
> Row interface. It seems the row attribute is not used as such apart from
> passing it to server side as a part of Exec object. Is this intentional or I
> am missing something.
>
> Though its comparatively older code now, but still would be good to know the
> reason.
>
> Thanks,
> Himanshu
>

This is a remnant from initial plans to do single-RPC-per-RS batching of
coprocessor RPCs via the parameterized
HConnectionManager.HConnectionImplementation.processBatchCallback() method.
Currently we do a single RPC per region for HTable.coprocessorExec()
invocations.

processBatchCallback() takes a List<? extends Row>, hence the "implements"
for Exec.

I had originally planned on doing this enhancement (there was one hurdle in
scoping of some vars within ExecRPCInvoker as I recall), but obviously
haven't gotten around to it, so feel free to remove the Row implementation.
I don't think it's needed for the current
HConnectionManager.HConnectionImplementation.processExecs() method.

I don't really agree that this is a big issue of confusion though.  Exec is
part of the internal implementation and referenceRow has local javadoc.
Exec class javadoc also references HTable.coprocessorExec javadoc, which
goes to lengths to explain how the row key is used.  But, again, the Row
implementation isn't currently used, so it's just dead weight.  Feel free to
drop it.

--gh
On Wed, Apr 27, 2011 at 10:06 PM, Stack <[EMAIL PROTECTED]> wrote:

> I took a look.  Could not figure it out.  I think 'reference row'
> needs to be explained better (or implementation of Row Interface
> removed -- gives impression that CP is row scoped; I don't think that
> the case).
>
> St.Ack
>
> On Wed, Apr 27, 2011 at 9:08 PM, Himanshu Vashishtha
> <[EMAIL PROTECTED]> wrote:
> > Its not clear why org.apache.hadoop.hbase.client.coprocessor.Exec
> implements
> > Row interface. It seems the row attribute is not used as such apart from
> > passing it to server side as a part of Exec object. Is this intentional
> or I
> > am missing something.
> >
> > Though its comparatively older code now, but still would be good to know
> the
> > reason.
> >
> > Thanks,
> > Himanshu
> >
>

On Wed, Apr 27, 2011 at 10:47 PM, Gary Helmling <[EMAIL PROTECTED]> wrote:

> This is a remnant from initial plans to do single-RPC-per-RS batching of
> coprocessor RPCs via the parameterized
> HConnectionManager.HConnectionImplementation.processBatchCallback() method.
> Currently we do a single RPC per region for HTable.coprocessorExec()
> invocations.
>
> processBatchCallback() takes a List<? extends Row>, hence the "implements"
> for Exec.
>
>
Actually, as a side effect of this, you _could_ currently construct a bunch
of Exec objects and call HTable.batch(List<Exec>) and I think it would work,
returning you a ExecResult[].  So I take back the part about it being
internal implementation only.  On the one hand, this could be useful, on the
other potentially confusing.  It depends where we want to take the CP RPC
stuff.

If we drop the Row implementation from Exec, we should also strip out the
Exec handling in HRegionServer.multi() and any other places.

--gh

For the least of it, one can get rid of "private byte[] referenceRow"
attribute in Exec class I'd say (?).

Yes, Exec implementing Row seems useful as you mentioned (though this use
case is not that much stressed upon I believe. I mean no test cases etc).
Pardon if i have missed them.

Thanks,
Himanshu

On Thu, Apr 28, 2011 at 12:04 AM, Gary Helmling <[EMAIL PROTECTED]> wrote:

> On Wed, Apr 27, 2011 at 10:47 PM, Gary Helmling <[EMAIL PROTECTED]>
> wrote:
>
> > This is a remnant from initial plans to do single-RPC-per-RS batching of
> > coprocessor RPCs via the parameterized
> > HConnectionManager.HConnectionImplementation.processBatchCallback()
> method.
> > Currently we do a single RPC per region for HTable.coprocessorExec()
> > invocations.
> >
> > processBatchCallback() takes a List<? extends Row>, hence the
> "implements"
> > for Exec.
> >
> >
> Actually, as a side effect of this, you _could_ currently construct a bunch
> of Exec objects and call HTable.batch(List<Exec>) and I think it would
> work,
> returning you a ExecResult[].  So I take back the part about it being
> internal implementation only.  On the one hand, this could be useful, on
> the
> other potentially confusing.  It depends where we want to take the CP RPC
> stuff.
>
> If we drop the Row implementation from Exec, we should also strip out the
> Exec handling in HRegionServer.multi() and any other places.
>
> --gh
>

On Wed, Apr 27, 2011 at 11:14 PM, Himanshu Vashishtha <
[EMAIL PROTECTED]> wrote:

> For the least of it, one can get rid of "private byte[] referenceRow"
> attribute in Exec class I'd say (?).
>
>
Um, what would Row.getRow() return in that case?  Sure, if you drop the Row
implementation, you can probably drop referenceRow too (the row key seems to
be in context for any of the other places we would need it).
--gh

Right, they go hand in hand. I saw the use case in multi call at RS where
all the rows are sorted based on the row attribute.

Himanshu

On Thu, Apr 28, 2011 at 12:33 AM, Gary Helmling <[EMAIL PROTECTED]> wrote:

> On Wed, Apr 27, 2011 at 11:14 PM, Himanshu Vashishtha <
> [EMAIL PROTECTED]> wrote:
>
> > For the least of it, one can get rid of "private byte[] referenceRow"
> > attribute in Exec class I'd say (?).
> >
> >
> Um, what would Row.getRow() return in that case?  Sure, if you drop the Row
> implementation, you can probably drop referenceRow too (the row key seems
> to
> be in context for any of the other places we would need it).
>
>
> --gh
>

Oh no, one use it for sorting in the multi method there in RS. forget what i
said :))

On Thu, Apr 28, 2011 at 12:14 AM, Himanshu Vashishtha <
[EMAIL PROTECTED]> wrote:

> For the least of it, one can get rid of "private byte[] referenceRow"
> attribute in Exec class I'd say (?).
>
> Yes, Exec implementing Row seems useful as you mentioned (though this use
> case is not that much stressed upon I believe. I mean no test cases etc).
> Pardon if i have missed them.
>
> Thanks,
> Himanshu
>
>
> On Thu, Apr 28, 2011 at 12:04 AM, Gary Helmling <[EMAIL PROTECTED]>wrote:
>
>> On Wed, Apr 27, 2011 at 10:47 PM, Gary Helmling <[EMAIL PROTECTED]>
>> wrote:
>>
>> > This is a remnant from initial plans to do single-RPC-per-RS batching of
>> > coprocessor RPCs via the parameterized
>> > HConnectionManager.HConnectionImplementation.processBatchCallback()
>> method.
>> > Currently we do a single RPC per region for HTable.coprocessorExec()
>> > invocations.
>> >
>> > processBatchCallback() takes a List<? extends Row>, hence the
>> "implements"
>> > for Exec.
>> >
>> >
>> Actually, as a side effect of this, you _could_ currently construct a
>> bunch
>> of Exec objects and call HTable.batch(List<Exec>) and I think it would
>> work,
>> returning you a ExecResult[].  So I take back the part about it being
>> internal implementation only.  On the one hand, this could be useful, on
>> the
>> other potentially confusing.  It depends where we want to take the CP RPC
>> stuff.
>>
>> If we drop the Row implementation from Exec, we should also strip out the
>> Exec handling in HRegionServer.multi() and any other places.
>>
>> --gh
>>
>
>

Yes, CP are region scoped.
One use is its extending Writable via Row and Execs are being used in RPC,
but that's a lame one I know :)

Himanshu

On Wed, Apr 27, 2011 at 11:06 PM, Stack <[EMAIL PROTECTED]> wrote:

> I took a look.  Could not figure it out.  I think 'reference row'
> needs to be explained better (or implementation of Row Interface
> removed -- gives impression that CP is row scoped; I don't think that
> the case).
>
> St.Ack
>
> On Wed, Apr 27, 2011 at 9:08 PM, Himanshu Vashishtha
> <[EMAIL PROTECTED]> wrote:
> > Its not clear why org.apache.hadoop.hbase.client.coprocessor.Exec
> implements
> > Row interface. It seems the row attribute is not used as such apart from
> > passing it to server side as a part of Exec object. Is this intentional
> or I
> > am missing something.
> >
> > Though its comparatively older code now, but still would be good to know
> the
> > reason.
> >
> > Thanks,
> > Himanshu
> >
>

hehe its extending Invocation too, so I take this use case back :))

On Wed, Apr 27, 2011 at 11:27 PM, Himanshu Vashishtha <
[EMAIL PROTECTED]> wrote:

> Yes, CP are region scoped.
> One use is its extending Writable via Row and Execs are being used in RPC,
> but that's a lame one I know :)
>
> Himanshu
>
>
> On Wed, Apr 27, 2011 at 11:06 PM, Stack <[EMAIL PROTECTED]> wrote:
>
>> I took a look.  Could not figure it out.  I think 'reference row'
>> needs to be explained better (or implementation of Row Interface
>> removed -- gives impression that CP is row scoped; I don't think that
>> the case).
>>
>> St.Ack
>>
>> On Wed, Apr 27, 2011 at 9:08 PM, Himanshu Vashishtha
>> <[EMAIL PROTECTED]> wrote:
>> > Its not clear why org.apache.hadoop.hbase.client.coprocessor.Exec
>> implements
>> > Row interface. It seems the row attribute is not used as such apart from
>> > passing it to server side as a part of Exec object. Is this intentional
>> or I
>> > am missing something.
>> >
>> > Though its comparatively older code now, but still would be good to know
>> the
>> > reason.
>> >
>> > Thanks,
>> > Himanshu
>> >
>>
>
>