Skip to content

Add CompletableFuture async RPC to channel #257

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 21, 2017

Conversation

acogoluegnes
Copy link
Contributor

@acogoluegnes acogoluegnes commented Mar 7, 2017

Add CompletableFuture<Command> asyncCompletableRpc(Method method) to Channel. This is foundation to react to asynchronous AMQP methods (most of the resource management methods). Right now, the RPC calls are serialized at the channel level, but this could be improved in the future by using a queue for the outstanding RPC calls.

Fixes #215

@acogoluegnes acogoluegnes changed the title DO NOT MERGE Add CompletableFuture async RPC to channel Add CompletableFuture async RPC to channel May 16, 2017
@michaelklishin
Copy link
Contributor

@acogoluegnes is this ready for QA? It was submitted in March but there is no "DO NOT MERGE" in the title.

@acogoluegnes
Copy link
Contributor Author

@michaelklishin Yes, it's ready for QA.

Conflicts:
	src/test/java/com/rabbitmq/client/test/ClientTests.java
Copy link
Contributor

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This turned out to be more straightforward than I expected but there's a couple of small things that need addressing.

}
}, executor).thenAcceptAsync(action -> {
try {
channel.basicPublish("", queue, null, "dummy".getBytes());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should wait for a confirm here, otherwise there can be a race condition between a message send by the channel to the queue master process and handling of the basic.consume below.

@@ -135,6 +137,20 @@ public AMQCommand exnWrappingRpc(Method m)
}
}

public CompletableFuture<Command> exnWrappingAsyncRpc(Method m)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a public method, I'd like to clarify the name. How about exceptionWrappingAsyncRpc?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now see we already have a "non-async" method with a similar name from way back. Disregard the above.

Copy link
Contributor

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added publisher confirms to the test.

@michaelklishin michaelklishin merged commit 3ba0254 into master May 21, 2017
@ofavre
Copy link

ofavre commented Sep 7, 2018

This asyncRpc facility does not permit sending Commands, that is Methods with body.
How can I perform an asyncBasicPublic() then?

@acogoluegnes
Copy link
Contributor Author

@ofavre If you mean publishing a message asynchronously, basic.publish is already asynchronous.

Please move the discussion to rabbitmq-users if you have further questions.

@michaelklishin
Copy link
Contributor

None of this code is meant to be used directly. Use the Channel API. Publishing is so asynchronous there is no response for it in the protocol (so Publisher Confirms had to be introduced).

@rabbitmq rabbitmq locked and limited conversation to collaborators Sep 7, 2018
@acogoluegnes acogoluegnes deleted the rabbitmq-java-client-215 branch March 21, 2019 08:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants