http: stop using retry_back_off.max_interval as implicit per-try timeout

[davidvo-lyft]

Utility::convertCoreToRouteRetryPolicy unconditionally copied the computed backoff max_interval into the route retry policy's per_try_timeout. Every client that builds a retry policy from config.core.v3.RetryPolicy (ext_authz, oauth2, jwt_authn, gcp_authn, the gRPC access logger, and the Envoy gRPC async client) therefore got a per-try deadline equal to its maximum backoff interval, a knob that has nothing to do with timeouts. As reported in #38847, an operator who set a small max_interval (1 to 2 ms in the repro) saw every try aborted almost immediately, with no config surface to express intent. This change stops setting per_try_timeout during the conversion, so each try is bounded by the caller's overall request timeout, matching route retry policy semantics where per_try_timeout defaults to unset. The change is guarded by a runtime feature so it can be reverted without a rollback.

Commit Message: http: stop using retry_back_off.max_interval as implicit per-try timeout

Additional Description: The conversion no longer copies max_interval into per_try_timeout. Guarded by envoy.reloadable_features.core_retry_policy_no_implicit_per_try_timeout (default true); setting it to false restores the previous behavior exactly. Removing this implicit copy is also a prerequisite for any future explicit per-try-timeout field on config.core.v3.RetryPolicy (as the issue author suggested): a dedicated field cannot coexist with the accidental max_interval default without the two fighting. That API addition needs separate api review and is intentionally out of scope here.

Risk Level: Medium. Behavior change for all consumers of the core-to-route retry conversion, runtime guarded and default on; the four HTTP consumers that always set an overall request timeout fall back to that deadline.

Testing:

• //test/common/http:utility_test (HttpUtility.TestConvertCoreToRouteRetryPolicy): with the guard default-on, the converted policy has no per_try_timeout for both the default policy and an explicit retry_back_off (1s/5s) case, while num_retries, base_interval, max_interval, and retry_on still convert; a TestScopedRuntime block flips the guard false and asserts the legacy 5s and 10s copies return. Red before the fix (asserts has_per_try_timeout() false but it was true), green after. Verified in the CI clang docker image.
• //test/common/grpc:async_client_impl_test (EnvoyAsyncClientImplTest.ParsedRetryPolicyWillBeUsed): a retry_back_off of base 1s / max 2s on the gRPC service yields perTryTimeout() == 0ms on the parsed policy (was 2000ms). Red before, green after.

Docs Changes: N/A

Release Notes: changelogs/current/minor_behavior_changes/http__core-retry-policy-per-try-timeout.rst (names the runtime flag and the revert procedure)

Platform Specific Features: N/A

Fixes #38847

Generative AI disclosure: Developed with AI assistance (Claude Code); I reviewed and understand every line and take full ownership of the submission.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[repokitteh-read-only]

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #46238 was opened by davidvo-lyft.

see: more, trace.

[paul-r-gall]

LGTM, but should we add a "per try timeout" field to the core RetryPolicy?

[paul-r-gall]

@botengyao wdyt about adding a per try timeout to the core RetryPolicy?

[davidvo-lyft]

Thanks @paul-r-gall . I agree an explicit per_try_timeout on config.core.v3.RetryPolicy is the right long-term direction, mirroring config.route.v3.RetryPolicy.per_try_timeout.

This PR is deliberately the prerequisite for that: as long as the conversion silently copies retry_back_off.max_interval into per_try_timeout, a real per-try-timeout field would fight that accidental default, so the implicit copy has to go first. The field itself is an API addition that needs its own api-shepherd review, so I kept it out of scope to keep this one a focused, runtime-guarded bug fix.

Happy to open a follow-up issue and take on the field addition once this lands. Does that split sound good?