Google.Cloud.Firestore: Backoff on FirestoreDb.RunTransactionAsync retry

[kasperhhk]

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is your feature request related to a problem? Please describe.
Regarding RunTransactionAsync on the FirestoreDb class in the Google.Cloud.Firestore package.

When we call it with MaxAttempts > 1 option, then we would like to have some sort of backoff policy, in our case for data contention, so that it does not spam the document when it is already in contention.
(note there is a backoff variable that is unused in the source code, not really relevant to the solution proposed below though)

Describe the solution you'd like
Either a backoff policy similar to what we see in the _batchGetCallSettings field or being able to pass an optional backoff policy to the method / client.
There's the issue of backwards compatibility, so maybe it does need to configurable by the user, defaulting to current behavior (no backoff).

Describe alternatives you've considered
Currently we're implementing our own backoff policy, wrapping around the firestoredb, but since you already have a backoff concept, it seemed reasonable to add it here as well.

Additional context
I admit that I'm a bit unsure if there's some internal backoff policy inside the lower internal functions that I haven't been able to divine, but from what I could see there's nothing obvious going on, so please correct me if there's anything I've misunderstood.

[jskeet]

This sounds entirely reasonable, but we'll need to talk with the Firestore SDK team - we'll want to make sure that we're consistent with other languages.

[jskeet]

I've talked with the Firestore team, and they're going to write an internal document to design how this can be achieved consistently across languages. Thanks for bringing it up.

[jskeet]

Just to avoid leaving you completely in the dark - this is still bubbling along. I'm hoping we'll get somewhere "soonish" but I can't make any promises about a timeline. We haven't forgotten about it though.

[jskeet]

@kasperhhk: Thinking about this in terms of what I'm doing for #11322, would a FirestoreDb-wide setting for retry configuration work for you? I can imagine some times when you might want per-call settings, but if we were to expose a FirestoreDbBuilder.RunTransactionRetrySettings property, would that at least get most of the way there?

[jskeet]

Actually, I now wonder whether we should just have a RetrySettings in TransactionOptions, given that that's got MaxAttempts already. (We'd probably need some sort of precedence - if the RetrySettings is specified, that overrides MaxAttempts, given that a RetrySettings effectively includes max attempts already.)

[jskeet]

(Or if we don't want to allow the set of RPC failures to be customized, we could potentially only expose the timing part. RetrySettings has quite a few parts though, so maybe we should just expose a RetrySettings property, and say that MaxAttempts and RetryFilter are ignored? Hmm.)

[jskeet]

@kasperhhk: If you could have a look at #11564 and add any thoughts about the API, that would be much appreciated. If you're happy with it, I'll write appropriate tests and check whether the Firestore team is okay with it too.