You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "George Porter (JIRA)" <ji...@apache.org> on 2009/07/06 19:33:14 UTC

[jira] Created: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Add plug-in mechanism for manipulating per-call metadata
--------------------------------------------------------

                 Key: AVRO-76
                 URL: https://issues.apache.org/jira/browse/AVRO-76
             Project: Avro
          Issue Type: Sub-task
            Reporter: George Porter
            Assignee: George Porter




-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734672#action_12734672 ] 

George Porter commented on AVRO-76:
-----------------------------------

One question I had, was when we read the handshake, the "meta" field is just listed as an Object.  Thus, I did the following:

try {
      sessMeta = (Map<Utf8, ByteBuffer>) handshake.meta;
    } catch (ClassCastException ce) {
      throw new AvroRuntimeException("Received improper session metadata type: " + ce);
    }

Is that necessary, and if so, is catching ClassCastException the most efficient way to handle that situation?

Your thoughts are appreciated.

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737349#action_12737349 ] 

George Porter commented on AVRO-76:
-----------------------------------

Doug,

In response to your question about why I copied the maps instead of directly setting them, I was concerned about a map changing after the set() method was called, resulting in hard to track down bugs.  Looking back over the code, that doesn't seem to be an issue, since the scope of those Maps end shortly after the set() call is invoked.

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

Removes the CastClassException checks when reading in the meta field, which is a Union type.  Instead, we check if it is null, and if so, create an empty local map.  Otherwise, we just directly cast it to the appropriate type

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

This patch addresses the three issues Doug mentioned in the previous thread:

1) metadata map is passed into instrumentation methods
2) multiple plugins can be registered
3) plugins are added via methods, not constructors

Additionally, this patch also provides support for per-session state (as requested in AVRO-65).  The API uses slightly different names from AVRO-65, but the functionality should be equivalent.

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting resolved AVRO-76.
------------------------------

       Resolution: Fixed
    Fix Version/s: 1.0.1

I just committed this.  Thanks, George!

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>             Fix For: 1.0.1
>
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734687#action_12734687 ] 

Doug Cutting commented on AVRO-76:
----------------------------------

> the "meta" field is just listed as an Object

It's a union between the Map<Utf8,ByteBuffer> and null.  The Java code generator is not yet smart enough to special case unions with null and generates Object for all union types.  So the right thing to do is handle null here, since the metadata is optional.  You should not need to handle ClassCastException explicitly, since it should never happen, but null might.


> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated AVRO-76:
-----------------------------

    Component/s: java

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735775#action_12735775 ] 

Doug Cutting commented on AVRO-76:
----------------------------------

 - Requestor should not have fields for metadata: that's not thread safe. Rather, each request should create a new metadata map, pass it to plugins to be populated, write it to the server, then drop it.  Similarly, a new connect metadata map should be created per handshake.
 - It might be simpler if {send,recieve}{error,response} passed an isError boolean, removing two methods from the plugin API.
 - I don't see a need for the clearRPCPlugins method.
 - Long-term, we're going to want to pass more data to these plugins.  E.g., someone is going to want the transciever or somesuch.  I suggest that we should use context objects as parameters, that we can add methods to to access new data.  So, we might have a SessionContext that has a getProtocol()  and getSessionMeta() method, and a RequestContext that extends this with methods getRequestMeta(), getMessage(), getRequest(), etc., and a ResponseContext that extends this with getResponseMeta(), getResponse(), isError(), etc..  Then, for example, we might call serverSendResponse(RequestContext, ResponseContext).  Does that make sense?

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

Renames the local variables "context" instead of rpcctx and sessctx

Directly sets map data structures in the RPCContext object to what is passed into them, rather than copying what was passed to them.

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

This patch clears out the session state before the handshake is written, and removes the ability to clear the list of RPC Plugins (to do that, you must close the connection and create a new one).



> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737283#action_12737283 ] 

Doug Cutting commented on AVRO-76:
----------------------------------

This is looking great!  A couple of questions:
 - can we name the local variable just 'context' rather than 'rpcctxt'?
 - in setXXXMeta, i don't understand why you copy data rather than just set context field.  are there cases where these may already have values?


> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728959#action_12728959 ] 

Doug Cutting commented on AVRO-76:
----------------------------------

Some quick comments:
 - Requestor and Responder should each support a list of plugins, not just a single plugin, so that one might implement authentication, authorization, tracing and other orthogonal mechanisms each as a plugin.
 - Plugins should be added with an addPlugin() method, not through constructors.  Over time, adding optional parameters leads to confusing parameter lists.  In the case of getClient(), we might change the signature to accept a Requestor
 - Metadata map should be passed as a parameter to the plugin's methods.



> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

This patch implements a plug-in mechanism for manipulating per-call metadata that was recently committed.

Several of the interfaces used to create Requestors and Responders, as well as the {code}getProxy(){code} methods used to instantiate RPC proxies have been extended to allow for the user to specify a class that will be used to set, query, and manipulate the per-call metadata.

People wanting to use this mechanism can create a subclass of {code}CallMetaHandler{code}, overriding the behavior of the instrumentation points that are called at key points in the code.  Those instrumentation points are:

{code}

public void clientSendRequest(String msgName, Schema s, Object request) { }
public void serverReceiveRequest(String msgName, Schema s, Object request) { }
public void serverSendResponse(String msgName, Schema s, Object response) { }
public void serverSendError(String msgName, Schema s, AvroRemoteException error) { }
public void clientReceiveResponse(String msgName, Schema s, Object response) { }
public void clientReceiveError(String msgName, Schema s, AvroRemoteException error) { }
{code}

In addition to these instrumentation APIs being called during the RPC exchange, they can also set or query per-call path state.  This path state is passed along the RPC.

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734351#action_12734351 ] 

Doug Cutting commented on AVRO-76:
----------------------------------

It seems error-prone to have connection and call metadata maps survive longer than the connection or call.  Rather, the Requestor should start with empty metadata for each connection and call, then plugins can populate the metadata before it is sent, then it is discarded.  The patch already handles Response metadata in this manner.

Also, calling clearRPCplugins seems error prone too.  This method is not used in the patch.  Let's leave it out for now.

Other than that, this is looking good.


> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12730629#action_12730629 ] 

George Porter commented on AVRO-76:
-----------------------------------

Thank you for the comments.  I'm working on an updated patch, but due to a busy week ahead, probably won't be able to get to it for another week.  Thanks!

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-76) Add plug-in mechanism for manipulating per-call metadata

Posted by "George Porter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-76?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

George Porter updated AVRO-76:
------------------------------

    Attachment: AVRO-76.patch

I overhauled the plug-in API to replace all of the individual parameters with a single RPCContext object.  This will make evolving the plug-in framework much easier in the future.  Right now the RPCContext holds a minimal amount of information, but as we need more, we can easily add new (protected) fields and public methods to that class.

Got rid of the fields in the requestor, and so we should be thread safe now

Got rid of the error methods--that is handled by an isError() method in the RPCContext, and a response() and error() methods for accessing either the response object or the AvroRemoteException

> Add plug-in mechanism for manipulating per-call metadata
> --------------------------------------------------------
>
>                 Key: AVRO-76
>                 URL: https://issues.apache.org/jira/browse/AVRO-76
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: George Porter
>            Assignee: George Porter
>         Attachments: AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch, AVRO-76.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.