[
https://issues.apache.org/jira/browse/AVRO-539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13042828#comment-13042828
]
James Baldassari commented on AVRO-539:
---------------------------------------
Thanks for the comments, Doug! I'll try to address them all:
bq. Rather than calling the new APIs 'asynchronous' might we instead consider
them 'Callback-based' and/or 'Future-based'? They'd be friendly to async
implementations, but a synchronous implementation would be permitted. In
particular, I think we can remove 'asynchronous' from the names of these
methods.
I see your point. The new API could theoretically be used in synchronous calls
when the Transceiver doesn't support async I/O, so it isn't inherently
asynchronous. I'll try to either eliminate the "Asynchronous" from the method
names and simply overload them or, when necessary, change these new method
names so that they end in "Callback" or "Future" as appropriate.
bq. Do we really need both Callback-based and Future-based APIs?
I personally find Callbacks to be more practical than Futures. However, there
may be use cases where Futures are more appropriate, so I just thought it would
be better to give users the option to use either one. Since the CallFuture
class was already being used in NettyTransceiver before I started making
changes, I basically got the Future API for free. All I did was expose it in
the public API. Users can always ignore the CallFuture that is returned by the
async methods. However, I wouldn't be opposed to removing the Future API if
you think it's just cluttering the interface.
bq. The name of the generated interface and methods should avoid potential
collisions with user-defined interfaces and messages, perhaps by using '$'.
Alternately, we might just generate a single interface and not use different
method names, rather distinguishing by method signature. No user method should
accept a org.apache.avro.ipc.Callback parameter, so we would not need to worry
about method signature collisions.
That's a good point. So one proposal is to generate methods like
methodName$Callback(...) to eliminate naming conflicts. Regarding your second
proposal, I think it would be great to overload the original methods with the
additional Callback parameter rather than creating special method names. I'll
see what I can do with that.
bq. Should we make generation of Callback/Future-based interfaces optional?
I think that's a good idea. In my first attempt at this patch I made the
interfaces optional by adding the 'async' keyword/property to the IDL and
schema. Philip Zeyliger pointed out that this probably isn't the cleanest
approach, and I agree. Can you think of a better way to make the new
interfaces optional?
bq. in Transceiver, can we implement the Callback/Future-based API
synchronously in terms of the existing API, rather than throwing an exception?
I think you're referring to the fact that the non-Netty Transceivers throw
UnsupportedOperationException. Yes, I think it should be possible to make
these methods work in a synchronous way for non-Netty Transceivers. So the
default implementation in Transceiver would be synchronous, and
NettyTransceiver would override it to make it asynchronous. I'll work on that.
bq. in Requestor, can we implement the synchronous version in terms of the
Callback-based API so that less logic is replicated?
I did quite a bit of refactoring in Requestor to prevent duplicating code
between the sync and async interfaces. However, once I make the change from
your previous comment to make non-Netty Transceivers work with the async APIs,
I think the synchronous methods can be implemented using only the asynchronous
API. So that will further reduce duplication. Note that this is one case
where it would be more useful to use the Future API rather than the Callback
API.
Thanks again for the feedback. I think your suggestions are going to really
improve this patch. I'll work on making these changes and post an update to
the patch.
> Allow asynchronous clients to specify a callback to be run when server
> processing completes
> -------------------------------------------------------------------------------------------
>
> Key: AVRO-539
> URL: https://issues.apache.org/jira/browse/AVRO-539
> Project: Avro
> Issue Type: New Feature
> Reporter: Jeff Hammerbacher
> Attachments: AVRO-539.patch
>
>
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
