feat(backend): add object listing and serve ListObjects - #367
Merged
Conversation
ListObjects was defined in the protocol, called by talon-fuse and both client SDKs, and implemented by nothing. Unlike StatObject (milvus-io#318), which mapped onto the existing BackendStore::head, listing had no counterpart at all. Pagination is in the return type rather than hidden behind an internal loop. S3 caps a page at 1000 keys, so a method that drained pages itself would buffer an entire prefix in memory before returning; ListPage carries an opaque cursor and the caller decides how much to pull. The default trait implementation returns Error::Unsupported, a new variant, rather than an empty page. A caller building a namespace needs to tell "this backend cannot list" from "this prefix is empty", and conflating them turns a missing capability into a silently empty result -- which is exactly how this gap survived so long. Each backend needed genuinely different work. S3 uses ListObjectsV2 and only its continuation token when IsTruncated says so, because S3 may echo a token on the final page and trusting it loops forever. GCS returns JSON with sizes encoded as strings, since 64-bit values do not survive JavaScript's 53-bit integers; reading them as numbers silently yields nothing. Azure names the field Content-Length rather than Size and emits <NextMarker/> empty on the last page instead of omitting it, so an empty marker must not be treated as a cursor. Implementing S3 first required fixing SigV4. canonical_query passed the raw query string through with a comment noting that requests carried no parameters; listing is the first that does. AWS rejects a signature computed over an unsorted or unencoded query with a 403 that reads like a credentials failure, so this would have been a confusing first symptom. S3 and Azure both answer with XML. Rather than take an XML dependency for two known response shapes, this adds a small reader that handles exactly them -- explicitly not namespaces, attributes, or nesting -- with tests for a truncated document and for &lt; not being double-expanded. The worker serves listings from WorkerRuntime rather than from either data-plane handler, so the io_uring and Tokio paths share one implementation. Adding StatObject in milvus-io#318 required patching both and I missed one; putting the logic below them removes that trap. Both the object count and the number of backend round trips are capped, since the control protocol has no cursor to hand a client. The FUSE mount still lists with an empty prefix, which cannot succeed: there is no cross-backend root listing, because a worker cannot enumerate every bucket across S3, GCS, and Azure. That failure is now logged loudly and explained rather than dismissed as "mounting an empty namespace" -- a mount that looks healthy while showing nothing is how milvus-io#318 and milvus-io#332 both stayed hidden for months. Populating from a configured prefix is milvus-io#366. Verified end to end against a live cluster: a Java client lists through the coordinator proxy to a worker, and a prefixed listing returns only matching objects. The prefix assertion initially passed for the wrong reason -- the test origin ignored the parameter -- so the fixture was fixed to filter before the result could be trusted. Closes milvus-io#332 Refs milvus-io#310, milvus-io#312, milvus-io#318, milvus-io#366 Co-Authored-By: Claude <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ListObjectswas defined in the protocol, called bytalon-fuseand both client SDKs, and implemented by nothing. UnlikeStatObject(#318), which mapped onto the existingBackendStore::head, listing had no counterpart at all.Pagination is in the type
S3 caps a page at 1000 keys. A method that drained pages internally would buffer an entire prefix in memory before returning, so the caller decides how much to pull.
The default implementation returns a new
Error::Unsupportedrather than an empty page. A caller building a namespace needs to tell "this backend cannot list" from "this prefix is empty" — conflating them turns a missing capability into a silently empty result, which is exactly how this gap survived.Three backends, three different traps
NextContinuationTokenon the final page. Trusting it without checkingIsTruncatedloops forever.Content-Length, notSize.<NextMarker/>is emitted empty on the last page rather than omitted — treating an empty marker as a cursor loops forever. Also: a SAS token is itself a query string, so listing params merge with&, not?.A prerequisite fix: SigV4
canonical_querypassed the raw query string through, with a comment noting that requests carried no parameters. Listing is the first that does. AWS rejects a signature over an unsorted or unencoded query with a 403 that reads like a credentials failure — a confusing first symptom for anyone enabling S3 listing.Fixed with 5 tests covering sort order,
/escaping in prefixes, and the+/=characters that appear in continuation tokens.Why a hand-written XML reader
S3 and Azure both answer with XML. Rather than add an XML dependency for two known response shapes, this adds a ~50-line reader that handles exactly them — explicitly not namespaces, attributes, or nesting. Tested against a truncated document (must not loop) and
&lt;(must not double-expand to<).One implementation, both data planes
The worker serves listings from
WorkerRuntime, not from either data-plane handler. AddingStatObjectin #318 required patching bothtokio_connanduring_connand I missed one — the io_uring path is the default, so direct-to-worker calls failed while I assumed the proxy was at fault. Putting the logic below both handlers removes that trap.FUSE: the silent degradation, now loud
The mount lists with an empty prefix, which cannot succeed even now — there is no cross-backend root listing, because a worker cannot enumerate every bucket across S3, GCS, and Azure.
That failure was previously dismissed as
"mounting an empty namespace"with a warning. This is how both #318 and #332 stayed hidden for months: the endpoints were unimplemented and the mount still reported success. It is now logged with the reason and a pointer to #366, which adds a configured prefix.Kept out of this PR because it needs a new config surface across six layers plus a decision on lazy versus eager population.
Verification
Full chain: Java client → coordinator proxy → worker → Azure listing → XML → namespace paths.
The prefix assertion initially passed for the wrong reason. My test origin ignored the parameter, so filtering appeared to work while returning everything. I noticed the output was identical for both calls, fixed the fixture to actually filter, and only then trusted it.
cargo test --workspace --all-features --locked— 36 suites, incl. 20 new listing testscargo clippy --workspace --all-targets --all-features -- -D warnings— cleanRUSTDOCFLAGS="-D warnings" cargo doc— clean (caught a broken intra-doc link)typos— clean (caught1ueGcxin a fixture; changed the token rather than adding a global exception)just gen-conformance-vectors— no diff; this adds a capability, not a wire changeCloses #332
Refs #310, #312, #318, #366
🤖 Generated with Claude Code