Commit 6110300
authored
feat: make retry-policy knobs configurable via connect() (both backends) (#433)
`retryMaxAttempts` / `retriesTimeout` / `retryDelayMin` / `retryDelayMax` lived
only in the internal `ClientConfig` (with defaults 5 / 15min / 1s / 60s) and were
consumed by the Thrift `HttpRetryPolicy` and forwarded to the kernel on the SEA
path (#420) — but `connect()` never ingested them from user options, and they
were not declared on `ConnectionOptions`. So a caller setting `retryMaxAttempts`
had no effect on either backend; the hardcoded defaults always applied.
Declare the four knobs on `ConnectionOptions` and copy them into `ClientConfig`
in `connect()` (per-key narrowed copy, matching the metricView / precision /
telemetry knobs). Because both backends already read `ClientConfig` — Thrift via
`HttpRetryPolicy.getConfig()`, SEA via `buildKernelRetryOptions(getConfig())` —
this one change makes the knobs honored on Thrift and SEA simultaneously, at
parity.
Semantics are total-attempts on both backends: `retryMaxAttempts=N` caps at N
total attempts (initial + retries); `0`/`1` both mean a single attempt. Verified
end-to-end on SEA against a persistent-429 fault: `retryMaxAttempts=2` → 2
attempts, `=0` → 1 attempt (previously both produced the default 5). Thrift's
`HttpRetryPolicy` increments-then-checks `attempt >= retryMaxAttempts`, giving
the same total-attempt count.
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>1 parent 93b0331 commit 6110300
3 files changed
Lines changed: 77 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
611 | 611 | | |
612 | 612 | | |
613 | 613 | | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
614 | 632 | | |
615 | 633 | | |
616 | 634 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
62 | 85 | | |
63 | 86 | | |
64 | 87 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
620 | 620 | | |
621 | 621 | | |
622 | 622 | | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
623 | 659 | | |
624 | 660 | | |
625 | 661 | | |
| |||
0 commit comments