hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gary Helmling (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5448) Support for dynamic coprocessor endpoints with PB-based RPC
Date Thu, 20 Dec 2012 19:41:13 GMT

    [ https://issues.apache.org/jira/browse/HBASE-5448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13537300#comment-13537300
] 

Gary Helmling commented on HBASE-5448:
--------------------------------------

[~saint.ack@gmail.com] Comments below:

bq. A thought. I think this pb-based way of doing dynamic cps elegant after looking at it
a while. I know 'elegant' is not the first thing that comes to mind when you have pb and rpc
in the mix, but hey, can't keep it to myself.

I'm glad someone sees it that way :)  It is at least consistent, if a little verbose.  I think
it could be more "elegant" if we did a custom PB compiler plugin and required service implementors
to compile their endpoint definitions with our own script or build target.  Then I think we
could control the generated service method signatures.  Maybe if I'm feeling especially crazy
I'll check that out over the holidays.  But I wouldn't consider actually shipping that unless
it significantly simplified these cases and didn't require additional mass changes.

bq. One thing I think we have lost though going to this new mechanism is the ability to do
generics: i.e. GenericProtocol over in hbase-server/src/test can't be made work now. I believe
this so because pb requires you specify a type: https://developers.google.com/protocol-buffers/docs/proto#simple
Do you agree G?

Yes, I agree, though in a way generics don't even apply with the use of protobufs.  Services
could do more dynamic interpretation of messages, but it would be up to them to implement
that in a way that made sense for the specific case.  I don't think there's anything we need
to do to support this.

bq. How to do exceptions in a coprocessor? I see where we set the exception on the controller
if there is one, but should we then abandon further processing – return? We need to call
the RpcCallback done, though, right?

Yes, the exception should be set on the controller in order to be returned to the client.
 It seems to be good practice to always call RpcCallback.done(), but it's not strictly required
for endpoint implementations and it should also be fine to pass a null argument in the case
of an exception.  Your implementation looks fine to me, assuming that "sum" is a required
field in the proto message, otherwise you could skip setting the dummy value in the response
on an exception.

One additional idea would be to define a custom unchecked exception (EndpointException extends
RuntimeException?) which we could watch for and use to set the exception in the controller,
but either with this or the current ResponseConverter.setControllerException() we're relying
on convention over a real contract, which doesn't seem great.

                
> Support for dynamic coprocessor endpoints with PB-based RPC
> -----------------------------------------------------------
>
>                 Key: HBASE-5448
>                 URL: https://issues.apache.org/jira/browse/HBASE-5448
>             Project: HBase
>          Issue Type: Sub-task
>          Components: IPC/RPC, master, migration, regionserver
>            Reporter: Todd Lipcon
>            Assignee: Gary Helmling
>             Fix For: 0.96.0
>
>         Attachments: HBASE-5448_2.patch, HBASE-5448_3.patch, HBASE-5448_4.patch, HBASE-5448.patch
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message