Skip to content

Conversation

@Binnn-MX
Copy link

@Binnn-MX Binnn-MX commented Dec 5, 2025

Issue #, if available:
N/A (Feature enhancement for MinIO compatibility)

Description of changes:
Added MinIO compatibility support to AWS S3 C++ SDK by implementing error code mapping for MinIO-specific errors. This change allows the SDK to properly handle and retry MinIO server errors when used with MinIO deployments instead of AWS S3.

Changes include:
Added 10 MinIO-specific error hash constants (InternalError, XMinioServerNotInitialized, XMinioIAMNotInitialized, XMinioBucketMetadataNotInitialized, ServerBusy, TooManyRequests, SlowDownRead, SlowDownWrite, RequestTimeTooSkewed, RequestTimeout, ClientDisconnected)
Implemented error mapping logic that converts MinIO errors to appropriate AWS CoreErrors with correct retry behavior
Maintained backward compatibility - existing AWS S3 error handling is unchanged
Properly categorized MinIO errors as retryable/non-retryable based on their nature

Check all that applies:
[✓] Did a review by yourself.
[✓] Added proper tests to cover this PR. (If tests are not applicable, explain.)
Tests should cover MinIO error code mapping and retry behavior verification
[✓] Checked if this PR is a breaking (APIs have been changed) change.
This is not a breaking change - only adds new error mappings while preserving existing behavior
[✓] Checked if this PR will not introduce cross-platform inconsistent behavior.
Error string mapping is platform-agnostic
[✓] Checked if this PR would require a ReadMe/Wiki update.

[✓] Linux
[] Windows
[] Android
[] MacOS
[] IOS
[] Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@CLAassistant
Copy link

CLAassistant commented Dec 5, 2025

CLA assistant check
All committers have signed the CLA.

@CheSema
Copy link
Member

CheSema commented Dec 8, 2025

Any examples when Clickhouse gets those errors and it does wrong thing?

@Binnn-MX
Copy link
Author

Binnn-MX commented Dec 9, 2025

Any examples when Clickhouse gets those errors and it does wrong thing?

Reason for adapting MinIO error codes:
When using MinIO as the storage backend, adapting its error codes ensures the stability of ClickHouse during storage management operations, avoiding user-visible exceptions. The storage layer is responsible for guaranteeing the correctness of data reads and writes, thereby minimizing end users’ perception and impact of storage control operations.

Current scenarios:
● A full restart of the MinIO cluster (horizontal scaling in or out), during which multiple storage nodes are restarted simultaneously.
● A rolling restart of the MinIO cluster (e.g., during a MinIO upgrade), during which storage nodes are restarted one by one.

For example, when MinIO performs control/management operations at the storage layer, ClickHouse is unable to parse the error messages returned by MinIO. As a result, the query executed by ClickHouse throws the following error:

Code: 499. DB::Exception: Received from xxx. DB::Exception: Message: Unable to parse ExceptionName: ClientDisconnected Message: Client disconnected before response was ready, bucket xxx, key xxx, object size 4944216. (S3_ERROR)
Code: 499. DB::Exception: Received from xxx. DB::Exception: Unable to parse ExceptionName: XMinioServerNotInitialized Message: Server not initialized yet, please try again.
Code: 499. DB::Exception: Unable to parse ExceptionName: ClientDisconnected Message: Client disconnected before response was ready This error happened for S3 disk. (S3_ERROR)

Based on joint testing of ClickHouse with MinIO, as well as standalone MinIO testing, the following error codes need to be adapted:
image

@JiaQiTang98
Copy link

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

@Binnn-MX Could you please sign this Contributor License Agreement

@Binnn-MX
Copy link
Author

Binnn-MX commented Dec 9, 2025

@JiaQiTang98 Done.

@JiaQiTang98
Copy link

Could we merge this PR now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants