Add retry to GrpcScheduler

[chrisstaite-menlo]

Description

Add support for retrying on the GrpcScheduler following the same model as the GrpcStore.

Fixes #322

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Run the GrpcScheduler with Reclient. It is no longer failing with concurrency issues, but it does appear that there are other issues still (see #323)

Checklist

• Updated documentation if needed
• Tests added/amended
• bazel test //... passes locally
• PR is contained in a single commit, using git amend see some docs

---

This change is 

[allada]

+@aaronmondal , could you review this one too?

Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, all discussions resolved (waiting on @aaronmondal)

[aaronmondal]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @chrisstaite-menlo)

---

cas/scheduler/grpc_scheduler.rs line 52 at r1 (raw file):

impl GrpcScheduler {
    pub fn new(config: &config::schedulers::GrpcScheduler) -> Result<Self, Error> {
        let jitter_amt = config.retry.jitter;

WDYT about defining the jitter_fn in new and remove the indirection via a separate new_with_jitter? It would reduce configurability a bit, but would save a public API function.

---

cas/scheduler/grpc_scheduler.rs line 97 at r1 (raw file):

        self.retrier
            .retry(
                retry_config,

nit: We should really find a better way to handle the retry stuff in general. This is so hard to read. Tracking this in #335

[chrisstaite-menlo]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aaronmondal)

---

cas/scheduler/grpc_scheduler.rs line 52 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

WDYT about defining the jitter_fn in new and remove the indirection via a separate new_with_jitter? It would reduce configurability a bit, but would save a public API function.

The purpose of injecting the jitter function is to ensure that this is reliably testable. You don't really want randomness to be injected into tests by this function. This follows the design from all of the other uses of Retry.

---

cas/scheduler/grpc_scheduler.rs line 97 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

nit: We should really find a better way to handle the retry stuff in general. This is so hard to read. Tracking this in #335

Yeah, it's not great... but it does allow you to place the functional block in a central point. Will be nice to see what better solution you come up with.

[aaronmondal]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @chrisstaite-menlo)