Register `s3.client.<client>.disable_chunked_encoding` in repository-s3 node settings
Description
This PR registers the s3.client.<client>.disable_chunked_encoding setting in the repository-s3 plugin.
Currently, configuring the setting in opensearch.yml such as:
s3.client.default.disable_chunked_encoding: true
results in a startup failure : unknown setting [s3.client.default.disable_chunked_encoding]
even though DISABLE_CHUNKED_ENCODING is defined in S3ClientSettings and supported at repository level.
Root Cause :
DISABLE_CHUNKED_ENCODING was not included in the list returned by S3RepositoryPlugin#getSettings(), so the setting was never registered as a valid node setting.
Fix:
Added S3ClientSettings.DISABLE_CHUNKED_ENCODING to S3RepositoryPlugin#getSettings().
Added a test assertion in S3RepositoryPluginTests#testGetExecutorBuilders to ensure the setting is registered.
Testing
./gradlew :plugins:repository-s3:check passes successfully.
Impact :
Users can now safely configure s3.client.default.disable_chunked_encoding in opensearch.yml for S3-compatible storage that does not support AWS chunked encoding (e.g., Oracle Object Storage, MinIO, Ceph, Cloudflare R2, etc.).
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **New Features**
* Exposes additional S3 client settings: request timeout, connection timeout, connection TTL, max connections, max sync connections, connection acquisition timeout, max pending connection acquires, and a chunked-encoding toggle.
* **Tests**
* Test suite updated to assert the expanded set of S3 configuration settings.
* **Other**
* Internal equality/hash behavior adjusted; public APIs unchanged.
<sub>โ๏ธ Tip: You can customize this high-level summary in your review settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Walkthrough
Exposes eight additional S3 client configuration settings via S3RepositoryPlugin, updates S3ClientSettings equality/hash/refine logic to include maxSyncConnections and primitive comparison for disableChunkedEncoding, and extends tests to assert the newly exposed settings.
Changes
| Cohort / File(s) | Summary |
|---|---|
S3 Repository Plugin plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3RepositoryPlugin.java |
Both occurrences of getSettings() now include additional S3 client configuration settings added after S3ClientSettings.SIGNER_OVERRIDE: REQUEST_TIMEOUT_SETTING, CONNECTION_TIMEOUT_SETTING, CONNECTION_TTL_SETTING, MAX_CONNECTIONS_SETTING, MAX_SYNC_CONNECTIONS_SETTING, CONNECTION_ACQUISITION_TIMEOUT, MAX_PENDING_CONNECTION_ACQUIRES, DISABLE_CHUNKED_ENCODING. Comment updated from "named s3 client configuration settings" to "s3 client configuration settings". |
S3 Client Settings plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3ClientSettings.java |
equals() updated to compare maxSyncConnections and use primitive equality for disableChunkedEncoding; hashCode() now includes maxSyncConnections; refine(Settings) adds a check comparing newMaxSyncConnections == maxSyncConnections. |
Tests plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3RepositoryPluginTests.java |
testGetExecutorBuilders expanded to assert presence of DISABLE_CHUNKED_ENCODING and the new S3 client settings (REQUEST_TIMEOUT_SETTING, CONNECTION_TIMEOUT_SETTING, CONNECTION_TTL_SETTING, MAX_CONNECTIONS_SETTING, MAX_SYNC_CONNECTIONS_SETTING, CONNECTION_ACQUISITION_TIMEOUT, MAX_PENDING_CONNECTION_ACQUIRES). |
Estimated code review effort
๐ฏ 2 (Simple) | โฑ๏ธ ~12 minutes
- Check both
getSettings()occurrences for consistent ordering and no duplicate entries. - Verify
S3ClientSettings.equals()andhashCode()includemaxSyncConnectionsconsistently and thatdisableChunkedEncodinguses the intended primitive comparison. - Confirm tests assert the exact setting objects/names as exposed.
Poem
๐ฐ I hopped through settings, one by one,
New timeouts, limits โ now there's more fun.
Max sync counts counted true and neat,
Hashes and equals now dance in beat.
๐ฅ Small hops, big clarity โ code complete!
Pre-merge checks and finishing touches
โ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | โ ๏ธ Warning | Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
โ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | โ Passed | The title accurately describes the main change: registering the disable_chunked_encoding setting in the S3 repository plugin, which aligns with the PR's primary objective. |
| Description check | โ Passed | The description includes the required sections: clear explanation of the problem, root cause analysis, the fix applied, testing confirmation, and user impact. All key information is present and well-structured. |
โจ Finishing touches
- [ ] ๐ Generate docstrings
๐งช Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
๐ Recent review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
๐ฅ Commits
Reviewing files that changed from the base of the PR and between f9ec501cb5274d96f4c7e03f8a56d55cde32d6a4 and 1da98e9450970f9b8ac7cd9929cc8b3374792008.
๐ Files selected for processing (3)
plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3ClientSettings.java(3 hunks)plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3RepositoryPlugin.java(2 hunks)plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3RepositoryPluginTests.java(1 hunks)
๐ง Files skipped from review as they are similar to previous changes (3)
- plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3RepositoryPluginTests.java
- plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3ClientSettings.java
- plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3RepositoryPlugin.java
โฐ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gradle-check
Comment @coderabbitai help to get the list of available commands and usage tips.
:x: Gradle check result for bfa670a02edb1091443eaf9ecbd7a7d8c5427a35: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 217b8b76dc352f4dbf584b5c9c0fec3608c5b38b: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 8d42f07d7146e46c3cfa90849a1a34619f02f7db: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
CI didn't start due to a Jenkins trigger 403 / jq parsing error. Requesting to retrigger gradle-check manually. Thank you!
:x: Gradle check result for 068a2bd4db521ac4b9f0fb1119d7f65c0bf6656f: null
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
I ran the suite locally via ./gradlew :plugins:repository-s3:internalClusterTest and it passes successfully.
The CI failure appears to be due to the Jenkins trigger issue (403 / jq parsing).
Requesting maintainers to manually retrigger gradle-check. Thank you!
Thanks for the suggestion! ๐
I will extend this PR to expose all missing S3 client settings (not only disable_chunked_encoding) as recommended.
Iโm working on adding:
- REQUEST_TIMEOUT_SETTING
- CONNECTION_TIMEOUT_SETTING
- CONNECTION_TTL_SETTING
- MAX_CONNECTIONS_SETTING
- MAX_SYNC_CONNECTIONS_SETTING
- CONNECTION_ACQUISITION_TIMEOUT
- MAX_PENDING_CONNECTION_ACQUIRES
- DISABLE_CHUNKED_ENCODING
Will push an update shortly.
:x: Gradle check result for aeceb9a63826073e965f3bbedc3f7fe1a38c75ef: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 12ac7d1b6883c02b262c5a58908c1e9f7570f560: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 07f99f13f094d1f5a3b5f81315cf5b58823cf4f1: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 1d6707907cc7558dac8158d49f7127213600d780: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 673752607e38e0c55e761bfeaada5c286516bda9: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for 92d39feadb3352e2218f49687284263e675cf172: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
All S3 plugin tests are passing locally and on CI before cleanup. The final failure occurs during Jenkins cleanup (docker prune / workspace wipe), which indicates a flaky infra failure unrelated to the code change.
Requesting a CI re-run. ๐
:x: Gradle check result for 23d184addefd2b15627585199f86349074a822bd: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
:x: Gradle check result for f9ec501cb5274d96f4c7e03f8a56d55cde32d6a4: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?
I have verified that ./gradlew spotlessApply produces no changes locally and formatting passes.
The Spotless failure on Jenkins is due to configuration caching (Cannot fingerprint input property 'stepsInternalEquality'), which is unrelated to changes in this PR.
Requesting CI retrigger / maintainer assistance.
:x: Gradle check result for 1da98e9450970f9b8ac7cd9929cc8b3374792008: FAILURE
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?