diff --git a/content/blog/design/listobjects-nosuchbucket.md b/content/blog/design/listobjects-nosuchbucket.md new file mode 100644 index 00000000..ffdad870 --- /dev/null +++ b/content/blog/design/listobjects-nosuchbucket.md @@ -0,0 +1,302 @@ +--- +title: "A ListObjects Shortcut Must Not Turn a Missing Bucket into an Empty One" +linkTitle: "ListObjects NoSuchBucket" +date: 2026-08-26 +lastmod: 2026-08-26 +author: "Ruohang Feng" +summary: > + Three ListObjects shortcuts return EOF before touching storage, causing a missing bucket to appear as an empty listing. This record explains the SILO #32 / PR #37 regression, S3 compatibility value, minimal shortcut-only existence check, cluster fan-out cost, derived risks, and acceptance decision. +tags: [Design, S3, Compatibility, ListObjects] +weight: 25 +draft: false +url: "/blog/design/listobjects-nosuchbucket/" +--- + +This document records the problem analysis, design discussion, and repair decision for [SILO #32](https://github.com/pgsty/silo/issues/32) and [PR #37](https://github.com/pgsty/silo/pull/37). + +> **Status on 2026-08-26:** [PR #37](https://github.com/pgsty/silo/pull/37) was updated to the DCO-signed head [`e9c5340be`](https://github.com/pgsty/silo/commit/e9c5340be94044daf3410272af54bc98832dd377), formally approved, and merged as [`49c8aeac4`](https://github.com/pgsty/silo/commit/49c8aeac403916f52f8588bbe8ee42753d86eeef); [#32](https://github.com/pgsty/silo/issues/32) closed automatically. DCO, VulnCheck, and all six Go CI jobs passed on the exact PR head; the post-merge `main` VulnCheck and all six Go CI jobs also passed. No tagged release, package, container image, deployment, or production endpoint has yet been verified to contain the repair.
+> **Scope:** verify bucket existence only for three listing shortcuts that bypass storage; do not restore the generic `checkBucketExist`, change the normal listing path, or introduce an existence cache.
+> **Release boundary:** local commit, push, remote CI, merge, tag, package, container image, deployment, and production verification are independent gates. + +## Too Long; Didn't Read (TL;DR) {#tldr} + +The problem is real and worth fixing. A normal `ListObjects`, `ListObjectsV2`, or `ListObjectVersions` request against a missing bucket reaches storage and receives `BucketNotFound`. Three inputs, however, return early: + +- a marker outside the prefix; +- `max-keys=0`; +- a prefix beginning with `/`, including the `Prefix="/"` boto3 reproduction from #32. + +Those branches return `io.EOF` directly. The caller treats EOF as a successful end of listing, so the client receives an empty 200 rather than S3's 404 `NoSuchBucket`. The identity of the same missing resource changes from an error to success solely because the selection parameters differ. That breaks S3 compatibility and blocks a real user's upgrade from the pre-regression release. + +The repair must not put an expensive bucket check back in every listing. The selected design replaces only the three bare `io.EOF` returns with a small helper. The helper calls `GetBucketInfo` once: it returns the real error if the bucket is absent or cannot be confirmed, and preserves `io.EOF` when the bucket exists. The normal listing hot path is untouched. Only requests that would otherwise exit before storage pay the extra peer-and-disk fan-out. + +That decision has now been executed: **the strengthened repair passed local review, the exact PR head passed every remote check, and the expected-head-guarded merge entered a green `main`.** + +## What is the problem? {#problem} + +### One API exposes two bucket-existence semantics {#two-semantics} + +#32 reproduces the defect by calling the following against a missing bucket: + +```python +s3.list_objects(Bucket="missing-bucket", Prefix="/") +``` + +AWS S3 raises `NoSuchBucket`; SILO returns a successful empty listing. The difference is not in authentication, routing, or XML serialization. It comes from the object-layer `listPath` control flow: + +```text +regular prefix + -> enter listMerged + -> consult storage + -> missing volume/bucket becomes BucketNotFound + -> HTTP 404 NoSuchBucket + +shortcut input + -> listPath returns io.EOF early + -> storage is never consulted + -> the caller treats EOF as normal completion + -> HTTP 200 with an empty listing +``` + +`/` is not the only trigger: + +| Shortcut condition | Why the result must be empty | Defect before the repair | +| --- | --- | --- | +| Marker does not begin with the prefix | The implementation does not scan this disjoint range | Returns EOF without confirming the bucket | +| `max-keys=0` | The caller asks for zero keys | Incorrectly equates “zero results” with “valid resource” | +| Prefix begins with `/` | SILO's flat key space produces no entries for this form | The filter short-circuits before bucket identity | + +For an existing bucket, returning an empty listing from these branches is a reasonable optimization. For a missing bucket, the same EOF masks the resource error that should take precedence. + +### The regression has a known origin {#regression} + +The reporter confirmed correct behavior in `RELEASE.2024-01-29T03-56-32Z` and the regression beginning with `RELEASE.2024-01-31T20-20-33Z`. The corresponding upstream change is [minio/minio#18917](https://github.com/minio/minio/pull/18917) / [`80ca12008`](https://github.com/minio/minio/commit/80ca120088be9950fa35467975dd9d8dc1bd4176). It removed `GetBucketInfo` from generic argument checks and relied on actual Put, List, and Multipart storage operations to expose a missing bucket. + +That optimization works on normal paths but leaves a gap: an early-return path never reaches the storage operation that is now responsible for producing the error. #32 does not require a broad rollback of the upstream optimization. It repairs the overlooked control-flow exits. + +## Why fix it? {#why-fix} + +### The S3 contract explicitly requires `NoSuchBucket` {#s3-contract} + +Both [AWS ListObjects](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjects.html) and [ListObjectsV2](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html) define `NoSuchBucket` as HTTP 404 when the specified bucket does not exist. `prefix`, marker, `start-after`, and `max-keys` select listing results; they must not turn a missing bucket identity into a successful request. + +`ListObjectVersions` shares the same object-layer listing engine. Giving V1, V2, and version listings the same existence behavior on the same shortcut inputs prevents the three public APIs from diverging further. + +### An empty 200 changes client decisions {#client-impact} + +An empty 200 and a 404 are not interchangeable presentation details: + +- 404 tells provisioning or test code to create the bucket, fix configuration, or stop; +- an empty 200 asserts that the bucket exists but has no matching objects; +- SDKs, synchronization tools, and integration tests continue down different branches; +- a test using SILO as an S3 substitute can pass locally and fail against AWS. + +#32 also establishes a direct upgrade impact: an application relying on the older correct behavior cannot upgrade past the regression. The repair restores both S3 parity and upgrade compatibility. + +### The repair surface is narrow and testable {#repair-value} + +The bug is confined to three adjacent early returns. It does not involve object data, metadata formats, sorting, pagination-token encoding, permissions, or wire schemas. A very small production change can be pinned down with object-layer and HTTP-level contracts, so the benefit clearly exceeds the implementation risk. + +## Why not restore the global check? {#performance-constraint} + +Upstream did not remove generic `GetBucketInfo` as incidental cleanup. [The motivation for #18917](https://github.com/minio/minio/pull/18917) states that checking the bucket before every Put, List, and Multipart operation fans out across servers; even after vectorization, the cost becomes visible beyond 100 nodes. + +In current SILO, `erasureServerPools.GetBucketInfo` calls `S3PeerSys.GetBucketInfo`. That operation concurrently asks every peer and reduces quorum per pool, while each peer checks its local bucket state. It is not a cheap in-memory map lookup. + +Two extremes are therefore unacceptable: + +- **never check:** keep the incorrect empty 200; +- **check before every List:** restore semantics while undoing a critical large-cluster optimization. + +The actual design question is whether the check can be confined to branches that never touch storage and therefore cannot discover the missing bucket naturally. It can. + +## How is it fixed? {#implementation} + +### Replace only three bare EOF returns {#three-shortcuts} + +In `cmd/metacache-server-pool.go`, each shortcut previously executed: + +```go +return entries, io.EOF +``` + +It now executes: + +```go +return entries, z.listPathShortcutEOF(ctx, o.Bucket) +``` + +The helper has only two classes of outcome: + +```go +func (z *erasureServerPools) listPathShortcutEOF(ctx context.Context, bucket string) error { + if _, err := z.GetBucketInfo(ctx, bucket, BucketOptions{}); err != nil { + return err + } + return io.EOF +} +``` + +- existing bucket: preserve the previous empty-list behavior; +- missing bucket: pass `BucketNotFound` into the existing error mapping, producing HTTP 404 `NoSuchBucket`; +- state cannot be confirmed: propagate quorum, offline, timeout, or context errors instead of fabricating success. + +The normal `listMerged`, metacache scan, sorting, pagination, and response-generation paths do not change. + +### Why the helper belongs here {#helper-boundary} + +The check must sit next to the shortcut for three reasons: + +1. only this layer knows that it is about to bypass every storage access; +2. moving it into generic argument validation charges every call; +3. moving it into the scan layer cannot help because these branches never scan. + +The name intentionally states the boundary. This is not a new generic `checkBucketExist`; it restores missing existence semantics immediately before a shortcut returns EOF. + +### Do not add a cache {#no-cache} + +A bucket-existence cache could reduce fan-out but immediately creates invalidation questions for create, delete, site replication, recovery, and expiry. Adding a second source of truth for three low-frequency shortcuts costs more complexity and consistency risk than it saves. + +The selected implementation uses the existing `GetBucketInfo` source of truth. If future telemetry shows that large clusters receive frequent `max-keys=0`, slash-prefix, or disjoint-marker probes, the project can evaluate a dedicated metadata fast path, rate limiting, or a carefully invalidated cache using real data rather than speculative machinery in this compatibility patch. + +## Test and review evidence {#verification} + +### Object-layer contract {#object-layer-tests} + +The object-layer test runs against single-drive and multi-drive erasure setups and exercises four inputs: + +- slash-prefixed prefix; +- zero limit; +- marker outside prefix; +- a regular prefix as a control that still receives the error naturally from storage. + +Each case covers `ListObjects`, `ListObjectsV2`, and `ListObjectVersions`, using the typed `isErrBucketNotFound` predicate rather than brittle English error-string comparison. + +### HTTP contract {#http-tests} + +The handler test sends genuine signed requests for all three public APIs: + +| API | Request shape | Assertion | +| --- | --- | --- | +| ListObjects | `GET /missing-bucket?prefix=/` | HTTP 404 and XML code `NoSuchBucket` | +| ListObjectsV2 | Add `list-type=2` | HTTP 404 and XML code `NoSuchBucket` | +| ListObjectVersions | Add `versions` | HTTP 404 and XML code `NoSuchBucket` | + +The HTTP test uses the real slash-prefix reproduction from #32. The other two shortcuts are enumerated at the object layer. This proves final wire behavior without repeating the full matrix in the slower handler fixture. + +### Local quality gates {#local-gates} + +The improved local commit passed: + +```text +go test ./cmd -count=1 +the new object-layer and HTTP regressions (10 subcases) +focused go test -race +related existing listing tests +CGO_ENABLED=0 go build ./... +go vet ./... +CI-scope gofmt and git diff --check +post-commit focused regression rerun +``` + +The full local `cmd` test completed in 116.215 seconds. An independent local Claude Code review used the Fable model at Max effort to inspect the exact tree, call paths, error mapping, tests, performance boundary, and this decision. Its verdict was **GO**, with no mandatory pre-merge change. + +The DCO-signed PR head `e9c5340be` then passed eight remote checks: [DCO](https://github.com/pgsty/silo/actions/runs/32961304270), [VulnCheck](https://github.com/pgsty/silo/actions/runs/32961304271), and six jobs in [Go CI](https://github.com/pgsty/silo/actions/runs/32961304309). After merge, the resulting `main` commit `49c8aeac4` independently passed [VulnCheck](https://github.com/pgsty/silo/actions/runs/32962256729) and all six [Go CI](https://github.com/pgsty/silo/actions/runs/32962256823) jobs. The slowest checks were PR cross-compile at 9 minutes 47 seconds and post-merge cross-compile at 9 minutes 30 seconds. + +## Can it introduce new problems? {#derived-risks} + +### Shortcut requests now fan out across the cluster {#fanout-cost} + +This is the most important and deliberately accepted cost. A shortcut on an existing bucket used to be little more than a local branch; it now calls `GetBucketInfo`. Directional local microbenchmarks observed: + +| Path | Observed magnitude | +| --- | ---: | +| Shortcut before the repair | about 0.55 μs, 7 allocations | +| Repaired single-drive shortcut | about 7.8–8.1 μs, 45–47 allocations | +| Repaired 32-drive shortcut | about 70–81 μs, 977 allocations | +| Normal 32-drive listing | about 0.95 ms | + +These numbers show local relative cost only; they are not a latency prediction for a 100+ node deployment. Real distributed execution adds peer networks, quorum, and slowest-node tail latency, potentially making the gap much larger. That is precisely why the check must not expand into the normal listing path. + +The risk concentrates in malformed or probe-style traffic. A misconfigured client polling `max-keys=0`, a slash prefix, or disjoint markers at high frequency can amplify what was a cheap request into peer-and-disk work. After merge, the actual frequency of these inputs should be observed through S3 traces or metrics; rate limiting or optimization should follow evidence. + +### A degraded cluster exposes more real errors {#degraded-cluster} + +Previously, a shortcut could return an empty 200 while peers were offline or bucket quorum was unavailable because it never consulted cluster state. The repair can return quorum, timeout, or service errors in those conditions. + +That is more honest behavior, not an availability regression: if the server cannot establish that the bucket exists, it must not assert a valid empty bucket. Clients depending on unconditional empty success will nevertheless observe a behavior change. + +### Bucket create/delete races are not linearizable {#race-window} + +`GetBucketInfo` and returning the empty result are two actions. The bucket can be deleted immediately after the check, or created immediately after a missing-bucket result is formed. This patch does not and should not add a transaction spanning bucket lifecycle to a listing shortcut. + +This is the same concurrency class as other APIs that validate a resource before acting. The repair guarantees that the request no longer succeeds with **no existence evidence at all**; it does not promise a cross-node, cross-lifecycle linearizable snapshot of an empty listing. + +### Clients relying on the bug will receive 404 {#behavior-change} + +Some clients may have adopted the missing bucket's empty 200 as fact. They will now enter an error branch. This is a visible compatibility change, but it restores the documented S3 contract and the pre-regression behavior. Preserving the bug merely transfers upgrade cost to clients that correctly rely on 404. + +### Two adjacent edges remain out of scope {#remaining-edges} + +The adversarial review recorded two non-blocking P3 boundaries: + +1. When resuming a metacache continuation, the `c.fileNotFound` branch still returns bare `io.EOF`. A stale or crafted continuation token used after bucket deletion could theoretically receive an empty 200. Adding `GetBucketInfo` there would affect normal continuation traffic and needs a separate performance and error-precedence design. +2. Some V1 and version-list marker/prefix combinations return `NotImplemented` during HTTP handler validation before reaching the object layer; the V2 `start-after` route can reach it. This patch fixes storage shortcuts masking a missing bucket; it does not redefine precedence between malformed parameters and resource errors. + +Neither blocks merge. The first is outside #32's ordinary initial-list reproduction; the second is inherited handler behavior. Recording them prevents “all three shortcuts are covered” from being overstated as byte-for-byte AWS parity for every possible parameter combination. + +## Alternatives considered {#alternatives} + +### Keep upstream behavior {#keep-upstream} + +This has zero performance change and minimizes fork divergence. It also keeps a documented S3 incompatibility, a regression with a known release boundary, and a misleading result when SILO is used as an integration-test substitute. For a narrow and well-tested compatibility repair, that tradeoff is no longer justified. + +### Restore generic `checkBucketExist` {#restore-global-check} + +This covers every path at once but reintroduces peer fan-out into every Put, List, and Multipart operation, directly undoing the large-cluster optimization from #18917. The cost is disproportionate and the option is rejected. + +### Fix only `Prefix="/"` {#slash-only} + +That passes the single issue reproduction but leaves the same root defect in `max-keys=0` and marker-outside-prefix. The branches are adjacent and share the same semantics, so one helper is simpler and less likely to regress. + +### Add a bucket-existence cache {#existence-cache} + +This makes shortcuts cheaper but requires semantics for create, delete, replication, recovery, and stale TTL windows. There is no telemetry showing enough shortcut traffic to justify that complexity, so it is not selected. + +## Complexity and cost-benefit {#tradeoff} + +| Dimension | Assessment | Rationale | +| --- | --- | --- | +| Production-code complexity | Low | Three call sites and a seven-line helper; no new state, dependency, or format | +| Test complexity | Low to medium | V1, V2, versions, three shortcuts, a control, and HTTP mapping all need coverage | +| Normal-path risk | Very low | No check is added to the `listMerged` hot path | +| Shortcut runtime cost | Materially higher | A local EOF becomes cluster-wide `GetBucketInfo` | +| Compatibility value | High | Restores 404 `NoSuchBucket`, pre-regression behavior, and S3 test fidelity | +| Operational complexity | Low | No migration, configuration, feature flag, cache, or cross-repository dependency | + +The overall cost-benefit is favorable. The reason is not that `GetBucketInfo` is cheap—it is not—but that its cost is strictly limited to three shortcuts that otherwise cannot discover the missing bucket. A narrow performance cost in exchange for explicit protocol correctness is better than either a global rollback or indefinitely preserving the incorrect behavior. + +## Acceptance decision and remaining gates {#decision} + +The final decision was: **accept and merge the strengthened PR #37 revision without expanding the production scope.** + +The accepted sequence was: + +1. replace the old fork head with the current-`main`, DCO-signed revision while preserving Jason Lin as a co-author; +2. retain typed error predicates, V1/V2/version-list object-layer coverage, and HTTP-level 404 / `NoSuchBucket` assertions; +3. update the PR description with the shortcut fan-out cost and unchanged normal-path boundary; +4. approve the fork workflows and require all eight reported checks to pass on exact head `e9c5340be`; +5. submit a formal approving review against that head; +6. merge with an expected-head guard, producing `49c8aeac4`, automatically close #32, and require the resulting `main` Go CI and VulnCheck to pass independently. + +No cache, feature flag, additional abstraction, or continuation-token redesign was required. High-frequency shortcut traffic and large-cluster tail latency remain observability follow-ups, not reasons for speculative code expansion. + +Repository integration is complete. A tag, package, `docker.io/pgsty/minio` image, deployment, and real S3-client verification must still complete before the repair can be described as delivered to users. + +## Conclusion {#conclusion} + +The issue is not merely “a slash prefix reports the wrong error.” The listing engine uses `io.EOF` to mean two different things: an empty result from an existing bucket and an early exit that never established whether the bucket exists. Removing generic existence checks for large-cluster performance was a sound upstream optimization, but the shortcuts violate its premise that a real storage operation will naturally surface a missing bucket. + +The selected repair restores that premise by calling the existing `GetBucketInfo` only at three storage-bypassing exits. It makes those requests more expensive and exposes real errors on degraded clusters; both are explicit costs. In return, SILO restores S3's 404 semantics, upgrade compatibility, and test fidelity while preserving the upstream optimization on the normal listing hot path. + +This worthwhile, controlled compatibility fix is now merged and green on `main`; release delivery remains a separate gate. diff --git a/content/blog/design/listobjects-nosuchbucket.zh.md b/content/blog/design/listobjects-nosuchbucket.zh.md new file mode 100644 index 00000000..152042e1 --- /dev/null +++ b/content/blog/design/listobjects-nosuchbucket.zh.md @@ -0,0 +1,302 @@ +--- +title: "ListObjects 快捷路径不能把不存在的桶伪装成空桶" +linkTitle: "ListObjects NoSuchBucket" +date: 2026-08-26 +lastmod: 2026-08-26 +author: "冯若航" +summary: > + 三条 ListObjects 快捷路径会在访问存储前直接返回 EOF,导致不存在的桶被错误地表示为空列表。本文记录 SILO #32 / PR #37 的回归来源、S3 兼容性价值、只在快捷路径检查桶存在性的最小修复、集群扇出代价、衍生边界与验收决策。 +tags: [设计, S3, 兼容性, ListObjects] +weight: 25 +draft: false +url: "/zh/blog/design/listobjects-nosuchbucket/" +--- + +本文是 [SILO #32](https://github.com/pgsty/silo/issues/32) 与 [PR #37](https://github.com/pgsty/silo/pull/37) 的问题分析、设计讨论与修复决策归档。 + +> **截至 2026-08-26 的状态:** [PR #37](https://github.com/pgsty/silo/pull/37) 已更新为带 DCO sign-off 的 head [`e9c5340be`](https://github.com/pgsty/silo/commit/e9c5340be94044daf3410272af54bc98832dd377),通过正式批准并合并为 [`49c8aeac4`](https://github.com/pgsty/silo/commit/49c8aeac403916f52f8588bbe8ee42753d86eeef);[#32](https://github.com/pgsty/silo/issues/32) 随后自动关闭。精确 PR head 的 DCO、VulnCheck 与六项 Go CI 全部通过,合并后 `main` 的 VulnCheck 与六项 Go CI 也全部通过。尚未验证任何 tag、软件包、容器镜像、部署或生产端点已经包含本修复。
+> **范围:** 只为三条绕过存储的列表快捷路径补上桶存在性检查;不恢复通用 `checkBucketExist`,不改变正常列表路径,也不增加存在性缓存。
+> **发布边界:** 本地提交、push、远端 CI、merge、tag、软件包、容器镜像、部署与生产验证是相互独立的门槛。 + +## 太长不看(TL;DR) {#tldr} + +这个问题是真的,而且值得修。对不存在的桶执行 `ListObjects`、`ListObjectsV2` 或 `ListObjectVersions` 时,普通请求会在扫描存储时得到 `BucketNotFound`;但以下三种输入会提前结束: + +- marker 不属于 prefix; +- `max-keys=0`; +- prefix 以 `/` 开头,包括 #32 中 boto3 使用的 `Prefix="/"`。 + +这些分支直接返回 `io.EOF`,上层将它解释为“列表正常结束”,于是客户端收到空的 200,而不是 S3 的 404 `NoSuchBucket`。同一个不存在的资源,仅仅因为过滤参数不同就从错误变成成功,这既破坏 S3 兼容性,也阻塞了从回归前版本升级的真实用户。 + +修复不应把昂贵的桶检查放回每一次列表请求。选定方案只把三个裸 `io.EOF` 改为调用一个小 helper:helper 调用一次 `GetBucketInfo`;桶不存在或集群无法确认时返回真实错误,桶存在时仍返回 `io.EOF`。因此正常列表热路径完全不变,额外的 peer/disk 扇出只由原本会在访问存储前结束的请求承担。 + +这项决策现已执行完成:**补强修复通过本地评审,精确 PR head 通过全部远端检查,并通过 expected-head guard 合入绿色 `main`。** + +## 这是什么问题 {#problem} + +### 同一个 API 出现两套桶存在性语义 {#two-semantics} + +#32 给出的最小复现是在不存在的桶上调用: + +```python +s3.list_objects(Bucket="missing-bucket", Prefix="/") +``` + +AWS S3 抛出 `NoSuchBucket`,SILO 却返回一个成功的空列表。差异不在认证、路由或 XML 编码,而在对象层 `listPath` 的控制流: + +```text +普通 prefix + -> 进入 listMerged + -> 访问存储 + -> 不存在的 volume/bucket 变成 BucketNotFound + -> HTTP 404 NoSuchBucket + +快捷输入 + -> listPath 提前返回 io.EOF + -> 完全没有访问存储 + -> 上层把 EOF 当成正常结束 + -> HTTP 200 + 空列表 +``` + +触发提前返回的不是只有 `/` prefix: + +| 快捷条件 | 为什么结果必为空 | 修复前的缺陷 | +| --- | --- | --- | +| marker 不以 prefix 开头 | 当前实现不扫描这个不相交区间 | 未确认桶是否存在就返回 EOF | +| `max-keys=0` | 调用者明确要求返回零个 key | 把“零结果”错误地等同于“资源有效” | +| prefix 以 `/` 开头 | SILO 的扁平 key 空间不会生成这种列表项 | 过滤条件在桶身份之前短路 | + +对存在的桶,这三个分支返回空列表是合理优化;对不存在的桶,同一个 EOF 会掩盖应该优先返回的资源错误。 + +### 这是一个有明确起点的回归 {#regression} + +报告者确认 `RELEASE.2024-01-29T03-56-32Z` 行为正确,从 `RELEASE.2024-01-31T20-20-33Z` 开始出现回归。对应上游变更是 [minio/minio#18917](https://github.com/minio/minio/pull/18917) / [`80ca12008`](https://github.com/minio/minio/commit/80ca120088be9950fa35467975dd9d8dc1bd4176):它从通用参数检查中删除了 `GetBucketInfo`,让实际 Put、List 与 Multipart 存储操作自行暴露不存在的桶。 + +这个优化对正常路径成立,但留下一个边角:提前返回的路径根本不会到达能够暴露错误的存储操作。#32 不是要求全面撤销上游优化,而是补上优化后遗漏的控制流分支。 + +## 为什么要修复 {#why-fix} + +### S3 契约明确要求 `NoSuchBucket` {#s3-contract} + +[AWS ListObjects](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjects.html) 与 [ListObjectsV2](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html) 都把 `NoSuchBucket` 定义为 404:指定桶不存在。`prefix`、marker、`start-after` 与 `max-keys` 是结果选择条件,不应让不存在的 bucket identity 变成一次成功请求。 + +`ListObjectVersions` 共享同一个对象层列表引擎。让三个公开列表 API 在同样的 shortcut 输入上遵守同一桶存在性语义,可以避免 V1、V2 与版本列表继续分叉。 + +### 错误的空列表会改变调用者决策 {#client-impact} + +空 200 与 404 不是可互换的展示细节: + +- 404 告诉 provisioning 或测试代码先创建桶、修正配置或终止流程; +- 空 200 声称桶存在,只是暂时没有匹配对象; +- SDK、同步工具和集成测试会沿两条不同的控制流继续执行; +- 使用 SILO 模拟 S3 的测试可能在本地通过,却在 AWS 上失败。 + +#32 还给出了直接升级影响:依赖旧有正确行为的应用无法升级到回归后的版本。修复因此同时恢复 S3 parity 和版本升级兼容性。 + +### 修复面很窄,也容易建立强回归契约 {#repair-value} + +问题集中在三个相邻的 early return,不涉及对象数据、元数据格式、排序、分页 token 编码、权限或 wire schema。可以用很少的生产代码修复,并在对象层与 HTTP 层精确锁定行为,收益明显高于实现风险。 + +## 为什么不能简单恢复全局检查 {#performance-constraint} + +上游删除通用 `GetBucketInfo` 不是随意清理。[#18917 的动机](https://github.com/minio/minio/pull/18917) 明确指出:Put、List 与 Multipart 每次先检查桶会在所有 server 间扇出;即使做过向量化,超过 100 个节点后成本仍明显可见。 + +在 SILO 当前实现中,`erasureServerPools.GetBucketInfo` 会调用 `S3PeerSys.GetBucketInfo`:请求并发发往所有 peer,再按 pool 聚合 quorum。每个 peer 还要检查本地 bucket 状态。它不是一次廉价的内存 map 查询。 + +因此存在两个都不应接受的极端: + +- **完全不检查:** 保留错误的空 200; +- **每次 List 都先检查:** 恢复正确语义,却撤销大型集群上的关键优化。 + +真正的设计问题是:能否只给“不会访问存储、因此无法自然发现缺桶”的分支补检查。答案是可以。 + +## 怎么修复 {#implementation} + +### 只替换三个裸 EOF {#three-shortcuts} + +在 `cmd/metacache-server-pool.go` 中,三个 shortcut 原来都执行: + +```go +return entries, io.EOF +``` + +改为: + +```go +return entries, z.listPathShortcutEOF(ctx, o.Bucket) +``` + +helper 的契约只有两类结果: + +```go +func (z *erasureServerPools) listPathShortcutEOF(ctx context.Context, bucket string) error { + if _, err := z.GetBucketInfo(ctx, bucket, BucketOptions{}); err != nil { + return err + } + return io.EOF +} +``` + +- 桶存在:保留原有空列表行为; +- 桶不存在:把 `BucketNotFound` 交给既有错误映射,HTTP 返回 404 `NoSuchBucket`; +- 集群无法可靠确认:传播 quorum、offline、timeout 或 context 错误,不再伪造成功。 + +正常的 `listMerged`、metacache 扫描、排序、分页和响应生成全部不变。 + +### 为什么 helper 放在这里 {#helper-boundary} + +检查必须紧贴 shortcut,原因有三点: + +1. 只有这一层知道自己即将绕过全部存储访问; +2. 上移到通用参数校验会让所有调用付费; +3. 下移到扫描层对这些分支无效,因为它们永远不会进入扫描。 + +helper 名称也刻意表达边界:它不是新的通用 `checkBucketExist`,而是“在 shortcut 返回 EOF 前补齐缺失的存在性语义”。 + +### 不引入缓存 {#no-cache} + +用 bucket-existence cache 可以降低扇出,但会立即引入创建、删除、site replication、恢复与过期策略的一致性问题。为了三个低频 shortcut 增加一套新的事实源,复杂度与失效风险都高于收益。 + +当前选择使用已有 `GetBucketInfo` 作为事实源。如果未来遥测证明大型集群频繁收到 `max-keys=0` 或 slash-prefix 探测,再基于数据考虑专用元数据快路、限流或安全缓存,而不是在这个兼容修复中预先设计。 + +## 测试与评审证据 {#verification} + +### 对象层契约 {#object-layer-tests} + +对象层测试在单盘与多盘 erasure setup 上,对以下四类输入逐一调用: + +- slash-prefixed prefix; +- zero limit; +- marker outside prefix; +- regular prefix,作为仍由存储自然报错的控制组。 + +每组都覆盖 `ListObjects`、`ListObjectsV2` 与 `ListObjectVersions`,并使用类型化的 `isErrBucketNotFound` 判断,而不是比较易碎的英文错误文本。 + +### HTTP 契约 {#http-tests} + +Handler 测试使用真实签名请求验证三个公开 API: + +| API | 请求形态 | 断言 | +| --- | --- | --- | +| ListObjects | `GET /missing-bucket?prefix=/` | HTTP 404,XML code 为 `NoSuchBucket` | +| ListObjectsV2 | 加 `list-type=2` | HTTP 404,XML code 为 `NoSuchBucket` | +| ListObjectVersions | 加 `versions` | HTTP 404,XML code 为 `NoSuchBucket` | + +HTTP 测试选择 #32 的真实 slash-prefix 复现;另外两个 shortcut 已在对象层穷举。这样既证明最终 wire behavior,又避免把同一矩阵在较慢的 handler fixture 中重复三遍。 + +### 本地质量门槛 {#local-gates} + +本地改进提交完成了以下验证: + +```text +go test ./cmd -count=1 +新对象层与 HTTP 回归测试(10 个子场景) +定向 go test -race +相关既有列表测试 +CGO_ENABLED=0 go build ./... +go vet ./... +CI 范围 gofmt 与 git diff --check +提交后的定向回归复验 +``` + +本地完整 `cmd` 测试用时 116.215 秒。独立本机 Claude Code 使用 Fable 模型与 Max effort 审查了精确代码树、调用路径、错误映射、测试、性能边界和本轮决策,给出 **GO**,没有 mandatory pre-merge change。 + +带 DCO sign-off 的 PR head `e9c5340be` 随后通过八项远端检查:[DCO](https://github.com/pgsty/silo/actions/runs/32961304270)、[VulnCheck](https://github.com/pgsty/silo/actions/runs/32961304271) 与 [Go CI](https://github.com/pgsty/silo/actions/runs/32961304309) 六个 job。合并后生成的 `main@49c8aeac4` 又独立通过 [VulnCheck](https://github.com/pgsty/silo/actions/runs/32962256729) 和 [Go CI](https://github.com/pgsty/silo/actions/runs/32962256823) 六个 job。最慢的 PR cross-compile 用时 9 分 47 秒,合并后 cross-compile 用时 9 分 30 秒。 + +## 会不会引入新问题 {#derived-risks} + +### Shortcut 现在会产生集群扇出 {#fanout-cost} + +这是本修复最重要、也是刻意接受的代价。存在桶上的三类请求过去约等于一次本地分支判断,现在需要 `GetBucketInfo`。本机方向性 microbenchmark 得到: + +| 路径 | 观察到的量级 | +| --- | ---: | +| 修复前 shortcut | 约 0.55 μs,7 allocs | +| 修复后单盘 shortcut | 约 7.8–8.1 μs,45–47 allocs | +| 修复后 32 盘 shortcut | 约 70–81 μs,977 allocs | +| 32 盘正常列表 | 约 0.95 ms | + +这些数字只说明本地相对关系,不是 100+ 节点生产延迟预测。真实分布式环境还包含 peer 网络、quorum 与最慢节点尾延迟,可能比本机差得多。也正因为如此,检查绝不能扩展到正常列表路径。 + +风险集中在异常或探测式流量:如果某个错误配置的客户端高频轮询 `max-keys=0`、slash prefix 或不相交 marker,它会把原本廉价的请求放大成 peer/disk 工作。合并后值得从 S3 trace 或 metrics 观察这些输入的实际频率;有证据时再限流或优化。 + +### 降级集群会暴露更多真实错误 {#degraded-cluster} + +过去 shortcut 在 peer 离线或 bucket quorum 不足时也可能返回空 200,因为它根本不接触集群状态。修复后,这些请求可能返回 quorum、timeout 或 service error。 + +这属于更诚实的行为,不是可用性回归:服务器无法确认桶存在时,不应声称它是一个有效的空桶。但依赖“无论集群状态如何都空成功”的客户端会观察到变化。 + +### 创建与删除并发仍不具备线性化快照 {#race-window} + +`GetBucketInfo` 与返回空列表是两个动作。桶可能在检查后立即删除,或在缺桶结果形成后立即创建。本补丁没有也不应该为列表 shortcut 引入跨 bucket lifecycle 的事务。 + +这与其他先验证资源、再执行操作的 API 属于同一并发类别。修复保证请求不会在**没有任何存在性证据**时直接成功,不承诺一个跨节点、跨生命周期的线性化空列表快照。 + +### 依赖旧错误行为的客户端会看到 404 {#behavior-change} + +有客户端可能已经把缺桶的空 200 当作事实使用。修复会让它们进入错误分支。这是可见兼容变化,但它恢复的是 S3 文档契约与回归前行为;保留 bug 只会把迁移成本留给正确依赖 404 的用户。 + +### 两个相邻边缘仍不在本轮范围 {#remaining-edges} + +对抗评审记录了两个非阻塞的 P3 边界: + +1. 恢复 metacache continuation 时,`c.fileNotFound` 分支仍直接返回 `io.EOF`。一个陈旧或构造的 continuation token 遇上已经删除的桶,理论上仍可能得到空 200。把 `GetBucketInfo` 放到这里会影响正常分页 continuation,性能与错误语义需要单独设计。 +2. V1 与版本列表的某些 marker/prefix 组合会在 HTTP handler 参数校验中先返回 `NotImplemented`,尚未到对象层;V2 的 `start-after` 能进入对象层。本补丁修复的是存储 shortcut 掩盖缺桶,不重新定义畸形参数与资源错误的优先级。 + +它们都不是合并 blocker:第一项不在 #32 的普通初始列表复现中,第二项是既有 handler 行为。文档保留它们,是为了避免把“已覆盖三个 shortcut”误写成“所有可能的参数组合都已实现逐字节 AWS parity”。 + +## 讨论过的替代方案 {#alternatives} + +### 保持上游现状 {#keep-upstream} + +优点是零性能变化并减少与上游差异。缺点是继续违反 S3 契约、保留有版本边界的回归,并让 SILO 作为集成测试替身时给出错误信号。对一个范围清楚、测试充分的兼容修复,这个取舍不再合理。 + +### 恢复通用 `checkBucketExist` {#restore-global-check} + +它能一次覆盖所有路径,却把 peer fan-out 加回每个 Put、List 与 Multipart 操作,直接撤销 #18917 的大型集群优化。收益与成本不成比例,应明确拒绝。 + +### 只修 `Prefix="/"` {#slash-only} + +这会通过 issue 的单个复现,却留下 `max-keys=0` 与 marker-outside-prefix 两个同根缺陷。三个分支相邻、语义相同,用同一个 helper 收敛更简单,也更不容易再次遗漏。 + +### 增加 bucket-existence cache {#existence-cache} + +它可以让 shortcut 便宜,但需要定义创建、删除、复制、故障恢复和 TTL 期间的陈旧语义。当前没有遥测证明这些 shortcut 的流量足以支撑这种复杂度,因此不采用。 + +## 复杂度与成本收益 {#tradeoff} + +| 维度 | 评价 | 说明 | +| --- | --- | --- | +| 生产代码复杂度 | 低 | 三处调用点与一个 7 行 helper;无新状态、依赖或格式 | +| 测试复杂度 | 低到中 | 要同时覆盖 V1、V2、versions、三类 shortcut、控制组与 HTTP 映射 | +| 正常路径风险 | 很低 | `listMerged` 热路径没有新增检查 | +| Shortcut 运行时成本 | 明显上升 | 从本地 EOF 变成 cluster-wide `GetBucketInfo` | +| 兼容收益 | 高 | 恢复 404 `NoSuchBucket`、回归前行为和 S3 测试保真度 | +| 运维复杂度 | 低 | 无迁移、配置、feature flag、缓存或跨仓库依赖 | + +成本收益比总体良好,关键原因不是 `GetBucketInfo` 很便宜——它并不便宜——而是额外成本被严格限制在原本无法自然发现缺桶的三条 shortcut。用窄幅性能成本换取明确的协议正确性,比全局回退或长期保留错误行为都更合理。 + +## 接受决策与后续门槛 {#decision} + +最终决策是:**接受并合并补强后的 PR #37,不继续扩大生产修改范围。** + +实际执行顺序是: + +1. 用基于当前 `main`、带 DCO sign-off 的版本替换陈旧 fork head,同时保留 Jason Lin 的 co-author 归属; +2. 保留类型化错误判断、V1/V2/版本列表对象层覆盖与 HTTP 级 404 / `NoSuchBucket` 断言; +3. 更新 PR 描述,明确 shortcut 扇出成本与正常路径不变的边界; +4. 批准 fork workflow,并要求精确 head `e9c5340be` 的八项检查全部通过; +5. 针对该 head 提交正式批准评审; +6. 使用 expected-head guard 合并为 `49c8aeac4`,让 #32 自动关闭,再独立要求生成的 `main` Go CI 与 VulnCheck 全绿。 + +本轮不需要新增缓存、feature flag、更多抽象或修改 continuation-token 语义。高频 shortcut 流量与大型集群尾延迟仍是后续可观测项,不是继续凭假设扩代码的理由。 + +仓库集成已经完成。只有 tag、软件包、`docker.io/pgsty/minio` 镜像、部署与真实 S3 客户端验证分别完成后,才能宣称用户已经获得修复。 + +## 结论 {#conclusion} + +问题的本质不是“prefix 为 `/` 时少报了一个错误”,而是列表引擎把 `io.EOF` 同时当成了两件不同的事:存在桶的空结果,以及从未确认桶存在的提前结束。上游为大型集群移除通用存在性检查是合理优化,但 shortcut 绕过了“让真实存储操作自然报错”的前提。 + +选定修复恢复这条前提,只在三个绕过存储的出口调用已有 `GetBucketInfo`。它会让这些请求变贵,也会在降级集群上暴露真实错误;这两点都是明确成本。作为交换,SILO 恢复 S3 404 语义、升级兼容性与测试保真度,同时完整保留正常列表热路径的上游优化。 + +这个值得接受、范围受控的兼容修复现已合入绿色 `main`;release delivery 仍是独立门槛。