You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Patrick Wendell (JIRA)" <ji...@apache.org> on 2010/06/16 00:01:29 UTC

[jira] Created: (AVRO-578) Add bytesWritten() to Encoder Class

Add bytesWritten() to Encoder Class
-----------------------------------

                 Key: AVRO-578
                 URL: https://issues.apache.org/jira/browse/AVRO-578
             Project: Avro
          Issue Type: Sub-task
            Reporter: Patrick Wendell
            Assignee: Patrick Wendell


For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.

Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12880399#action_12880399 ] 

Patrick Wendell commented on AVRO-578:
--------------------------------------

Okay I think some re-factoring may need to be done to the RPCPugin module. The problem is that RPCContext's are confusingly used to pass information in both directions (eg requestor --> plugin and plugin --> requestor). Here is an example:

Reponder wants to generate a response, it needs to:

* Call plugin.serverSendResponse(context) to *get* the meta-data to encode in this response (from plugin)
* Generate encoded payload given the meta-data
* Call plugin.serverSendResponse(context) to *give* the response payload to the plugin

Obviously this cannot happen with only one call of serverSendResponse(context).

Not sure what the best way to change this is that will be extensible and useful to future plugin writers. One way would be to add add two calls in each place, one which interacts with plugin pre-encoding payload and the other after. That feels ugly to me... thoughts?

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Resolved: (AVRO-578) Add RPC Payload to RPCContext class

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

Doug Cutting resolved AVRO-578.
-------------------------------

     Hadoop Flags: [Reviewed]
    Fix Version/s: 1.4.0
       Resolution: Fixed

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>             Fix For: 1.4.0
>
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3, AVRO-578.patch.v4, AVRO-578.patch.v5
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Attachment: AVRO-578.patch

This patch adds RPC payload to RPCContext and populates it appropriately.

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add bytesWritten() to Encoder Class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

The easiest way to see the size might be to sum the sizes of the List<ByteBuffer> that are read/written by the Transceiver on the client and by the Responder#respond() on a the server.

> Add bytesWritten() to Encoder Class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

This looks good to me.  Should we add a test?

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Attachment: AVRO-578.patch.v5

Nits addressed...

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3, AVRO-578.patch.v4, AVRO-578.patch.v5
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12880694#action_12880694 ] 

Philip Zeyliger commented on AVRO-578:
--------------------------------------

It's a question of whether we are considering the Plugin API evolving or stable, and I'm not sure.  The safest thing to do is to add more methods to the RPCContext class, representing the clearer API you propose, and mark as deprecated the old ones.  The code will continue calling both for a release, until we can delete the deprecated methods.  If we're less scrupulous, we could delete the old methods altogether, but that might cause someone pain.

Doug, do you have an opinion?

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Attachment: AVRO-578.patch.v3

I started to do more heavy re-factoring but decided it was excess.

This version just takes the two points where this was an issue (client send and server send) and breaks them into two methods, one before payload is generated and one after. Also, I included tests to verify that the content is being populated correctly.

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add bytesWritten() to Encoder Class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

> The way you mention only gives request size to the responder and response size to the requester.

I don't see that.  Both Responder#respond() and Transceiver#transceive() see both requests and responses.

> Add bytesWritten() to Encoder Class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

Looks generally good.  Some nits:
 - I think we can use List in method signatures and field declarations. LinkedList should be used as the implementation, since it supports faster prepends, but that detail can be hidden, I think.
 - we could add a method like ByteBufferOutputStream#prepend(List<ByteBuffer>... lists).  this could then have the option of converting things to perhaps a single ByteBuffer when small, to reduce the number of i/o operations, since each ByteBuffer may correspond to a write and/or read.  We may or may not make that optimization now, but it might be a nice option to have later.
 - RPCContext: new public methods in a public class should have javadoc.  some of the existing public methods there lack javadoc, which is a bug.
 - there's some commented-out code in Requestor that should probably be deleted.

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3, AVRO-578.patch.v4
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12880003#action_12880003 ] 

Philip Zeyliger commented on AVRO-578:
--------------------------------------

I threw this up on reviewboard (pretty nifty, btw!).  I don't think we've configured reviewboard to paste in its messages in the JIRA, so I'm manually adding them.  The thing I'm most worried about is Plugins keeping a hold of the RpcContext object for longer than they ought to, and inadvertently keeping the requests and responses.

-- Philip


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


For testing, you should check out src/test/java/org/apache/avro/RPCMetaTestPlugin.java.  You can assert that the payload fields are set, that the right number of bytes is being sent and received, etc.


trunk/lang/java/src/java/org/apache/avro/ipc/RPCContext.java
<http://review.hbase.org/r/210/#comment1106>

   The other things here are either private or protected; this probably deserves to be too.



trunk/lang/java/src/java/org/apache/avro/ipc/RPCContext.java
<http://review.hbase.org/r/210/#comment1107>

   if this is the only accessor you have, do you need setPayload at all, or should you provide access to the entire payload?  If you provide access to the payload, it might make sense to mark it in the javadoc as "read only", since plugins really shouldn't be editing the payload on the fly.



trunk/lang/java/src/java/org/apache/avro/ipc/RPCContext.java
<http://review.hbase.org/r/210/#comment1104>

   [nit] I think the style here is "for (A a : as) {" (i.d., space both before and after ":", but I could be wrong.



trunk/lang/java/src/java/org/apache/avro/ipc/Requestor.java
<http://review.hbase.org/r/210/#comment1105>

   It seems a bit odd to use the same field for both request and response payloads.

   I'm also a bit concerned that the payload data might be large, and that we're storing it in this RpcContext object for potentially a while.  Is the RpcContext object being stored for a while?

   One way to do this might be to add plugin.clientReceiveResponse(context, response) (i.e., give the response directly to the plugin).  That breaks the cleanliness of the API.  Another way to do it would be to explain in the javadoc for the plugin that the payload is available only during clientReceiveResponse.


- Philip

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add bytesWritten() to Encoder Class

Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12879190#action_12879190 ] 

Patrick Wendell commented on AVRO-578:
--------------------------------------

Ya except would be nice to monitor outgoing/incoming bandwidth from either node (requester/responder). The way you mention only gives request size to the responder and response size to the requester. 

> Add bytesWritten() to Encoder Class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Attachment: AVRO-578.patch.v2

I added a stub for a test, but until we actually generate metrics for the payloads, there's no direct way to test this. Going to submit a patch for that later today, which will fill in this test.

Also I change getMessage() to message() in the RPCContext class to make it match the other getter's already declared in that file.

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Attachment: AVRO-578.patch.v4

Handling the payloads distinctly is the best way to do it, as the interface doesn't have to change. Attached is another rev that does this.

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3, AVRO-578.patch.v4
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

Patrick>  One way would be to add add two calls in each place, one which interacts with plugin pre-encoding payload and the other after. That feels ugly to me...

What's a better alternative?  Should we have specific mungeServerRequestMetadata(Map<String,ByteBuffer>) and a recordStatsAboutServerResponsePayload(List<ByteBuffer>) methods instead?  That'd be cleaner, but might overly constrain what's possible.

Philip> It's a question of whether we are considering the Plugin API evolving or stable, and I'm not sure.

I don't think the plugin API is yet used much, so I think it could change without breaking much, if anything.

Some comments on the patch:
 - What's a use case for preServerSendResponse? If a plugin wants to add metadata to the response, it now needs to do it in the serverRecieveRequest() method, which limits what it can do.  I can imagine plugins that want to add stuff to the metadata that depends on the response.  That was possible before, but no longer is.  Perhaps metadata should always be put in a separate ByteBuffer at the head of the response, then we might could (1) serialize the response payload; (2) run the plugins; (3) serialize the metadata.  Then you could collect stats on payload size, but not metadata size.  This would allow a plugin that encrypted or checksummed payloads.
 - Similarly, in Requestor, we might serialize the payload, then call plugins, then serialize the metadata.
 - In Requestor, can we combine the two loops calling postClientSendRequest()?

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Commented: (AVRO-578) Add RPC Payload to RPCContext class

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

Doug Cutting commented on AVRO-578:
-----------------------------------

I just committed this, with a few cosmetic tweaks.  Thanks, Patrick!

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch, AVRO-578.patch.v2, AVRO-578.patch.v3, AVRO-578.patch.v4, AVRO-578.patch.v5
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add bytesWritten() to Encoder Class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Component/s: java

> Add bytesWritten() to Encoder Class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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


[jira] Updated: (AVRO-578) Add RPC Payload to RPCContext class

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

Patrick Wendell updated AVRO-578:
---------------------------------

    Summary: Add RPC Payload to RPCContext class  (was: Add bytesWritten() to Encoder Class)

> Add RPC Payload to RPCContext class
> -----------------------------------
>
>                 Key: AVRO-578
>                 URL: https://issues.apache.org/jira/browse/AVRO-578
>             Project: Avro
>          Issue Type: Sub-task
>          Components: java
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-578.patch
>
>
> For stats/monitoring it is helpful to see how many bytes are encoded for a given RPC call. Right now Encoder's don't track how many payload bytes are actually written out when encoding is done.
> Ideally this bytesWritten() would be in Encoder interface, however not sure JSON plugin can track the number of characters actually written, so alternatively just could be added to BinaryEncoder, and stats plugin will only provide payload sizes when that encoder is used.

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