kvcache-ai/Mooncake

Fork 0

mirror of https://github.com/kvcache-ai/Mooncake.git synced 2026-11-05 01:17:55 +00:00

[GH-ISSUE #4461] [Store] Proposal: S3-compatible ObjectStorageAdapter for the distributed storage backend #1130

New issue

Open

opened 2026-10-29 21:23:11 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-29 21:23:11 +00:00

Owner

Originally created by @pingqiu on GitHub (Oct 3, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4461

Proposal: an S3-compatible `ObjectStorageAdapter` for the distributed storage backend

docs/source/design/store/oss-backend.md notes that key-based storage services such as OSS and S3 plug in through service-specific adapters, and that S3 needs a compatible adapter rather than a different endpoint. We would like to contribute that adapter, so Mooncake Store can offload to any S3-compatible store: AWS S3, SeaweedFS, MinIO, Ceph RGW and others.

Proposed shape

A new "s3" value for fs_adapter_type, next to "oss" in storage_backend.cpp, behind the same libcurl + OpenSSL build condition.
Reading oss_adapter.cpp, almost everything is protocol-neutral: URI encoding, canonical query, LIST XML parsing, the curl multi batch engine, iovec uploads and range reads. The OSS-specific parts are BuildUrl, BuildAuthorization and the x-oss-* header names in PrepareRequest. We would factor the shared part into a common base and add an S3 subclass with AWS Signature V4 (AWS4-HMAC-SHA256, x-amz-* headers), path-style addressing and a custom endpoint. The OSS adapter's behaviour would not change.
Configuration with the same shape as the OSS adapter: endpoint, bucket, region, access key, secret, session token, path style, max connections and buffer sizes, read from MOONCAKE_S3_* environment variables (or whatever prefix you prefer).

Tests

S3 versions of oss_adapter_test.cpp and oss_adapter_config_test.cpp, and a live test helper like oss_live_test_helpers.h.
Live runs against SeaweedFS and MinIO, plus a store-level run that offloads through kDistributed + s3 and reads everything back.

Questions before we start

Is a shared base with the OSS adapter acceptable, or would you prefer a standalone S3 adapter, or one built on the AWS SDK already linked for HA snapshots (common/s3_helper.h)?
Any preference for the environment variable prefix or the build flag name?

Happy to adjust the plan to whatever fits the project best.

Originally created by @pingqiu on GitHub (Oct 3, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4461 ### Proposal: an S3-compatible `ObjectStorageAdapter` for the distributed storage backend `docs/source/design/store/oss-backend.md` notes that key-based storage services such as OSS and S3 plug in through service-specific adapters, and that S3 needs a compatible adapter rather than a different endpoint. We would like to contribute that adapter, so Mooncake Store can offload to any S3-compatible store: AWS S3, SeaweedFS, MinIO, Ceph RGW and others. **Proposed shape** - A new `"s3"` value for `fs_adapter_type`, next to `"oss"` in `storage_backend.cpp`, behind the same libcurl + OpenSSL build condition. - Reading `oss_adapter.cpp`, almost everything is protocol-neutral: URI encoding, canonical query, LIST XML parsing, the curl multi batch engine, iovec uploads and range reads. The OSS-specific parts are `BuildUrl`, `BuildAuthorization` and the `x-oss-*` header names in `PrepareRequest`. We would factor the shared part into a common base and add an S3 subclass with AWS Signature V4 (`AWS4-HMAC-SHA256`, `x-amz-*` headers), path-style addressing and a custom endpoint. The OSS adapter's behaviour would not change. - Configuration with the same shape as the OSS adapter: endpoint, bucket, region, access key, secret, session token, path style, max connections and buffer sizes, read from `MOONCAKE_S3_*` environment variables (or whatever prefix you prefer). **Tests** - S3 versions of `oss_adapter_test.cpp` and `oss_adapter_config_test.cpp`, and a live test helper like `oss_live_test_helpers.h`. - Live runs against SeaweedFS and MinIO, plus a store-level run that offloads through `kDistributed` + `s3` and reads everything back. **Questions before we start** 1. Is a shared base with the OSS adapter acceptable, or would you prefer a standalone S3 adapter, or one built on the AWS SDK already linked for HA snapshots (`common/s3_helper.h`)? 2. Any preference for the environment variable prefix or the build flag name? Happy to adjust the plan to whatever fits the project best.

bsakaguchi commented

2026-10-29 21:23:12 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Oct 3, 2026):

Thanks for opening this issue, @pingqiu!

Field	Value
Issue	#4461
GitHub user ID	`6647175`
Reporter	@pingqiu

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Oct 3, 2026): Thanks for opening this issue, @pingqiu! | Field | Value | |-------|-------| | **Issue** | #4461 | | **GitHub user ID** | `6647175` | | **Reporter** | @pingqiu | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:23:12 +00:00

Author

Owner

@pingqiu commented on GitHub (Oct 7, 2026):

A follow-up with a working implementation, so the design question has something concrete behind it.

Branch: https://github.com/pingqiu/Mooncake/compare/main...store-s3-standalone (based on current main)

Shape: a standalone S3 adapter. The OSS adapter is not modified.

New backend. "s3" is a new fs_adapter_type, registered next to "oss" in storage_backend.cpp under the same libcurl + OpenSSL build condition.
Separate engine. S3ObjectStorageAdapter sits on its own protocol-neutral request engine, RestObjectStorageAdapter. It does not touch oss_adapter.cpp/.h, so existing OSS users see no change. The cost is that the OSS adapter and the new engine duplicate some code for now.
Signing and protocol details. Requests use AWS Signature V4, with path-style or virtual-hosted addressing. Empty query values are sent as name=, and the ListObjectsV2 continuation token goes back unencoded, as S3 returns it.
Configuration. Settings come from MOONCAKE_S3_*, with AWS_* fallbacks.
- Credentials are read as one set, so a stray AWS_SESSION_TOKEN is never sent with MOONCAKE_S3_* keys.
- The region falls back to AWS_REGION, then AWS_DEFAULT_REGION, then us-east-1 with a warning.
- IP and localhost endpoints use path-style addressing.
Retries. Transient failures are retried, as the AWS SDKs do: up to 3 attempts with backoff. That covers connection resets, 429 and 5xx, and timeouts while the connection is still being set up (DNS, TCP, TLS). A request that was sent and then timed out is not retried, so a stalled server costs one request timeout, and client errors are not retried. Without the retries, the batch live test failed intermittently against AWS: a dropped SYN or a stalled TLS handshake occasionally pushed a connect past the connect timeout.

Tests

Signing. SigV4 known-answer tests check against values computed independently with botocore.
Configuration. Tests cover credential sets, region fallback, scheme and addressing.
Scripted servers. Tests cover the continuation token, retry on 503 and dropped connections, no retry on 403, and giving up after 3 attempts.
Request engine. Because the S3 engine is separate, 24 of the engine tests in oss_adapter_test.cpp are ported to it in s3_adapter_engine_test.cpp, plus a connect-timeout test:
- batch refill and connection limits;
- reset, truncated and timeout faults;
- range checks;
- ordering and partial failures;
- iovec rewind;
- the health check.
Where retry changes an outcome, the test says so. The slow timeout tests are gated behind MOONCAKE_RUN_S3_FAULT_TIMEOUT_TESTS=1.
Live object tests. Lifecycle, 1105-object batches with paginated listing, and a wrong-secret negative control pass against SeaweedFS, MinIO and AWS S3 (us-east-1, repeated runs).
Store-level round trip. This goes through CreateStorageBackend with MOONCAKE_DISTRIBUTED_FS_TYPE=s3:
- the object health check;
- a multi-slice BatchOffload of 33 objects, including an opaque key containing NUL, /, % and \;
- a check that exactly those objects are in the bucket;
- BatchLoad compared byte for byte;
- IsExist.
It passes against SeaweedFS, MinIO and AWS S3.
Existing tests. The OSS and file storage tests pass unchanged.

Questions

Is a standalone adapter acceptable for a first PR? I also have a variant where OssObjectStorageAdapter and S3ObjectStorageAdapter share RestObjectStorageAdapter (no duplication, no public API change), and I can open that separately if you prefer it. An implementation on the AWS SDK already linked for HA snapshots (common/s3_helper.h) is also possible if that is the direction you want.
Are the MOONCAKE_S3_* names and the reuse of the HAVE_OSS_ADAPTER build flag fine?

If the shape is OK, I'll open the PR as one squashed commit.

@pingqiu commented on GitHub (Oct 7, 2026): A follow-up with a working implementation, so the design question has something concrete behind it. Branch: https://github.com/pingqiu/Mooncake/compare/main...store-s3-standalone (based on current `main`) **Shape: a standalone S3 adapter. The OSS adapter is not modified.** - **New backend.** `"s3"` is a new `fs_adapter_type`, registered next to `"oss"` in `storage_backend.cpp` under the same libcurl + OpenSSL build condition. - **Separate engine.** `S3ObjectStorageAdapter` sits on its own protocol-neutral request engine, `RestObjectStorageAdapter`. It does not touch `oss_adapter.cpp/.h`, so existing OSS users see no change. The cost is that the OSS adapter and the new engine duplicate some code for now. - **Signing and protocol details.** Requests use AWS Signature V4, with path-style or virtual-hosted addressing. Empty query values are sent as `name=`, and the ListObjectsV2 continuation token goes back unencoded, as S3 returns it. - **Configuration.** Settings come from `MOONCAKE_S3_*`, with `AWS_*` fallbacks. - Credentials are read as one set, so a stray `AWS_SESSION_TOKEN` is never sent with `MOONCAKE_S3_*` keys. - The region falls back to `AWS_REGION`, then `AWS_DEFAULT_REGION`, then us-east-1 with a warning. - IP and `localhost` endpoints use path-style addressing. - **Retries.** Transient failures are retried, as the AWS SDKs do: up to 3 attempts with backoff. That covers connection resets, 429 and 5xx, and timeouts while the connection is still being set up (DNS, TCP, TLS). A request that was sent and then timed out is not retried, so a stalled server costs one request timeout, and client errors are not retried. Without the retries, the batch live test failed intermittently against AWS: a dropped SYN or a stalled TLS handshake occasionally pushed a connect past the connect timeout. **Tests** - **Signing.** SigV4 known-answer tests check against values computed independently with botocore. - **Configuration.** Tests cover credential sets, region fallback, scheme and addressing. - **Scripted servers.** Tests cover the continuation token, retry on 503 and dropped connections, no retry on 403, and giving up after 3 attempts. - **Request engine.** Because the S3 engine is separate, 24 of the engine tests in `oss_adapter_test.cpp` are ported to it in `s3_adapter_engine_test.cpp`, plus a connect-timeout test: - batch refill and connection limits; - reset, truncated and timeout faults; - range checks; - ordering and partial failures; - iovec rewind; - the health check. Where retry changes an outcome, the test says so. The slow timeout tests are gated behind `MOONCAKE_RUN_S3_FAULT_TIMEOUT_TESTS=1`. - **Live object tests.** Lifecycle, 1105-object batches with paginated listing, and a wrong-secret negative control pass against SeaweedFS, MinIO and AWS S3 (us-east-1, repeated runs). - **Store-level round trip.** This goes through `CreateStorageBackend` with `MOONCAKE_DISTRIBUTED_FS_TYPE=s3`: - the object health check; - a multi-slice `BatchOffload` of 33 objects, including an opaque key containing NUL, `/`, `%` and `\`; - a check that exactly those objects are in the bucket; - `BatchLoad` compared byte for byte; - `IsExist`. It passes against SeaweedFS, MinIO and AWS S3. - **Existing tests.** The OSS and file storage tests pass unchanged. **Questions** 1. Is a standalone adapter acceptable for a first PR? I also have a variant where `OssObjectStorageAdapter` and `S3ObjectStorageAdapter` share `RestObjectStorageAdapter` (no duplication, no public API change), and I can open that separately if you prefer it. An implementation on the AWS SDK already linked for HA snapshots (`common/s3_helper.h`) is also possible if that is the direction you want. 2. Are the `MOONCAKE_S3_*` names and the reuse of the `HAVE_OSS_ADAPTER` build flag fine? If the shape is OK, I'll open the PR as one squashed commit.

bsakaguchi commented

2026-10-29 21:23:12 +00:00

Author

Owner

@he-yufeng commented on GitHub (Oct 7, 2026):

Thanks for putting a working branch behind the proposal. Answering the two questions with current main as reference.

Standalone adapter first: yes, that is the right sequencing. Rewriting the shipped OSS adapter in the same PR that introduces a new backend mixes a behavior-preserving refactor with a feature, and review has to verify both at once. Land s3 standalone, then send the shared RestObjectStorageAdapter variant as a follow-up refactor with two real consumers to validate against. On the AWS SDK route: I would not take it. The SDK is linked for HA snapshots, but pulling it into the store client build for everyone is heavier than the libcurl + OpenSSL baseline the OSS adapter already keys on, and your ported engine tests would no longer exercise the same request path.
Names: MOONCAKE_S3_* primary with AWS_* fallback matches the house pattern exactly (oss_adapter_config.cpp does MOONCAKE_OSS_* primary, OSS_* alias, via ReadPrimaryOrAlias). Fine as proposed.

One thing I would not reuse literally: HAVE_OSS_ADAPTER. Today that define just means "CURL and OpenSSL were found" (mooncake-store/src/CMakeLists.txt:213-223), but the name and the status line both say OSS. If s3 registers under the same define, a build without the deps prints "OSS adapter: Disabled" and says nothing about s3, and the #else branch at the dispatch site (storage_backend.cpp:5618-5625) would tell an s3 user to install "libcurl and OpenSSL" with an OSS-flavored message. Give s3 its own define (HAVE_S3_ADAPTER) keyed off the same CURL+OpenSSL find_package, with its own status line and its own #else error string at dispatch. Cheap, and keeps the build output honest per adapter.

Two details to make explicit in the PR when you open it:

The retry line you drew (retry only before the request is sent, never after a sent-request timeout) is the conservative right one. Put it in a comment at the retry site: a PUT that timed out may have landed server-side, and the retry policy deliberately accepts a stale-object window over a duplicate-write one.
The 1105-object paginated listing test is the one most likely to rot silently against real S3 rate limits. If it is not already per-run skippable, gate it like the fault-timeout tests.

One squashed commit is fine, [Store] prefix. The SigV4 known-answer tests against botocore values plus the wrong-secret negative control are exactly the coverage this area needs.

@he-yufeng commented on GitHub (Oct 7, 2026): Thanks for putting a working branch behind the proposal. Answering the two questions with current main as reference. 1. Standalone adapter first: yes, that is the right sequencing. Rewriting the shipped OSS adapter in the same PR that introduces a new backend mixes a behavior-preserving refactor with a feature, and review has to verify both at once. Land s3 standalone, then send the shared RestObjectStorageAdapter variant as a follow-up refactor with two real consumers to validate against. On the AWS SDK route: I would not take it. The SDK is linked for HA snapshots, but pulling it into the store client build for everyone is heavier than the libcurl + OpenSSL baseline the OSS adapter already keys on, and your ported engine tests would no longer exercise the same request path. 2. Names: MOONCAKE_S3_* primary with AWS_* fallback matches the house pattern exactly (oss_adapter_config.cpp does MOONCAKE_OSS_* primary, OSS_* alias, via ReadPrimaryOrAlias). Fine as proposed. One thing I would not reuse literally: HAVE_OSS_ADAPTER. Today that define just means "CURL and OpenSSL were found" (mooncake-store/src/CMakeLists.txt:213-223), but the name and the status line both say OSS. If s3 registers under the same define, a build without the deps prints "OSS adapter: Disabled" and says nothing about s3, and the #else branch at the dispatch site (storage_backend.cpp:5618-5625) would tell an s3 user to install "libcurl and OpenSSL" with an OSS-flavored message. Give s3 its own define (HAVE_S3_ADAPTER) keyed off the same CURL+OpenSSL find_package, with its own status line and its own #else error string at dispatch. Cheap, and keeps the build output honest per adapter. Two details to make explicit in the PR when you open it: - The retry line you drew (retry only before the request is sent, never after a sent-request timeout) is the conservative right one. Put it in a comment at the retry site: a PUT that timed out may have landed server-side, and the retry policy deliberately accepts a stale-object window over a duplicate-write one. - The 1105-object paginated listing test is the one most likely to rot silently against real S3 rate limits. If it is not already per-run skippable, gate it like the fault-timeout tests. One squashed commit is fine, [Store] prefix. The SigV4 known-answer tests against botocore values plus the wrong-secret negative control are exactly the coverage this area needs.

bsakaguchi referenced this issue

2026-10-29 21:32:15 +00:00

[PR #1130] [MERGED] feat(metrics): add task completion latency tracking and reporting #2031

bsakaguchi referenced this issue

2026-10-29 21:33:08 +00:00

[PR #1305] [CLOSED] [TE] Revert "feat(metrics): add task completion latency tracking and reporting" #2145