grpc / grpc/grpc-java

perAttemptRecvTimeout in RetryPolicy does nothing

Aperta
#12,919 3 commenti 19 reazioni 0 assegnatari Vedi su GitHub
bug
Lingua principale
Java
Stelle
12.1k
Fork
4k
Merge medio
2g 17h
PR unite (30g)
37

Descrizione

### What version of gRPC-Java are you using?
1.76.0 (but it's the same problem in the latest release too)

### What is your environment?
Android app with `gprc-java` and `grpc-android` libs

### What did you expect to see?
https://github.com/grpc/grpc-java/pull/8301 added `perAttemptRecvTimeoutNanos` to the `RetryPolicy` but the problem that it's never used so it's not possible to set a timeout for an attempt within the gRPC call. Not sure but might be related to this issue https://github.com/grpc/grpc-java/issues/1943

### What did you see instead?
I would expect `RetriableStream` to use `perAttemptRecvTimeoutNanos` from `RetryPolicy` so `makeRetryDecision` will actually retry when an attempt takes longer than specified in `perAttemptRecvTimeoutNanos`

### Steps to reproduce the bug
Can be reproduced with any channel and `RetryPolicy` that has `perAttemptRecvTimeout`.

### A bit of context why do I even need it
I'm looking for a way to detect and recover from a black hole gRPC connection that happens in my app.
I have logs from production with `DEADLINE_EXCEEDED` exception that either has `waiting_for_connection` or `remote_addr=/10.0.2.2:8443` (it's from local test but the point that there is a `remote_addr` in production). As far as I understand in the first case the channel is in `CONNECTING` state and in the second is in `READY` but neither of them can succeed and backend has no errors. I reproduced the issue locally by using [toxiproxy](https://github.com/Shopify/toxiproxy) and simulated a black hole connection. My idea is to pass `perAttemptRecvTimeout` in `RetryPolicy` so I can rely on gRPC internal retry mechanism and when an attempt fails with `DEADLINE_EXCEEDED` to call `channel.enterIdle` (same as `AndroidChannelBuilder` does when detects the change in network to force a new connection for the next call) in `ClientStreamFactory` (same idea as for refreshing an expired auth token but without `CallCredentials` as was suggested here and it actually works in our app https://github.com/grpc/grpc-java/issues/7345#issuecomment-679295003)
```
object : ClientStreamTracer.Factory() {
override fun newClientStreamTracer(
info: ClientStreamTracer.StreamInfo,
headers: Metadata,
): ClientStreamTracer = object : ClientStreamTracer() {
override fun streamClosed(status: Status) {
if (status.code == Status.Code.DEADLINE_EXCEEDED) {
channel.enterIdle()
}
}
}
}
```

I also considered keepAlive option but it doesn't seems to work as smooth as the idea above and if I'm not wrong - it will not recover from a black hole connection when the channel is in `CONNECTING` because it's only sent for an established connection so the channel has to be in `READY` state. And also it will drain battery and spam the backend.

Also I thought to have a retry interceptor but I'm afraid it's a fragile approach that previously caused us a lot of crashes when it was attempted for token refresh and retry afterwards. (Probably it's possible to implement it flawlessly but it likely it will be difficult to understand and easy to break in the future and therefore not considered).

And as a last resort it's also possible to use try/catch and retry on the call-site and manage the channel there but it requires to change every single call-site in the app and can be easily forgotten for new calls. So not a sustainable option.

Just wanted to share what I already thought of and I believe the first suggestion that uses `perAttemptRecvTimeout` is the best among listed approaches. I would be happy to hear if there are other possible robust ways to fix it.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia tracciando RetryPolicy e RetriableStream, concentrandoti su come perAttemptRecvTimeoutNanos raggiunge makeRetryDecision. Usa lo scenario di black hole segnalato o un channel e una RetryPolicy equivalenti per verificare che un tentativo che supera il timeout configurato produca DEADLINE_EXCEEDED e attivi il comportamento di retry previsto.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
android, java
Ambito
backend-api-design, networking
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Attiva
Chiarezza
Abbastanza chiara
Idoneità per principianti
55/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.