Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][pip] PIP-408 add maxRetryRequestTimes in ClientConfigurationData and remove duplicate retry logic #23997

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions pip/pip-408.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# PIP-408: add maxRetryRequestTimes in ClientConfigurationData and remove duplicate retry logic

# Background knowledge

AsyncHttpConnector is utilized in the admin-client where it serves a critical function. The apply method in AsyncHttpConnector intercepts requests and transmits them using Jersey.

The retry count is governed by the maxRetries property, which is inherited from the maxRequestRetry attribute of the httpClient configuration passed to the constructor. This configuration property defaults to 5 in DefaultAsyncHttpClientConfig.

# Motivation

In AsyncHttpConnector, the retry logic is implemented through the retryOrTimeOut method, while the DefaultAsyncHttpClient used for sending requests also has its own retry mechanism. This dual-layer implementation may result in the actual number of retries exceeding the configured maxRetries value.

# Goals

The actual request retry count matches the configured value of the maxRetries property.

## In Scope

Configure maxRetries directly through ClientConfigurationData in AsyncHttpConnector rather than inheriting from the httpclient's config.
Set maxRequestRetry in the httpclient's configuration to 0 to disable its built-in retry mechanism. This approach ensures retries are exclusively handled at the connector level while maintaining the configured retry behavior.

# High Level Design

Add a maxRetryRequestTimes property in ClientConfigurationData to configure the maximum retry attempts.
In AsyncHttpConnector, use this ClientConfigurationData property to set retry counts instead of relying on the maxRequestRetry attribute from the httpClient's config. Explicitly set the maxRequestRetry property in the httpClient variable's configuration to 0 to disable retry logic at the HTTP client layer.
This isolates retry control to the connector-level configuration while preventing redundant retry behavior from the underlying client implementation.

## Design & Implementation Details

1. **Retry Configuration Support**
Added `maxRetryRequestTimes` in `ClientConfigurationData` (default: 5) to control admin client request retry behavior. Configuration propagates to `AsyncHttpConnector` implementations. This change preserves the default request retry count in configurations. The newly added retry count matches DefaultAsyncHttpClient's original default value.

2. **Client API Enhancements**
- Extended `ClientBuilder` interface with new `maxRetryRequestTimes()` method
- Implemented configuration injection in `ClientBuilderImpl`

3. **HTTP Connector Refactoring**
- Modified `AsyncHttpConnector` to use centralized configuration instead of AsyncHttpClient's internal retry setting
- Explicitly disabled underlying library retry via `confBuilder.setMaxRequestRetry(0)`

4. **Validation**
Added parameter verification test in `ClientBuilderImplTest` to validate retry time configuration constraint
## Public-facing Changes

### Configuration

Add a maxRetryRequestTimes property in ClientConfigurationData to configure the maximum number of request retries.

#### ClientConfigurationData.java

```java
/**
* Set the max retry times for a request.
*
* @param maxRetryTimes
* @return the client builder instance
*/
ClientBuilder maxRetryTimes(int maxRetryTimes);
```

# Backward & Forward Compatibility

Fully compatible.

# Links

* Mailing List discussion thread:https://lists.apache.org/thread/643rfwgn292kyqkdxrmgckqbr1flsz59
* Mailing List voting thread: