Repository navigation
[Store] Bound concurrent object uploads during batch offload - #6
Conversation
Upload independent object-storage keys with at most eight workers, then report the successful keys and sizes once after every worker has joined. Keep the existing durable upload fencing and partial-failure semantics. Add bounded-overlap and partial-failure regression tests. Related: MindLab-Research/mint#2000
Treat throwing adapters as failed uploads so one key cannot terminate the provider worker or promote an incomplete batch. Test mixed standard and non-standard exceptions, successful keys and sizes, and all-failed caller/worker batches. Related: MindLab-Research/mint#2000
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
xjtu-ctgg
left a comment
There was a problem hiding this comment.
Review and fresh development-host E2E verification for 599255d39791c0e2a8e76c0febe3f527fb74bc77 against base 94140eed:
No blocking code findings in this review. Audited the bounded atomic work queue, source/iovec lifetime through join, success-key/metadata alignment, partial errors/exceptions, thread-start fallback, FileStorage pending retries/lifecycle lock, and the per-key upload/deletion fencing. This is an AI-assisted self-review, not a human approval.
Fresh verification on both requested development hosts:
- 26/26 focused unit tests on each host.
- Test-only pthread_create fault injection: 5 cases pass when all worker creation is refused; a 16-key mixed-result case passes when 2 workers start before EAGAIN. Both hosts pass. The E2E provider never loads this test injection.
- Two new full storage gates PASS (one per host), including independent hashes, eviction retry, real allocation failure, restart/cold recovery, checkpoint Materialize and offload-disabled fail-closed behavior.
| Host | Run ID | Full gate | Allocation failure (s) | Durability failure (s) |
|---|---|---|---|---|
| beijing | oss-e2e-cfb8a3bf817d49f0bbce54d15f517dc2 |
PASS | 62.139 | 3.011 |
| second | oss-e2e-93f0e18ea1174734a4d025665ec87cd8 |
PASS | 63.099 | 3.014 |
- Additional real-OSS parallel integrity run
oss-e2e-e23acf78a2a74ef5a40c3116debf0879: 14 and 32 concurrent PutBlob requests with 1/2/3MiB objects, plus checkpoint; all 47 OSS size/SHA-256 checks and all 47 cold reads after stopping/restarting the entire isolated stack pass. Checkpoint Materialize/file hashes also pass. - Provider SHA-256 on both hosts:
87acbcf18d93e3005f725e070a852b912a68db9478a0f61239530cfebcda88bb. Head/source and immutable event/runtime-binding hashes verified. - Scoped pre-commit, changed-line formatting and diff checks pass. GitHub currently exposes no Actions checks for this head; these results are development-host evidence.
The PR has been changed from draft to Ready for review as requested and remains unmerged. No image build, deployment or production change. 69 test objects in exactly 3 owned prefixes were cleaned and list readback confirms all are empty. Evidence is retained at /opt/mindlab/codex-2000-review on the respective hosts.
Description
After MindLab-Research/mint-ops#76 reduced polling and read-lease delays,
BatchOffloadstill serialized independent object uploads, so concurrent result batches waited for the sum of their cloud PUT times.Upload at most eight objects concurrently, including the calling thread; join all workers, then call completion once with only successful keys and their matching sizes. Keep per-key upload/deletion fencing and the FileStorage lifecycle lock. Worker-start failure falls back to fewer workers.
Follow-up review also reproduced a process-abort bug in the first revision (
cf5c0424): an adapter exception escaping an upload worker terminates the provider (isolated test returned SIGABRT / -6). The final revision contains exceptions per key, leaves that key pending for the existing retry path, and continues other uploads. Tests cover standard/non-standard exceptions, mixed success/error/throw results, caller-only and multi-worker all-failed batches, and completion after uploads drain.Related: MindLab-Research/mint#2000, claimed with
take it. Open / Ready for review at the user's request; no merge without the user's decision.Module
mooncake-store)Type of Change
How Has This Been Tested?
Development hosts only: Beijing
115.191.57.4compiled and ran the full width scan plus one full gate;47.81.60.206ran the same unit binary and three consecutive full gates. No image build or deployment.Test commands:
Test results:
Single-object mean: 0.338s -> 0.341s (+0.9%).
Full storage gate receipts
oss-e2e-2bc7a2c7e4fe4c4fad0b51328a8c1439oss-e2e-15a5cb06e9634b1ca1bc0c56776e3bb2oss-e2e-eaf58d36e75b40c88b7742e19dff4543oss-e2e-769298ecd3de42a59c4bde9369c318c1All four runs invoke the original untrimmed
Gate.run(): pressure and successful eviction retries; independent OSS size/SHA-256; adapter+optimizer Publish; stop all owned memory holders and cold Get of 8 objects; checkpoint Materialize and file hashes; real native allocation failure; and rejection when offload is disabled. The failure receipts recordlocal_hit=false, an unchanged source and UNAVAILABLE.Chunk/read/sidecar buffers are 1GiB and the provider segment is 512MiB, so the 768MiB allocation fixture reaches the native allocator as one physical chunk and returns
rc=-200within the 80s client limit. Retry-log matching maps chunk keys to parent logical keys. No assertion or negative control is skipped. Durability rejection stays within its 6s limit.Source and runtime binding
599255d39791c0e2a8e76c0febe3f527fb74bc77; fork main base94140eed2894b68d0a34e094cc3251f6181de6cf.87acbcf18d93e3005f725e070a852b912a68db9478a0f61239530cfebcda88bb.383e08139b3aabaf0169bb21b2ea85a5017afa0ae7f8c5120d5c9c805c86be9c.6c98aa636d6998fb5b1edf89ce8f7090685c75a5provider after mint-ops#76. Same Master, dev95af8d2a sidecar, 6c98 shared libraries, 100ms provider polling and 5000ms read lease. Candidate main also contains merged prefix-normalization fix(store): normalize trailing slashes in the S3 key prefix #2; the test prefixes have no trailing slash.runtime-binding.json.Repro, build, unit, scan and immutable gate receipts are preserved in
/opt/mindlab/codex-2000-r2on the respective hosts, with named-file archives excluding credentials, payloads and SDK debug logs. Cleanup removed 999 objects from exactly 30 owned prefixes; all were listed again and confirmed empty.The two hosts ran isolated local Master/provider/sidecar stacks against real OSS. These are development correctness and batch-latency results, not a shared-Master cross-region capacity test.
One Beijing width-7 attempt stopped before candidate samples because the unchanged sidecar exited with a gRPC transport error during startup; its test port was subsequently observed in TIME_WAIT. The failed logs are retained. Widths 7 and 14 were restarted in full on free ports 26308–26313 and only their completed attempts are included. No provider exception or upload failure was observed in that startup attempt.
Checklist
./scripts/code_format.shAI Assistance Disclosure
Claude Code investigated the original bottleneck and drafted the parallel upload change. Codex prepared the final code/tests, reproduced and fixed the worker exception abort, built on Beijing, validated the identical artifacts on two development hosts, and assembled these receipts. Human line-by-line review remains pending. No merge performed.
Final-head review and fresh E2E verification (2026-11-04)
Review and fresh development-host E2E verification for
599255d39791c0e2a8e76c0febe3f527fb74bc77against base94140eed:No blocking code findings in this review. Audited the bounded atomic work queue, source/iovec lifetime through join, success-key/metadata alignment, partial errors/exceptions, thread-start fallback, FileStorage pending retries/lifecycle lock, and the per-key upload/deletion fencing. This is an AI-assisted self-review, not a human approval.
Fresh verification on both requested development hosts:
oss-e2e-cfb8a3bf817d49f0bbce54d15f517dc2oss-e2e-93f0e18ea1174734a4d025665ec87cd8oss-e2e-e23acf78a2a74ef5a40c3116debf0879: 14 and 32 concurrent PutBlob requests with 1/2/3MiB objects, plus checkpoint; all 47 OSS size/SHA-256 checks and all 47 cold reads after stopping/restarting the entire isolated stack pass. Checkpoint Materialize/file hashes also pass.87acbcf18d93e3005f725e070a852b912a68db9478a0f61239530cfebcda88bb. Head/source and immutable event/runtime-binding hashes verified.The PR has been changed from draft to Ready for review as requested and remains unmerged. No image build, deployment or production change. 69 test objects in exactly 3 owned prefixes were cleaned and list readback confirms all are empty. Evidence is retained at
/opt/mindlab/codex-2000-reviewon the respective hosts.