Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add callback-based method in Channel #215

Closed
acogoluegnes opened this issue Nov 16, 2016 · 5 comments
Closed

Add callback-based method in Channel #215

acogoluegnes opened this issue Nov 16, 2016 · 5 comments

Comments

@acogoluegnes
Copy link
Contributor

acogoluegnes commented Nov 16, 2016

E.g. in Channel:

void asyncRpc(Method method, CommandCallback callback) throws IOException;

With:

public interface CommandCallback {
	void handle(Command response);
}

This could allow to build a reactive API and also, combined with NIO, to have a truly non-blocking client (right now RPC calls are serialized, at least at the channel level).

@michaelklishin
Copy link
Member

I suspect we then want an entire new ChannelN implementation.

@michaelklishin
Copy link
Member

…and perhaps an interface, e.g. AsyncChannel. I'd also consider using futures over callbacks (or at least both).

@acogoluegnes
Copy link
Contributor Author

We may not have to rewrite ChannelN from scratch, I hope we can change the outstanding RPC call property in favor of a queue and support the callback scenario.
The point here is to have a foundation for reactive and opinionated extensions to build on (they tend to prefer callback over futures usually).
The AsyncChannel scenario would be something else, more involving I guess.
I'll try to start experimenting in the next days/weeks.

@smaldini
Copy link

smaldini commented Nov 16, 2016

Yes please no Future or it would be CompletableFuture if the java version supports it. Even so, the problem is more that we need to have the basic abilities to :

  • trigger async work on demand (call method), ultimately we will have to bridge in a reactive API with a deferring operation to trigger the call on subscribe only.
  • not blocking on that async method at all, Future#get would only encourage that, kafka mixed up these two API styles.
  • getting the status in the callback : success(result)/error(cause) or getting 2 callbacks for success/error
  • resulting only when operation is complete for single result or n times for n results on stream of callbacks (channel consuming) with close/error callback to be provided
  • inspired from https://github.com/reactive-streams/reactive-streams-jvm even tho its not implementing it. For instance success/error should be unique and terminal, definition of "blocking operations" and effort to keep it efficient.

@michaelklishin
Copy link
Member

I have no problem with rolling with CompletableFuture and requiring JDK 8 now that we are no longer tied to RabbitMQ releases and thus can bump version numbers at will.

@acogoluegnes acogoluegnes modified the milestones: 4.2.0, 4.1.0 Jan 3, 2017
@acogoluegnes acogoluegnes modified the milestones: 5.0.0, 4.2.0 Feb 20, 2017
acogoluegnes added a commit that referenced this issue Mar 7, 2017
acogoluegnes added a commit that referenced this issue Mar 8, 2017
acogoluegnes added a commit that referenced this issue Mar 8, 2017
acogoluegnes added a commit that referenced this issue May 16, 2017
acogoluegnes added a commit that referenced this issue Aug 1, 2017
RPC response comes from the reading threads and this can cause
deadlocks if the client doesn't use CompletableFuture#*async
methods.

References #215
acogoluegnes added a commit that referenced this issue Sep 14, 2017
Even if CompletableFuture is completed in the reading thread,
client API (e.g. CompletableStage#then*, Reactor Flux/Mono)
provide ways to control the threading behavior.

References #215
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants