Pubsub: Provide callback capabilities to publishAsync

[viniciusccarvalho]

It would be nice if publishAsync could offer something like Guava's ListenableFuture, so we could add non blocking callbacks to the results of the operation.

[mziccard]

Hi @viniciusccarvalho, thanks for your interest!

We decided not to expose Guava's ListenableFuture in our API. Mainly due to the fact that as soon as we switch to Java8 we would have to get rid of them in favor of CompletableFuture.

However, the futures we return are actually a ListenableFuture. So, if you are using Guava and are really in need for non blocking callbacks on publishAsync you can use JdkFutureAdapters.listenInPoolThread without paying the price for any additional thread. Please keep in mind that this might be subject to change, in particular when we switch to Java8, but as soon as we do that you will have a native method to register callbacks.

Out of curiosity, what is your use case for registering callbacks on publishAsync?

[viniciusccarvalho]

Got it, I was hoping for CompletableFuture but I thought it was asking too much :)

Mainly to be able to return immediately and attach a listener in case of errors for example. One thing we are struggling is with the overall throughput of pubsub. We thought on using the spotify client, but since gRPC is becoming active we gave it a try.

Initial tests are showing gRPC a bit slower than the traditional REST client, which is a bit weird, we may not be setting the right knobs. But as a general rule, sending single messages it usually takes ~50ms per 128byte message with REST but its taking ~70ms with gRPC. Those numbers drop by half usually when running from GCE, but it's still a lot compared to other systems such as kafka or even rabbitMQ.

Batching makes wonders, we are able to easily reach peaks of 18k messages/sec per thread when batching them, but it adds some overhead on our end to process the batch and we loose any acks received if clients want to get a guarantee. It's a tradeoff we understand, but we are trying to find better ways to scale out the throughput of a single client.

Regards

[mziccard]

Initial tests are showing gRPC a bit slower than the traditional REST client, which is a bit weird, we may not be setting the right knobs.

This is strange. Which REST client are you using? I assume that to test gRPC you are using this library, is this the case?

[viniciusccarvalho]

I thought so as well. I'm using google's own REST API.

<dependency>
            <groupId>com.google.apis</groupId>
            <artifactId>google-api-services-pubsub</artifactId>
            <version>v1-rev11-1.22.0</version>
        </dependency>

To be more precise. I can come up with two sample projects to demo this. I pretty much created two simple apps that have an endpoint where one express : how many messages, payload size and batch (if any) size. One using REST and other gRPC. And I then just yield the time it took to get the ack back. To my surprise the gRPC version of this library (0.3.0) is having a much higher response time.

[viniciusccarvalho]

Hi Marco, I created this project : https://github.com/viniciusccarvalho/spring-pubsub
that has two implementations (REST & gRCP) and you could simply test it. Let me know if you see something really horrific .

Regards

[garrettjonesgoogle]

The latency problem was also observed in #1338 .

[garrettjonesgoogle]

After some internal discussion, I propose that we go ahead with switching from Future to ListenableFuture throughout google-cloud-java. Motivation:

1. gRPC has already set a precedent for exposing ListenableFuture.
2. This doesn't mean we will open the floodgates for Guava types; we will set a policy of considering them on a case-by-case basis.
3. We aren't moving to Java 8 in the short term due to concerns around Android and various companies potentially still needing to vet it. Even when we do move to Java 8, we don't necessarily have to switch to CompletableFuture - we have the option of continuing to use ListenableFuture for a while after that.
4. The performance impacts of not exposing ListenableFuture are big enough that certain use cases would completely avoid using the library because of it (Pub/Sub).

Tagging some people for visibility and discussion if there is disagreement: @anthmgoogle @aozarov @kluever @cpovirk @lesv @davidtorres

[cpovirk]

That sounds good to me on the Guava side.

A couple further notes:

• If you are interested in CompletableFuture for its more convenient chaining API, you may be interested in our proposed FluentFuture, which would extend ListenableFuture and provide instance methods like transform. However, this won't be in Guava for another release or two (best case, by the end of the year; worst case, months more). Additionally, we're not 100% sure that we'll be adding it at all. If adding it would be important to you, please let us know. (Even if you return ListenableFuture today, you could change your APIs to return FluentFuture later, though doing so in a compatible way is a little subtle. Even if you don't, we'll probably be changing our common ListenableFuture implementations to extend FluentFuture, so users can cast or cheaply convert the returned values to FluentFuture instances.)

• The maintainers of java.util.concurrent have spoken of CompletableFuture as "a common underlying implementation... for others creating or reworking higher-level APIs" rather than as a type that most end users will interact directly with. Now, I get the impression that they're OK with people who use CompletableFuture directly, and of course people do. But I at least think they would say "You don't need to migrate to CompletableFuture if you're happy with something else." Hopefully we can make you happy with ListenableFuture :)

[davidtorres]

Just for the record, this sound good to me from the Pub/Sub side.

[garrettjonesgoogle]

So things have changed since my last comment on this thread. It turns out that Guava is a pretty problematic dependency for certain consumers. So for now, we are going to introduce our own type that wraps ListenableFuture, called RpcFuture. @pongad is implementing this in the high-performance rewrite of Pub/Sub.