Skip to content
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

Streaming gRPC client has no backpressure #631

Closed
joroKr21 opened this issue May 28, 2024 · 8 comments · Fixed by #636
Closed

Streaming gRPC client has no backpressure #631

joroKr21 opened this issue May 28, 2024 · 8 comments · Fixed by #636

Comments

@joroKr21
Copy link
Contributor

joroKr21 commented May 28, 2024

It uses an unbounded queue:

But there is no external backpressure mechanism, we request the next message as soon as we receive one:

runtime.unsafe.run(queue.offer(ResponseFrame.Message(message)) *> call.request(1)).getOrThrowFiberFailure()

In addition, it would be beneficial to use a (configurable) batch size > 1 when requesting messages from the server.

If that's ok in terms of IP, I can port the solution from fs2-grpc to zio-grpc.

@thesamet
Copy link
Contributor

thesamet commented May 28, 2024 via email

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 7, 2024

A PR that adds backpressure (disabled by default) will be accepted.

Where can we configure it? I can't find a good place.

@thesamet
Copy link
Contributor

thesamet commented Jun 7, 2024

See

private[zio_grpc] val queueSizeProp = "zio_grpc.backpressure_queue_size"
val backpressureQueueSize: IO[StatusException, Int] =
ZIO
.config(zio.Config.int("backpressure_queue_size").nested("zio_grpc").withDefault(16))
.mapError { (e: zio.Config.Error) =>
Status.INTERNAL.withDescription(s"$queueSizeProp: ${e.getMessage}").asException()
}

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 7, 2024

I added it as a parameter to ZChannel: #636
If you tell me how to call the setting, I could also make it a property.

@thesamet
Copy link
Contributor

thesamet commented Jun 7, 2024

Can you rephrase your question, I am not sure exactly what are you looking for.

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 8, 2024

In my PR I just added a constructor parameter to ZChannel - should I convert it to a property and if so how should we call it?

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 8, 2024

Or do you mean that I should reuse backpressure_queue_size for the client?

@thesamet
Copy link
Contributor

Since it's a constructor parameter, no need to introduce a property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants