241 Commits

Include renames

Author	SHA1	Message	Date
Chris LuandGitHub	5536d88fbb	azure: let the blob endpoint be configured (#10460) ... * azure: let the blob endpoint be configured The service url was always derived as <account>.blob.core.windows.net, which leaves out Azure Government, Azure China, and private endpoints. Name the blob service url instead and those accounts become reachable. The url has to be https, since the account key or the bearer token would otherwise travel in the clear. * azure: reject an endpoint that carries no hostname A url like https://:443/ has a host of ":443", so the emptiness check on Host let it through and the request only failed once it reached Azure. The hostname is what has to be there.	2026-07-27 16:41:13 -07:00
Chris LuandGitHub	3ae4e9c563	azure: authenticate with Entra ID instead of a storage account key (#10456) ... * azure: authenticate the blob sink with Entra ID Shared account keys have to be distributed and rotated everywhere a sink runs. Leaving account_key empty now falls back to the identity chain, so a workload identity or managed identity carries the authorization instead. * azure: authenticate remote storage with Entra ID The remote storage client demanded an account key and refused to start without one. Fall back to the identity chain when it is absent, and let azure.client_id pin a user-assigned identity. * azure: reject a malformed storage account name The account name is interpolated into the service URL, so a name carrying a "/", "?" or "@" moves the authority elsewhere and an authenticated request follows it. Hold callers to Azure's own naming rule instead. * azure: keep a leftover environment key off the identity path A configured client id asks for Entra ID, but AZURE_STORAGE_ACCESS_KEY still filled in the account key behind it. An old mounted secret would go on authenticating until it rotated, and the failure then blamed the key. * azure: say what the identity path reads from the environment A pinned client id alone is not enough for workload identity: the tenant and the projected token come from the environment, and missing them only surfaces later, when a token is first requested.	2026-07-27 14:12:14 -07:00
Chris LuandGitHub	19ce7c0b6f	consolidate the duplicated transient-error classifiers onto util.IsTransientError (#10429) ... * util: match transient error messages case-insensitively, expose the message form The same condition reaches different layers capitalized differently -- a volume server relays its idle timeout as "I/O timeout" inside a JSON string -- and the callers that grew their own substring lists all lower-case first. Also split out IsTransientErrorMessage for the paths that carry only the text, such as the per-file status strings in a batch delete response, and pick up "no route to host" and "network is unreachable" from the gRPC classifier. * filersink: classify transient network errors through util.IsTransientError The local list caught i/o timeout, connection reset, and broken pipe but not connection refused, no such host, unexpected EOF, the syscall errnos, or the gRPC and S3 overload codes. Keep only the bare io.EOF case, which is transient here -- a truncated chunk read -- but a clean stream end elsewhere. * filer deletion: reuse util.IsTransientErrorMessage for the network patterns Six of the sixteen patterns were already covered. Keep the ones specific to this pipeline -- read-only volumes, lookup failures, backpressure -- and note why context cancellation stays retryable here: it decides whether to requeue the deletion, not whether to retry a call. * wdclient: fold the shared classifier into the volume lookup retry check The string tail duplicated the shared list and missed the syscall errnos and net.Error timeouts. Keep "connection" and "timeout", which are broader than the shared classifier on purpose: a volume lookup is a cheap read-only call.	2026-07-24 11:08:18 -07:00
Chris LuandGitHub	c438c5ef94	filer.replicate: acknowledge notifications after the sink write, not on receipt (#10427) ... * filer.replicate: commit the kafka offset after replicating, not on receipt The partition consumer committed the offset as soon as it handed the message to the channel, so a sink write that failed was logged and the message was already behind the committed offset -- never redelivered, permanently missing from the sink. Commit in onSuccessFn instead, and hold the committed offset behind the oldest offset that failed to replicate so a restart redelivers from there. * filer.replicate: delete the sqs message after replicating, not on receipt ReceiveMessage deleted the message before the replicator had a chance to run, so a failed sink write dropped it for good. Move the delete into onSuccessFn and leave the message in the queue otherwise, letting the visibility timeout redeliver it.	2026-07-24 10:43:48 -07:00
Chris LuandGitHub	47b491b53c	mount: version open file handles by filer log position (#10403) ... * filer: stamp a log position on lookup and remote-cache responses Metadata events are logged after their store write and stamped with the filer clock. Reading that clock before serving an entry therefore gives a timestamp with a causal guarantee: every event at or below it is reflected in the returned entry. Clients caching filer state can use it as the entry's version to order the response against subscription events, including events committed before the call but delivered after it. * mount: version open file handles by filer log position A subscription event refreshing an open handle did a second lookup; a transient failure left the handle pinned to its old entry with no retry, since the subscription cursor had already advanced. The deeper problem is ordering: the handle is a cache written by three unordered channels — the async invalidation worker, local mutation acks, and open-time lookups — and overwriting cached state safely requires knowing which write is newer. The filer log timestamp is that order, and it now travels with every value instead of being derived out of band. Events carry it natively; lookup and remote-cache responses carry the log position stamped before the serving read; mutation acks carry it in their returned event; and the local store pairs each read with a version cursor advanced under the same lock as the store write. Each handle records the version its entry reflects, and one rule replaces the per-site reasoning: state at or below the handle's version is old news and must not be installed. The invalidation itself applies the event's own entry — no lookup, so no transient-failure window — except under a cached parent, where the store entry is the ordered merge of the event and anything applied since, and its version outranks the event's. An uncached parent receives no store writes, so a hit there would be a stale leftover masking the event. A vacated path (delete, rename away) keeps the last entry so unlinked-but-open reads still work. Directory builds version the completed directory at the listing snapshot and re-invalidate buffered events at that version, since their mid-build refresh ran against an incomplete store. The tests replay every race this replaces machinery for: rollback of a newer local flush (queued, cached, and read-through), stale leftovers under uncached parents, the build window including abort, handles opened after an event was queued, events landing mid-lookup, and undelivered events at remote-cache time across a filer failover. * filer: serialize the log position fence with mutations, stamp mutation acks The fence stamped before an unlocked entry read could precede state the read returned: a mutation writes storage first and assigns its event timestamp only at notify time, so a lookup racing that window handed the mount an entry newer than its fence, and the event's later delivery looked like fresh news — destroying dirty pages for a change the handle already had. The mutation handlers already hold an exclusive per-path lock across read, write, and notify; the lookup and remote-cache reads now take it shared around the stamp and the read, making the fence exact: everything at or below it is in the entry, nothing above it is. A no-change update returns success without an event, leaving the mount nothing to fence with even though the response confirms current state. Create and update acks now carry a log position stamped under the same lock, and the mount falls back to it whenever the ack has no event. Also regenerate the VT marshalers, which the earlier generation missed: without them a VT round-trip silently zeroed every log position. * java: sync filer.proto * mount: scope store versions to what they vouch for; atomic handle install The store's version cursor claimed too much. Advanced by local mutation acks and directory listing snapshots, it inflated the version of store reads for unrelated paths whose events the subscription still owed, and those events were then fenced out permanently. The cursor now tracks subscription progress only — events arrive in log order, so everything at or below it has been delivered for every path — and a completed listing records its snapshot as a per-directory floor instead of a global claim. Local acks never touch it: they version their own handle directly. Buffered build events advance the cursor at delivery, since their store write may never happen (abort) while their invalidation is already queued; their read-through directory pairs no store read with it, and rename fragments are applied first. Concurrent first opens raced: a slower opener's older lookup could overwrite the newer entry a faster opener had installed, while the monotonic version kept the newer timestamp — an old entry fenced at a new version, immune to every correcting event. Entry and version are now installed as one decision under the handle map lock, and an install that does not outrank the handle's version is dropped. The remote-cache commit also escaped the fence: it wrote storage and notified without the path lock, so a lookup's shared-locked fence and read could land between the two and hand out the cached state under-versioned. The commit now re-reads and writes under the exclusive path lock, and backs off entirely when the entry changed during the download — the concurrent writer supersedes the cached content. * mount: floors gate store applies; installs respect handle users; renames join the fence A directory floor certifies the listing state as of its snapshot, but a delayed event at or below the floor was still applied to the store — rolling the content back to pre-snapshot state while the floor kept claiming the snapshot version, so the correcting events were fenced out of every future read. Events are now gated against the affected directory's floor, each half of a rename independently. Fences are lower bounds: a listing or lookup can include a mutation whose event has not been delivered yet, and that event later passes every gate carrying state the handle already holds. Such a re-delivery now advances the version without destroying dirty pages or reinstalling the entry — invalidating local writes over a no-op was the real damage in every remaining under-fence window, including the unlocked listing snapshot, which no per-path lock can serialize. The concurrent-open install moved from the map lock to the handle lock every reader, writer, and invalidation synchronizes on, and rejects what cannot improve the handle: dirty state (local writes would be lost), unversioned lookup responses (they cannot outrank anything, and two zero-version opens must not overwrite each other), and anything not strictly newer. New handles are still fully initialized before the map exposes them. Renames committed metadata and emitted events with no path lock, so a lookup could read the renamed state under a fence preceding its events. Both rename handlers now hold the source and destination locks, ordered by path, across commit and notification; descendants of a renamed directory are not individually locked and rely on the no-op re-delivery handling above. * mount: per-entry store versions replace the cursor and directory floors The store's aggregate versions — a global subscription cursor and per-directory listing floors — were versions at coarser granularity than the values they described, and every over-claiming bug in this series traced to that gap: an aggregate vouching for state its source never saw. Each store entry now carries the filer log position of the write that produced it — the event that applied it, or the listing snapshot that inserted it, recorded in the store's key-value space under the same lock as the entry write. The store becomes what the handle already is: a last-writer-wins register with one rule, install only what outranks the current claim. The cursor, the floors, their advancement rules, the pairing ordering constraint, and the floor gating all collapse into that rule. Applies are gated per entry, each half of a rename independently; an unversioned local write clears the claim its content no longer proves; version records lingering after a bulk folder wipe cannot fence a recreate, since a claim only blocks while its entry exists. Listing inserts are stamped at build completion, before the buffered replay so newer replayed events override the stamp. Filer side, the fence dance every versioned read must perform is now a single choke point, fencedFindEntry, so a future read RPC gets the lock-serialized stamp by construction rather than by convention. * mount: judge no-op re-deliveries against an immutable base, not the live entry The equal-state skip compared the incoming event to the live handle entry, but local writes mutate the live entry — size, timestamps, chunks — so a delayed event re-delivering the base the handle was opened with no longer matched, and the installer destroyed the dirty pages and rolled the entry back over nothing new. The handle now keeps an immutable snapshot of the filer state it last installed or acknowledged, refreshed at every install and mutation ack (flush acks snapshot the request entry before the id mapping mutates it), and the no-op judgment runs against that base: an event carrying the base brings nothing, whatever the live entry has diverged to since. * mount: tombstones for versioned deletes, absence floors, copy enrollment Four gaps in the per-entry version protocol, all the same shape: a versioned fact with nothing carrying its version. A deletion is a fact about a path with no entry left to hold it — clearing the record let a delayed older event resurrect the deleted path, permanently, since the deletion's own redelivery is dedup-suppressed. Versioned deletes now leave a tombstone record that fences without an entry; renames tombstone their source the same way. Plain records still only block while their entry exists, so records lingering after a bulk folder wipe cannot fence a recreate. A completed listing proves absences as well as presences: a name it omitted was deleted as of the snapshot, and a delayed create below the snapshot re-creates it. The snapshot is kept per directory strictly as an absence fence, consulted only when a path has neither an entry nor a version record — present entries carry their own versions and never touch it, which is what separates this from the over-claiming floor it replaces. A rebuild against a pre-upgrade filer returns no snapshot; stamping now clears the children's records in that case, so a reinserted entry cannot reactivate the stale claim its previous incarnation left behind and reject valid events below it. Server-side copies installed the copied entry without enrolling in the base protocol, so the copy's own event differed from the stale pre-copy base and destroyed writes made to the destination after the copy. The install now refreshes the base and takes its version from the fenced readback. * mount: deletion facts outlive the cache's knowledge of the entry A versioned delete of a path the store held no entry for recorded nothing, so a delayed older event recreated the path — permanently, with the deletion's redelivery dedup-suppressed. The tombstone is now written whenever a versioned event vacates a path: the deletion is a fact about the path, not about what this cache happened to hold. For an absent entry, the listing's absence floor now speaks whatever older record remains: a tombstone at one position does not exhaust what is known about the path when a newer snapshot has confirmed the name still absent, and an event between the two was slipping past both. A committed copy whose readback failed installed a synthesized base with local timestamps; the copy's real event legitimately differs from it, and was read as foreign state — destroying writes made to the destination after the copy. The handle now marks that its own event is en route and adopts that event's state as the base without touching the live entry or the dirty pages; the adoption is one-shot, so a genuinely foreign event still invalidates. * mount: authoritative acks cancel pending event adoption; tombstones scoped and pruned The copy-event adoption flag could outlive its purpose: a flush after the failed readback installs a newer base and advances the version, the copy's own event is then version gated without consuming the flag, and the next genuinely foreign event was silently adopted — base advanced, live entry and dirty pages untouched — leaving the mount to later overwrite that remote change. Every local acknowledgment now installs its base through one helper that also cancels any pending adoption: the ack supersedes the mutation the adoption was waiting for. Tombstones were written for every versioned delete under the mount and survived directory eviction by design, growing LevelDB with historical deletions on delete-heavy mounts. They are now scoped to directories whose cached state the fence actually protects — an uncached parent never serves from the store nor applies the resurrecting insert — and a completed listing prunes the direct-child tombstones its absence floor supersedes, leaving only those above the snapshot. The store gains a key-prefix visitor for the sweep. * mount: acked saves install their value; trailer snapshots; direct-child prune range A version must never advance without its value. saveEntry stamped any open handle with the acknowledgment's version, but a handle opened while the save was in flight holds the pre-mutation entry — stamping it fenced out the events carrying the state it lacked, permanently, with the local apply performing no invalidation and the redelivery deduplicated. The acknowledged entry is now installed together with its version, through the same guarded install the racing-open path uses: under the handle lock, only when it outranks the handle, never over dirty local writes. Empty listings return no in-band snapshot — a snapshot-only response would be read as an entry by older consumers — so directories that end empty gained no absence floor and their tombstones were never pruned. The filer now sends the snapshot in the stream trailer, which older clients ignore, and the client reads it when no in-band snapshot arrived. Empty directories get real floors, their tombstones prune, and their buffered replays gain the snapshot filter instead of the replay-all fallback. Version records now encode the parent directory and name separated by a NUL, making a directory's direct children one contiguous key range: the tombstone prune scans exactly them under the cache lock, instead of walking every descendant record — the whole store, for root. * mount: fix dirty-page loss, uid/gid base, download race, copy adopt, leak; dedup Correctness fixes from the versioned-invalidation review: - A foreign delete/rename-away of a file held open with unflushed local writes destroyed the dirty pages unconditionally. A process may keep writing to an unlinked-but-open file and those writes were already acknowledged; preserve the pages when the handle is dirty. - downloadRemoteEntry stored the handle's base with filer-side uid/gid while every candidate it is later compared against is in local form, so under a non-identity UidGidMapper an unchanged re-delivery looked foreign and force-destroyed dirty pages. Map the base to local. - downloadRemoteEntry wrote the entry/base/version triple under only the handle's shared lock, so two concurrent reads of the same remote-only file could tear it. Serialize the install with a dedicated mutex (invalidation is already excluded by the exclusive handle lock). - A committed server-side copy whose readback failed adopted the FIRST event past the version gate as its base; a foreign write delivered first was silently swallowed. Adopt only an event whose content matches the synthesized base — the copy's own event — and install any other normally. - The deferred-create path relied on AcquireFileHandle installing the passed entry on a pre-existing handle, which the version rework dropped. Restore that install in the compat wrapper; the versioned open path keeps its gated install. Growth and hot-path cost: - Per-entry version records and tombstones leaked when a directory was evicted or read-through without a rebuild. An uncached directory gates its own inserts, so its records fence nothing; clear a directory's child version records when it is wiped for eviction. - FindEntry paid for the version KvGet on every lookup/getattr cache hit and threw it away. FindEntry now reads only the entry; the hot lookupEntry cache-hit path skips the version entirely. Cleanups: - Extract ackVersionTsNs over the shared response interface, replacing the metadata-event-else-log-ts snippet copy-pasted at four ack sites. - Extract acquireRenamePathLocks, replacing the verbatim sorted two-path lock fence in both rename handlers. * mount: no resurrection on foreign delete, version no-event acks, gate downloads, tighten copy adopt Follow-ups to the review patches: - Preserving dirty pages on a foreign delete let the next flush pass the isDeleted guard and CreateEntry, resurrecting the remotely-unlinked name. Mark the handle deleted in the vacate branch: the open fd can still read its buffered writes, but a flush no longer recreates the file. - A no-event acknowledgment (log fence only) synthesized a metadata event with TsNs 0, so the cache stored the entry unversioned and an older subscriber event rolled it back. Stamp the synthesized event with the ack's log position at all four ack sites. - downloadRemoteEntry serialized its install but did not check the version, so an older response arriving last overwrote the entry/base while the monotonic version kept the newer value, fencing corrections out. Install only when the response is at least as new as the handle. - sameEntryContent compared only size and chunks, so a foreign chmod with unchanged content was adopted as the copy's own event. Compare everything except server-assigned timestamps, so a metadata-only foreign change installs instead. * mount: trim comments to the non-obvious why The versioning work accumulated multi-line comment blocks restating what the code says. Keep the constraint a reader cannot derive — why a fence is exact, why a version must not advance without its value, why an uncached parent's records fence nothing — and drop the rest. * mount: distinguish rename from delete, tighten the download and adopt gates - A rename emits a nil old-path invalidation just like an unlink, so the vacate branch marked the handle deleted and later writes through the already-open descriptor were skipped instead of persisted. Carry the delete/rename distinction on the invalidation and mark only an actual delete. - The remote-download install accepted an unversioned response regardless of the handle's version, so during a rolling upgrade a delayed response could install stale content under a newer version. Require the response to be at least as new, with one exception: a handle still lacking local chunks takes the content anyway — it cannot read without it — but does not claim the response's log position. - Copy-event adoption returned without installing, so a foreign touch arriving before the copy's own event lost its timestamps. Content is unchanged either way, so the dirty pages stay valid; a clean handle now takes the entry, while a dirty one keeps its diverged version. * mount: one directory floor instead of a record per child; agree on TTL Review feedback: - Build completion wrote one KV record per direct child inside the cache write lock, so a large directory stalled every other cache operation for O(children) store writes. The directory's listing snapshot already covers every child it saw; make that floor the version for any child without a record of its own, and a child earns a record only when a later event touches it. One map write per build replaces the per-child writes, with the same fencing. - The presence probe read the store directly and so counted a TTL-expired entry as present, judging the path by a record describing content that has logically vanished. It now applies the same expiry the read path does, and an expired path falls back to its directory floor. - Preserve ErrNotFound identity when the commit-time re-read finds the object deleted, so callers still surface a 404. - Assert the rename-away source fence timestamp in the invalidation test. Also record the tombstone ceiling: distinct deleted names in a cached directory accumulate until it is rebuilt or evicted, which prunes everything at or below the new snapshot. * mount: pin the fence's clock domain instead of letting skew decide A log-position fence is stamped by one filer's clock under that filer's in-process lock, so comparing it to an event another filer logged is comparing two unrelated clocks. The two error directions are not equally costly: applying an event the fence already covered is a re-apply the base-equality check absorbs, while skipping one it does not cover leaves the handle holding exactly the state the event was meant to correct, with the subscription cursor already past it — the unhealable staleness this whole PR exists to remove. So refuse to guess. Fences now carry the signature of the filer that stamped them, and a handle records it alongside the position. An event is only fenced out when the filer that logged it is the one that stamped the fence — the logging filer appends its own signature, so its presence identifies the clock domain. Events from any other filer are applied. Positions taken from events keep comparing as before; the subscription already delivers those in order. The invalidation callback takes a struct now: it carries the path, entry, position, delete/rename distinction, and signatures, and was about to need a fifth positional parameter. * mount: follow a foreign rename; key page invalidation on content, not equality - A rename's old-path invalidation now carries the destination, and the handle follows the file there: an open fd tracks the inode, and leaving it on the old path made its next flush recreate that name instead of updating the renamed file. - Dirty pages overlay content, so only a content change invalidates them. Keying that on exact equality meant any timestamp-only event destroyed them, which the copy-adoption marker existed to paper over — a foreign touch could consume the marker and leave the copy's own event to drop the post-copy writes. Comparing content instead makes the marker unnecessary, so it is gone: a metadata-only event keeps the overlay, and a dirty handle keeps its diverged entry unless foreign content supersedes it. - A remote download response that is merely older is now refused even when the handle still lacks chunks; only an unversioned one is taken (and claims no position), since an older response's content predates what the handle reflects. - A refused or unversioned download no longer publishes to the metadata cache, where a zero-position event would clear the entry's version and let an older subscriber event roll the cache back. * mount: page invalidation keys on content alone; unversioned writes claim no position - sameEntryContent compared everything but timestamps, so a foreign chmod, chown, or xattr change counted as a content change and destroyed the dirty-page overlay. It was strict only to serve the copy-adoption marker, which is gone; its one caller now asks the question it actually needs — did the bytes change — so metadata-only events leave the overlay alone. - A rename over an existing file destroys that file, but its open handle was left live and still pointed at the name the renamed source now occupies, so its flush could overwrite it. MovePath already reports the displaced inode; mark that handle deleted. - An acknowledgment was refused whenever its position was numerically lower, even when a different filer stamped the fence it lost to. Two known, differing signatures mean unrelated clocks, so the comparison no longer applies there; unknown signatures still compare as before. - A local write with no log position behind it now records that explicitly instead of deleting its version record. Absence means the directory listing covers the path, which is why the snapshot floor applies; local content the listing never saw must not inherit it, or the events that would correct it are fenced out. * mount: widen the existing lookup functions instead of forking WithVersion twins The versioning work grew a parallel function for every accessor that needed to return a log position — lookupEntryWithVersion beside lookupEntry, maybeLoadEntryWithVersion beside maybeLoadEntry, FindEntryWithVersion beside FindEntry, AcquireFileHandleWithVersion beside AcquireFileHandle, advanceEntryVersion beside advanceEntryVersionTsNs, plus a getPbEntryWithVersion wrapper and an InsertListedEntriesForTest hook. Two names for one operation is two places to keep in step, and the split let callers pick the one that happened to compile. Each pair is now the single original name carrying the position, with callers that do not want it discarding it. filer_pb.GetEntry returns the fence its response already carried rather than a mount-side wrapper re-issuing the lookup, and InsertEntry takes the position its content reflects rather than a test-only twin that inserted without one. The one behavioural knot the merge exposed: AcquireFileHandle had been installing the entry on a pre-existing handle only in its unversioned form, which conflated 'the caller is authoritative' with 'the lookup had no version'. Deferred create is the only caller that means the former, so it now installs explicitly and the map function just acquires.	2026-07-23 17:44:02 -07:00
3 people	c17aebeecf	fix(filer.backup): prevent silent backup data loss on transient not-found (#10295) ... * fix(filer.backup): stop silently dropping events on transient not-found Under a write burst, filer.backup could consume a metadata event (advancing the persisted offset) without replicating the file, with no error logged: 1. filersink CreateEntry/UpdateEntry swallowed replicateChunks errors (glog.Warningf + return nil), so the offset advanced past entries that were never written. 2. The manifest-chunk branch of replicateChunks resolved via LookupFileId with no retry, unlike the data-chunk branch — transient lookup races dropped exactly the large manifest-backed files while small inline-content siblings landed. 3. isIgnorable404 matched "LookupFileId" / "volume id ... not found", misclassifying those races as genuine source 404s at the backup layer. Fix: on a replicateChunks failure the filer sink now skips only when the live source has moved past the replayed version (deleted or strictly-newer mtime) — lossless, a later event carries the current content — and propagates otherwise so the event is retried. The manifest resolve retries transient errors like the data-chunk path. isIgnorable404 is narrowed to genuine 404s; non-filer sinks and the initial-snapshot walk, which relied on the broad match as their only lossless-skip valve, now make the same live-source decision (filersink.SourceSupersedes) instead of retrying forever on a permanently gone volume. Tests cover propagation of unconfirmed lookup failures, the narrowed 404 classification, and the supersession guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer.backup): derive supersession path with directory fallback eventSourceSuperseded built the source path from NewParentPath alone. Legacy metadata events (persisted by older filers) carry an empty NewParentPath, so the probe looked up "/<name>", read the miss as "source gone", and skipped a live file on a transient lookup error — the silent drop this change is meant to eliminate. Derive the path via MetadataEventTargetFullPath (the same directory fallback genProcessFunction uses) and cover both event shapes with TestEventSupersessionProbe_PathDerivation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filersink): retry manifest resolve only for transient errors, bounded when unverifiable The manifest-resolve retry stopped only when hasSourceNewerVersion proved the source moved past the replayed version, which wedged the sink in two cases: incremental sinks use dated target keys that cannot map back to a source path (supersession never provable), and permanent resolve errors (corrupt manifest data, bad file ids) fail forever while the source entry stays live. Gate the retry instead: keep retrying only transient errors (volume-lookup races, network interruptions), stop after a few attempts when supersession cannot be checked, and propagate everything else immediately so the configured metadata error policy applies (-disableErrorRetry included). Propagation is lossless: filer.backup's fallback decides with the event's real source key, and both filer.backup and filer.sync re-deliver the event (RetryForeverOnError) without advancing the offset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer.backup): make error classifiers nil-safe isIgnorable404, isSourceLookupError, and isTransientResolveError called err.Error() without a nil guard. All current call sites pass a non-nil error, but the guard is free and matches isRetryableNetworkError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-10 01:47:46 -07:00
4 people	bdcc3154ed	refactor: centralize genUploadUrl in UploadOption (#10164) ... * refactor: centralize genUploadUrl in UploadOption Replace inline genFileUrlFn closures with operation.GenUploadUrl field: - Add GenUploadUrl func(host, fileId) string to UploadOption struct - Add GenUploadUrlProxy(filerAddress string) utility function - Remove genFileUrlFn parameter from UploadWithRetry signature - Update all callers: mount, gateway, mq, filer_copy, filer_sync This matches the weed mount -filerProxy pattern exactly, factorizing the URL generation logic across all consumers. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * docker release: run all platform jobs in one wave, cache rocksdb compile Drop max-parallel so the 13 per-platform builds run together instead of two waves of 8 (rocksdb was queuing behind the cap and starting ~8 min late). Keep cache-to mode=max for rocksdb: its RocksDB static_lib compile is sha-independent, so it caches across releases and stops being the ~16-min long-pole that gates the merge fan-in. go-build variants stay mode=min. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * refactor: centralize genUploadUrl in UploadOption Replace inline genFileUrlFn closures with operation.GenUploadUrl field: - Add GenUploadUrl func(host, fileId) string to UploadOption struct - Add GenUploadUrlProxy(filerAddress string) utility function - Remove genFileUrlFn parameter from UploadWithRetry signature - Update all callers: mount, gateway, mq, filer_copy, filer_sync This matches the weed mount -filerProxy pattern exactly, factorizing the URL generation logic across all consumers. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * Remove accidental ROCmFPX submodule reference * gofmt chunk upload option block * Preserve broker cipher and re-read proxy filer per upload attempt Chunk uploads must keep the configured Cipher, and both the mount and broker current filer can change on failover, so build the proxy upload URL inside the closure instead of capturing the address once. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-06-30 20:45:43 -07:00
3 people	a11d81b21f	fix(filer.backup): repair chunk-incomplete and stale destination entries (#10082) ... * fix(filer.backup): repair chunk-incomplete and stale destination entries filer.backup left destinations diverged while metadata advanced — chunk-incomplete (missing/gapped ranges at full attr.file_size) or holding a chunk superseded by a missed overwrite. The skip/repair decision keyed on filer.FileSize (the attr), which a truncated entry keeps full, so it never repaired. Decide from actual chunk state instead: - coversReference: range-by-range containment (scalar byte totals and attr FileSize/Md5 cannot see chunk-level gaps). - hasStaleBackupChunk: a backup-written chunk (SourceFileId) the source no longer lists; ignores out-of-band (rsync/direct) chunks. - destinationMatchesReference: allocation-free positional fast path gating the above so they run only on divergence (the in-sync path stays cheap). - A strictly-newer destination is never repaired, so an older out-of-order replay cannot roll it back. The stale signal is deferred at equal mtime (same-second versions cannot be ordered; reliable S3 sub-second ordering is a separate fix). Tests in filer_sink_test.go. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * filer.backup: verify chunk range in destinationMatchesReference fast path The allocation-free fast path matched a destination chunk to its reference by SourceFileId alone. That is correct today only because replicateOneChunk copies the source chunk's Offset/Size verbatim, so SourceFileId identity implies an identical range — an invariant that lives in another file with no guard linking the two. If replication ever re-chunks (split/coalesce), a chunk with the right SourceFileId but a different range would fast-path as a full match and skip a needed repair (a false positive in the very class this change otherwise prevents). Compare Offset/Size alongside SourceFileId so the fast path is self-contained and can only be more conservative (a range mismatch falls through to the precise coversReference/hasStaleBackupChunk checks). Add tests for a shifted offset and a larger size at matching identity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 14:23:38 -07:00
Chris LuandGitHub	e12052ee6b	fix(filer.sync): replicate a rename as an atomic move, not a no-op update (#9895) ... * fix(filer.sync): replicate a rename as create-then-delete, not an in-place update A rename arrives as a single metadata event carrying both the old and new entry. The filer sink was routed to UpdateEntry, which looks up the old path but issues the update against the new parent without changing the name — and the filer UpdateEntry RPC cannot move an entry. So the rename was dropped: the old path lingered and the new path never appeared (same-dir renames rewrote the old name in place). Route a real move (the sink path changed) through CreateEntry(new) then DeleteEntry(old) in both the replicator and the filer.sync/backup driver, the way the other sinks already handle it; reach UpdateEntry only for true in-place updates. Create before delete so a crash between the two leaves the entry visible rather than lost. * fix(filer.sync): derive the rename delete key like the create key, guard the watched root The rename delete leg rebuilt the old key with a raw util.Join, bypassing the sink-side key normalization the create leg gets from buildKey — so a rename could create the new entry and then fail to delete the old one under a transformed key. Build the old key through buildKey too, and skip the delete when the moved entry is the watched root itself (where the old key would resolve to the target root and recursively delete the whole sink tree). * test(filer.sync): cover the in-place update delete-then-create fallback order The recording sinks always reported foundExisting, so the fallback that an in-place update takes when the entry is missing on the sink was never run. Make it configurable and assert the fallback deletes before it recreates the same key, in both the replicator and the filer.sync drivers. * feat(filer.sync): move filer-sink renames natively via AtomicRenameEntry create-then-delete is unsafe for the filer sink: CreateEntry returns nil without creating on a transient chunk-copy error, so the paired delete could remove the only valid destination copy; a directory rename also deleted the old subtree before descendants were recreated, and left old chunks behind. Add an optional EntryMover sink capability and implement it on the filer sink via AtomicRenameEntry — one atomic, metadata-only move that relocates a whole subtree in a single transaction. Renames prefer it; sinks without a native move keep create-then-delete. When the old path is already gone (a descendant the parent rename moved, or one never replicated) MoveEntry creates the new path instead, re-checking existence with a lookup so a rolled-back move that left the old entry intact is retried rather than mistaken for gone. * docs(filer.sync): note entryMissing's gRPC not-found string fallback is deliberate	2026-06-09 12:54:28 -07:00
Chris LuandGitHub	7b07d8177a	fix(filer.sync): scope filesystem key sanitization to the local sink (#9894) ... * fix(filer.sync): scope filesystem key sanitization to the local sink destKey ran every sink key through escapeKey, whose Windows build strips colons. Colons are illegal in NTFS filenames so the local sink needs that, but s3/filer/azure/gcs/b2 accept them as ordinary key bytes — stripping them silently diverged the destination key (a source a:b replicated as ab). Move the sanitization into the local sink behind a Windows build tag, applied at every entry point so the previously-unescaped in-place-update paths stay consistent. Non-local sinks now keep the raw key; non-Windows builds are unchanged; a leading drive-letter colon is preserved. * test(filer.sync): cover incremental destKey and localsink update/delete sanitization Lock the colon-preserving behavior for the incremental destKey branch, and extend the Windows local-sink test to assert UpdateEntry and DeleteEntry also sanitize the key, not just CreateEntry.	2026-06-09 10:18:49 -07:00
3 people	202517c02a	fix(filer.backup): skip replay events whose source chunk was superseded or deleted (#9886) ... * fix(filer.backup): skip replay events whose chunk no longer exists on the source "Source" is the filer we replicate FROM (e.g. green in a green->blue backup). Replaying the metadata log from a checkpoint can hit an event whose chunk was since overwritten/deleted and garbage-collected on the source volume. Fetching it returns 0 bytes (a permanent size mismatch), which the sink propagated to the subscription — so the same offset retried forever and replication stalled. Skip the event only when proven stale; otherwise keep refusing so genuine loss of a live file still halts loudly: - onCorruptChunk centralizes the three errChunkSizeMismatch sites. - getEntryMtimeNs compares mtime at nanosecond precision so same-second rewrites (git's config.lock dance) are ordered correctly. - sourceSupersedes re-reads the entry's current state on the source: gone (ErrNotFound) or a strictly-newer mtime than the replayed version -> skip; any other lookup error keeps the entry. Skipping is lossless: events are full-entry snapshots, so a later event re-carries the current chunks and a delete event reconciles a removed file. * test(filer.backup): cover the superseded-chunk skip decision - TestSourceSupersedes: not-found (sentinel / wrapped / gRPC string) and nil entry -> skip; network error -> keep; source newer -> skip; same/older -> keep. - TestGetEntryMtimeNs: nanosecond precision, same-second ordering, nil safety. - TestOnCorruptChunkRefusesWhenSupersessionUnconfirmed: never skip silently when supersession cannot be confirmed. * fix(filer.backup): don't infer supersession for incremental sinks In incremental mode the sink key carries a date prefix (sinkDir/YYYY-MM-DD/relPath) that cannot be reversed to a real source path, so a source lookup would always be ErrNotFound and wrongly classify a live entry as deleted — skipping it. Make targetPathToSourcePath report "unmappable" in incremental mode; hasSourceNewerVersion already declines to skip when the source path cannot be mapped. Found in code review. Non-incremental sinks (filer.backup green->blue) are unaffected. * refactor(filer.backup): name the mtime param sourceMtimeNs; note ns overflow bound - Rename the threaded sourceMtime parameter to sourceMtimeNs across the internal replicate/fetch helpers so the unit is explicit (it only feeds hasSourceNewerVersion, which compares in nanoseconds). - Document that getEntryMtimeNs's int64 ns arithmetic is safe until ~year 2262. No behavior change. * fix(filer.backup): order same-second versions in the CreateEntry skip and update gates The CreateEntry already-replicated short-circuit and chooseUpdateAction still compared second-grained mtime, so a newer version written within the same second could be skipped as already-replicated or overwritten by an older same-second replay. Route both through getEntryMtimeNs, matching the precision the chunk-replication path already uses. * test(filer.backup): cover same-second update-action ordering * docs(filer.backup): trim verbose comments to terse why * fix(filer.backup): check supersession against the rename's new path For a rename the filer sink updates in place (the delete+create branch is skipped for sink name "filer"), so the corrupt-chunk supersession check queried the pre-rename key. Its source-side ErrNotFound was read as "superseded", silently advancing the checkpoint without applying the rename. Map the incoming entry's new path (newParentPath/newEntry.Name) for both update branches. * fix(filer.backup): detect a deleted source even when the replayed mtime is epoch hasSourceNewerVersion returned early when sourceMtimeNs <= 0, skipping the source lookup, so a deleted entry with mtime 0 (a valid epoch timestamp) never got the gone verdict and wedged on permanent retries. Always look up; gate only the newer-mtime comparison on a valid replayed mtime. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-06-09 08:53:29 -07:00
Chris LuandGitHub	2386fa550a	grpc: don't tear down the shared master connection on a caller's own timeout (#9775) ... A Canceled/DeadlineExceeded from the caller's per-request context was treated like a dead channel: it closed the shared cached ClientConn and cancelled every other in-flight RPC on it with "the client connection is closing". Under a burst of concurrent chunk assigns (e.g. a large S3 multipart upload) one slow assign hitting its 10s attempt timeout could poison the connection for all the rest, cascading into a flood of 500s. Thread the caller's context into shouldInvalidateConnection and only invalidate on Canceled/DeadlineExceeded while that context is still live, which isolates the genuine stale-channel signal (a peer restart behind a k8s Service VIP). To carry the context, add a ctx parameter to the existing WithGrpcClient, WithMasterClient, and WithMasterServerClient; the master assign and volume-lookup paths pass their per-attempt context and every other caller passes context.Background().	2026-06-01 15:11:02 -07:00
Chris LuandGitHub	57797c9b38	filer.sync: repair a destination shorter than the source (#9778) ... When the destination's stored mtime is newer than the incoming source version, UpdateEntry skips the update (last-writer-wins). A copy left truncated by an earlier failed replication trips this: the source kept the file's original mtime while the partial copy was written recently, so it looks "newer" and is never corrected. When the destination is strictly shorter than the source, re-replicate the full source content and replace the chunk list instead of skipping. Same shorter-than-source bypass for CreateEntry.	2026-06-01 13:04:23 -07:00
Chris LuandGitHub	2e3fabbf24	filer.sync: back off on transient upload errors (#9777) ... A destination volume server that hits its idle deadline while reading a large upload body under load returns 400 "read tcp ...: i/o timeout". fetchAndWrite retried that on the flat ~1s RetryUntil backoff, hammering the already-overloaded destination. Route i/o timeout, connection reset, broken pipe and net.Error timeouts through the same escalating 10s-2min backoff already used for EOF so it can recover.	2026-06-01 12:18:17 -07:00
Chris LuandGitHub	9cb9699e9d	fix(replication/s3sink): forward entry mime as ContentType (#9710) ... * fix(replication/s3sink): forward entry.Attributes.Mime as ContentType Same gap as the remote_storage S3 client: filer.replicate uploads via s3manager.Uploader without populating ContentType, so replicated objects on S3-compatible backends (e.g. Backblaze B2) store binary/octet-stream and browsers refuse to render HTML, CSS, etc. Pass entry.Attributes.Mime through to UploadInput.ContentType, leaving the header unset when no Mime is recorded so the remote keeps its own default. * fix(replication/s3sink): nil-guard entry.Attributes when reading Mime * Revert "fix(replication/s3sink): nil-guard entry.Attributes when reading Mime" This reverts commit 08c3698e44. The function already dereferences entry.Attributes.Mtime and entry.Attributes.Md5 unconditionally on the same path, so a nil guard on Mime alone is inconsistent and provides no real safety.	2026-05-27 12:20:51 -07:00
Chris LuandGitHub	cd68313929	fix(filer.sync): resolve manifest chunks against source filer (#9705) ... * fix(filer.sync): resolve manifest chunks against source filer `UpdateEntry` was passing `filer.LookupFn(fs)` — the sink filer client — into `compareChunks`. But `oldEntry`/`newEntry` chunks come from the source cluster, so manifest resolution must hit the source filer's volume servers. With two clusters that have overlapping volume IDs (common once they grow past a few hundred volumes), the sink lookup returns its own volume's URLs and the fetch 404s on the source's fileKey: compare chunks error: fail to read manifest 631,0babe...: 404 Not Found The 404 aborts the diff, the manifest chunk never gets replicated, and the target ends up with whatever flat chunks happened to land from earlier partial syncs — visible as `SIZE_MISMATCH` in filer.sync.verify on files large enough to use chunk manifests (~150 GB+ in practice). Only the manifest path was wrong; flat-chunk reads in `fetchAndWrite` already use `fs.filerSource.ReadPart`. * trim comment * test(filer.sync): regression test for source-filer manifest lookup Two recording filer gRPC servers stand in for source and sink. Driving UpdateEntry with a manifest chunk and observing which one receives LookupVolume proves compareChunks routes source-side lookups through fs.filerSource, not fs. Reverting the fix flips the call onto the sink filer and fails the assertion. * drop test	2026-05-27 10:23:29 -07:00
4 people	675020b342	fix(filer.sync): validate chunk size in FilerSink to prevent 0-byte propagation (#9701) ... * fix(filer.sync): validate chunk size in FilerSink to prevent 0-byte propagation FilerSink.fetchAndWrite previously trusted the source response and the upload result blindly: a 200 OK / Content-Length: 0 reply from a broken source volume was happily uploaded as a 0-byte needle to the destination, and the destination filer metadata was then written with the source chunk size. The result was permanent silent corruption -- ls shows the file at its original size but reads fail with EIO. Add two cheap defenses inside fetchAndWrite: 1. After assembling fullData, compare its length against sourceChunk.Size. 2. After a successful upload, compare uploadResult.Size against sourceChunk.Size. Both checks wrap a new sentinel errChunkSizeMismatch that the retry callback recognizes and refuses to retry -- needle.size=0 on disk is a persistent state, not a transient network error, so the sync should stop loudly on the affected entry instead of looping or, worse, silently propagating it. Tests: * TestValidateReplicatedChunkSize -- table-driven coverage of healthy, legitimately empty, zero-byte read, short read, and truncated upload cases. * TestFetchAndWriteRejectsZeroByteSource -- end-to-end: an httptest source that returns 200 OK with an empty body must cause fetchAndWrite to return errChunkSizeMismatch after exactly one source hit (fail fast, no retry storm). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * filer.sync: bubble size-mismatch past CreateEntry/UpdateEntry Three follow-ups on the chunk-size validation: - Use %w in replicateOneChunk so the errChunkSizeMismatch sentinel survives the wrap and reaches errors.Is callers up the stack. - In FilerSink.CreateEntry/UpdateEntry, surface errChunkSizeMismatch instead of warning-and-nil. Other errors (deleted source chunk, transient network) keep the existing swallow so a hiccup doesn't stall the stream. - Drop validateReplicatedUploadSize: uploadResult.Size is set client-side from the same len(fullData) we already validated pre-upload, so the second check can't fail. Test: scope the RetryWaitTime override to the one test that needs it, add a regression that locks in the errors.Is chain through replicateChunks. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-05-26 20:47:53 -07:00
Chris LuandGitHub	eae8f33db5	fix(filersink): return lock-free snapshot from ActiveTransfers (#9604) ... ChunkTransferStatus embeds a sync.RWMutex, so returning a slice of it made callers copy the lock when ranging. Split out a copyable ChunkTransferSnapshot holding the data fields and return that instead.	2026-05-21 02:40:04 -07:00
Chris Lu	e648c76bcf	go fmt	2026-04-10 17:31:14 -07:00
Chris LuandGitHub	2919bb27e5	fix(sync): use per-cluster TLS for HTTP volume connections in filer.sync (#8974) ... * fix(sync): use per-cluster TLS for HTTP volume connections in filer.sync (#8965) When filer.sync runs with -a.security and -b.security flags, only gRPC connections received per-cluster TLS configuration. HTTP clients for volume server reads and uploads used a global singleton with the default security.toml, causing TLS verification failures when clusters use different self-signed certificates. Load per-cluster HTTPS client config from the security files and pass dedicated HTTP clients to FilerSource (for downloads) and FilerSink (for uploads) so each direction uses the correct cluster's certificates. * fix(sync): address review feedback for per-cluster HTTP TLS - Add insecure_skip_verify support to NewHttpClientWithTLS and read it from per-cluster security config via https.client.insecure_skip_verify - Error on partial mTLS config (cert without key or vice versa) - Add nil-check for client parameter in DownloadFileWithClient - Document SetUploader as init-only (same pattern as SetChunkConcurrency)	2026-04-07 14:11:44 -07:00
3 people	3cea900241	fix: replication sinks upload ciphertext for SSE-encrypted objects (#8931) ... * fix: decrypt SSE-encrypted objects in S3 replication sink * fix: add SSE decryption support to GCS, Azure, B2, Local sinks * fix: return error instead of warning for SSE-C objects during replication * fix: close readers after upload to prevent resource leaks * fix: return error for unknown SSE types instead of passing through ciphertext * refactor(repl_util): extract CloseReader/CloseMaybeDecryptedReader helpers The io.Closer close-on-error and defer-close pattern was duplicated in copyWithDecryption and the S3 sink. Extract exported helpers to keep a single implementation and prevent future divergence. * fix(repl_util): warn on mixed SSE types across chunks in detectSSEType detectSSEType previously returned the SSE type of the first encrypted chunk without inspecting the rest. If an entry somehow has chunks with different SSE types, only the first type's decryption would be applied. Now scans all chunks and logs a warning on mismatch. * fix(repl_util): decrypt inline SSE objects during replication Small SSE-encrypted objects stored in entry.Content were being copied as ciphertext because: 1. detectSSEType only checked chunk metadata, but inline objects have no chunks — now falls back to checking entry.Extended for SSE keys 2. Non-S3 sinks short-circuited on len(entry.Content)>0, bypassing the decryption path — now call MaybeDecryptContent before writing Adds MaybeDecryptContent helper for decrypting inline byte content. * fix(repl_util): add KMS initialization for replication SSE decryption SSE-KMS decryption was not wired up for filer.backup — the only initialization was for SSE-S3 key manager. CreateSSEKMSDecryptedReader requires a global KMS provider which is only loaded by the S3 API auth-config path. Add InitializeSSEForReplication helper that initializes both SSE-S3 (from filer KEK) and SSE-KMS (from Viper config [kms] section / WEED_KMS_* env vars). Replace the SSE-S3-only init in filer_backup.go. * fix(replicator): initialize SSE decryption for filer.replicate The SSE decryption setup was only added to filer_backup.go, but the notification-based replicator (filer.replicate) uses the same sinks and was missing the required initialization. Add SSE init in NewReplicator so filer.replicate can decrypt SSE objects. * refactor(repl_util): fold entry param into CopyFromChunkViews Remove the CopyFromChunkViewsWithEntry wrapper and add the entry parameter directly to CopyFromChunkViews, since all callers already pass it. * fix(repl_util): guard SSE init with sync.Once, error on mixed SSE types InitializeWithFiler overwrites the global superKey on every call. Wrap InitializeSSEForReplication with sync.Once so repeated calls (e.g. from NewReplicator) are safe. detectSSEType now returns an error instead of logging a warning when chunks have inconsistent SSE types, so replication aborts rather than silently applying the wrong decryption to some chunks. * fix(repl_util): allow SSE init retry, detect conflicting metadata, add tests - Replace sync.Once with mutex+bool so transient failures (e.g. filer unreachable) don't permanently prevent initialization. Only successful init flips the flag; failed attempts allow retries. - Remove v.IsSet("kms") guard that prevented env-only KMS configs (WEED_KMS_*) from being detected. Always attempt KMS loading and let LoadConfigurations handle "no config found". - detectSSEType now checks for conflicting extended metadata keys (e.g. both SeaweedFSSSES3Key and SeaweedFSSSEKMSKey present) and returns an error instead of silently picking the first match. - Add table-driven tests for detectSSEType, MaybeDecryptReader, and MaybeDecryptContent covering plaintext, uniform SSE, mixed chunks, inline SSE via extended metadata, conflicting metadata, and SSE-C. * test(repl_util): add SSE-S3 and SSE-KMS integration tests Add round-trip encryption/decryption tests: - SSE-S3: encrypt with CreateSSES3EncryptedReader, decrypt with CreateSSES3DecryptedReader, verify plaintext matches - SSE-KMS: encrypt with AES-CTR, wire a mock KMSProvider via SetGlobalKMSProvider, build serialized KMS metadata, verify MaybeDecryptReader and MaybeDecryptContent produce correct plaintext Fix existing tests to check io.ReadAll errors. * test(repl_util): exercise full SSE-S3 path through MaybeDecryptReader Replace direct CreateSSES3DecryptedReader calls with end-to-end tests that go through MaybeDecryptReader → decryptSSES3 → DeserializeSSES3Metadata → GetSSES3IV → CreateSSES3DecryptedReader. Uses WEED_S3_SSE_KEK env var + a mock filer client to initialize the global key manager with a test KEK, then SerializeSSES3Metadata to build proper envelope-encrypted metadata. Cleanup restores the key manager state. * fix(localsink): write to temp file to prevent truncated replicas The local sink truncated the destination file before writing content. If decryption or chunk copy failed, the file was left empty/truncated, destroying the previous replica. Write to a temp file in the same directory and atomically rename on success. On any error the temp file is cleaned up and the existing replica is untouched. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-04-06 00:32:27 -07:00
Chris LuandGitHub	72eb93919c	fix(gcssink): prevent empty object finalization on write failure (#8933) ... * fix(gcssink): prevent empty object finalization on write failure The GCS writer was created unconditionally with defer wc.Close(), which finalizes the upload even when content decryption or copy fails. This silently overwrites valid objects with empty data. Remove the unconditional defer, explicitly close on success to propagate errors, and delete the object on write failure. * fix(gcssink): use context cancellation instead of obj.Delete on failure obj.Delete() after a failed write would delete the existing object at that key, causing data loss on updates. Use a cancelable context instead — cancelling before Close() aborts the GCS upload without touching any pre-existing object.	2026-04-05 16:07:49 -07:00
Chris LuandGitHub	4fd974b16b	fix(azuresink): delete freshly created blob on write failure (#8934) ... * fix(azuresink): delete freshly created blob on write failure appendBlobClient.Create() runs before content decryption and copy. If MaybeDecryptContent or CopyFromChunkViews fails, an empty blob is left behind, silently replacing any previous valid data. Add cleanup that deletes the blob on content write errors when we were the ones who created it. * fix(azuresink): track recreated blobs for cleanup on write failure handleExistingBlob deletes and recreates the blob when overwrite is needed, but freshlyCreated was only set on the initial Create success path. Set freshlyCreated = needsWrite after handleExistingBlob so recreated blobs are also cleaned up on content write failure.	2026-04-05 16:07:34 -07:00
Mmx233andGitHub	69cd5fa37b	fix: S3 sink puts all entry.Extended into Tagging header instead of only object tags (#8930) ... * test: add failing tests for S3 sink buildTaggingString * fix: S3 sink should only put object tags into Tagging header * fix: avoid sending empty x-amz-tagging header	2026-04-05 12:16:04 -07:00
Jaehoon KimandGitHub	d3cea714d0	fix(filer.backup): local sink readonly permission (#8907)	2026-04-03 05:36:56 -07:00
Chris LuandGitHub	9552e80b58	filer.sync: show active chunk transfers when sync progress stalls (#8889) ... * filer.sync: show active chunk transfers when sync progress stalls When the sync watermark is not advancing, print each in-progress chunk transfer with its file path, bytes received so far, and current status (downloading, uploading, or waiting with backoff duration). This helps diagnose which files are blocking progress during replication. Closes #8542 * filer.sync: include last error in stall diagnostics * filer.sync: fix data races in ChunkTransferStatus Add sync.RWMutex to ChunkTransferStatus and lock around all field mutations in fetchAndWrite. ActiveTransfers now returns value copies under RLock so callers get immutable snapshots.	2026-04-02 13:08:24 -07:00
Chris LuandGitHub	8572aae403	filer.sync: support per-cluster mTLS with -a.security and -b.security (#8872) ... * filer.sync: support per-cluster mTLS with -a.security and -b.security flags When syncing between two clusters that use different certificate authorities, a single security.toml cannot authenticate to both. Add -a.security and -b.security flags so each filer can use its own security.toml for TLS. Closes #8481 * security: fatal on failure to read explicitly provided security config When -a.security or -b.security is specified, falling back to insecure credentials on read error would silently bypass mTLS. Fatal instead. * fix(filer.sync): use source filer's fromTsMs flag in initOffsetFromTsMs A→B was using bFromTsMs and B→A was using aFromTsMs — these were swapped. Each path should seed the target's offset with the source filer's starting timestamp. * security: return error from LoadClientTLSFromFile, resolve relative PEM paths Change LoadClientTLSFromFile to return (grpc.DialOption, error) so callers can handle failures explicitly instead of a silent insecure fallback. Resolve relative PEM paths (grpc.ca, grpc.client.cert, grpc.client.key) against the config file's directory.	2026-04-01 11:05:43 -07:00
Chris LuandGitHub	ced2236cc6	Adjust rename events metadata format (#8854) ... * rename metadata events * fix subscription filter to use NewEntry.Name for rename path matching The server-side subscription filter constructed the new path using OldEntry.Name instead of NewEntry.Name when checking if a rename event's destination matches the subscriber's path prefix. This could cause events to be incorrectly filtered when a rename changes the file name. * fix bucket events to handle rename of bucket directories onBucketEvents only checked IsCreate and IsDelete. A bucket directory rename via AtomicRenameEntry now emits a single rename event (both OldEntry and NewEntry non-nil), which matched neither check. Handle IsRename by deleting the old bucket and creating the new one. * fix replicator to handle rename events across directory boundaries Two issues fixed: 1. The replicator filtered events by checking if the key (old path) was under the source directory. Rename events now use the old path as key, so renames from outside into the watched directory were silently dropped. Now both old and new paths are checked, and cross-boundary renames are converted to create or delete. 2. NewParentPath was passed to the sink without remapping to the sink's target directory structure, causing the sink to write entries at the wrong location. Now NewParentPath is remapped alongside the key. * fix filer sync to handle rename events crossing directory boundaries The early directory-prefix filter only checked resp.Directory (old parent). Rename events now carry the old parent as Directory, so renames from outside the source path into it were dropped before reaching the existing cross-boundary handling logic. Check both old and new directories against sourcePath and excludePaths so the downstream old-key/new-key logic can properly convert these to create or delete operations. * fix metadata event path matching * fix metadata event consumers for rename targets * Fix replication rename target keys Logical rename events now reach replication sinks with distinct source and target paths.\n\nHandle non-filer sinks as delete-plus-create on the translated target key, and make the rename fallback path create at the translated target key too.\n\nAdd focused tests covering non-filer renames, filer rename updates, and the fallback path.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix filer sync rename path scoping Use directory-boundary matching instead of raw prefix checks when classifying source and target paths during filer sync.\n\nAlso apply excludePaths per side so renames across excluded boundaries downgrade cleanly to create/delete instead of being misclassified as in-scope updates.\n\nAdd focused tests for boundary matching and rename classification.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix replicator directory boundary checks Use directory-boundary matching instead of raw prefix checks when deciding whether a source or target path is inside the watched tree or an excluded subtree.\n\nThis prevents sibling paths such as /foo and /foobar from being misclassified during rename handling, and preserves the earlier rename-target-key fix.\n\nAdd focused tests for boundary matching and rename classification across sibling/excluded directories.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix etc-remote rename-out handling Use boundary-safe source/target directory membership when classifying metadata events under DirectoryEtcRemote.\n\nThis prevents rename-out events from being processed as config updates, while still treating them as removals where appropriate for the remote sync and remote gateway command paths.\n\nAdd focused tests for update/removal classification and sibling-prefix handling.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Defer rename events until commit Queue logical rename metadata events during atomic and streaming renames and publish them only after the transaction commits successfully.\n\nThis prevents subscribers from seeing delete or logical rename events for operations that later fail during delete or commit.\n\nAlso serialize notification.Queue swaps in rename tests and add failure-path coverage.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip descendant rename target lookups Avoid redundant target lookups during recursive directory renames once the destination subtree is known absent.\n\nThe recursive move path now inserts known-absent descendants directly, and the test harness exercises prefixed directory listing so the optimization is covered by a directory rename regression test.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tighten rename review tests Return filer_pb.ErrNotFound from the bucket tracking store test stub so it follows the FilerStore contract, and add a webhook filter case for same-name renames across parent directories.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix HardLinkId format verb in InsertEntryKnownAbsent error HardLinkId is a byte slice. %d prints each byte as a decimal number which is not useful for an identifier. Use %x to match the log line two lines above. * only skip descendant target lookup when source and dest use same store moveFolderSubEntries unconditionally passed skipTargetLookup=true for every descendant. This is safe when all paths resolve to the same underlying store, but with path-specific store configuration a child's destination may map to a different backend that already holds an entry at that path. Use FilerStoreWrapper.SameActualStore to check per-child and fall back to the full CreateEntry path when stores differ. * add nil and create edge-case tests for metadata event scope helpers * extract pathIsEqualOrUnder into util.IsEqualOrUnder Identical implementations existed in both replication/replicator.go and command/filer_sync.go. Move to util.IsEqualOrUnder (alongside the existing FullPath.IsUnder) and remove the duplicates. * use MetadataEventTargetDirectory for new-side directory in filer sync The new-side directory checks and sourceNewKey computation used message.NewParentPath directly. If NewParentPath were empty (legacy events, older filer versions during rolling upgrades), sourceNewKey would be wrong (/filename instead of /dir/filename) and the UpdateEntry parent path rewrite would panic on slice bounds. Derive targetDir once from MetadataEventTargetDirectory, which falls back to resp.Directory when NewParentPath is empty, and use it consistently for all new-side checks and the sink parent path.	2026-03-30 18:25:11 -07:00
Chris LuandGitHub	937a168d34	notification.kafka: add SASL authentication and TLS support (#8832) ... * notification.kafka: add SASL authentication and TLS support (#8827) Wire sarama SASL (PLAIN, SCRAM-SHA-256, SCRAM-SHA-512) and TLS configuration into the Kafka notification producer and consumer, enabling connections to secured Kafka clusters. * notification.kafka: validate mTLS config * kafka notification: validate partial mTLS config, replace panics with errors - Reject when only one of tls_client_cert/tls_client_key is provided - Replace three panic() calls in KafkaInput.initialize with returned errors * kafka notification: enforce minimum TLS 1.2 for Kafka connections	2026-03-29 13:45:54 -07:00
Chris LuandGitHub	81369b8a83	improve: large file sync throughput for remote.cache and filer.sync (#8676) ... * improve large file sync throughput for remote.cache and filer.sync Three main throughput improvements: 1. Adaptive chunk sizing for remote.cache: targets ~32 chunks per file instead of always starting at 5MB. A 500MB file now uses ~16MB chunks (32 chunks) instead of 5MB chunks (100 chunks), reducing per-chunk overhead (volume assign, gRPC call, needle write) by 3x. 2. Configurable concurrency at every layer: - remote.cache chunk concurrency: -chunkConcurrency flag (default 8) - remote.cache S3 download concurrency: -downloadConcurrency flag (default raised from 1 to 5 per chunk) - filer.sync chunk concurrency: -chunkConcurrency flag (default 32) 3. S3 multipart download concurrency raised from 1 to 5: the S3 manager downloader was using Concurrency=1, serializing all part downloads within each chunk. This alone can 5x per-chunk download speed. The concurrency values flow through the gRPC request chain: shell command → CacheRemoteObjectToLocalClusterRequest → FetchAndWriteNeedleRequest → S3 downloader Zero values in the request mean "use server defaults", maintaining full backward compatibility with existing callers. Ref #8481 * fix: use full maxMB for chunk size cap and remove loop guard Address review feedback: - Use full maxMB instead of maxMB/2 for maxChunkSize to avoid unnecessarily limiting chunk size for very large files. - Remove chunkSize < maxChunkSize guard from the safety loop so it can always grow past maxChunkSize when needed to stay under 1000 chunks (e.g., extremely large files with small maxMB). * address review feedback: help text, validation, naming, docs - Fix help text for -chunkConcurrency and -downloadConcurrency flags to say "0 = server default" instead of advertising specific numeric defaults that could drift from the server implementation. - Validate chunkConcurrency and downloadConcurrency are within int32 range before narrowing, returning a user-facing error if out of range. - Rename ReadRemoteErr to readRemoteErr to follow Go naming conventions. - Add doc comment to SetChunkConcurrency noting it must be called during initialization before replication goroutines start. - Replace doubling loop in chunk size safety check with direct ceil(remoteSize/1000) computation to guarantee the 1000-chunk cap. * address Copilot review: clamp concurrency, fix chunk count, clarify proto docs - Use ceiling division for chunk count check to avoid overcounting when file size is an exact multiple of chunk size. - Clamp chunkConcurrency (max 1024) and downloadConcurrency (max 1024 at filer, max 64 at volume server) to prevent excessive goroutines. - Always use ReadFileWithConcurrency when the client supports it, falling back to the implementation's default when value is 0. - Clarify proto comments that download_concurrency only applies when the remote storage client supports it (currently S3). - Include specific server defaults in help text (e.g., "0 = server default 8") so users see the actual values in -h output. * fix data race on executionErr and use %w for error wrapping - Protect concurrent writes to executionErr in remote.cache worker goroutines with a sync.Mutex to eliminate the data race. - Use %w instead of %v in volume_grpc_remote.go error formatting to preserve the error chain for errors.Is/errors.As callers.	2026-03-17 16:49:56 -07:00
3 people	b665c329bc	fix(replication): resume partial chunk reads on EOF instead of re-downloading (#8607) ... * fix(replication): resume partial chunk reads on EOF instead of re-downloading When replicating chunks and the source connection drops mid-transfer, accumulate the bytes already received and retry with a Range header to fetch only the remaining bytes. This avoids re-downloading potentially large chunks from scratch on each retry, reducing load on busy source servers and speeding up recovery. * test(replication): add tests for downloadWithRange including gzip partial reads Tests cover: - No offset (no Range header sent) - With offset (Range header verified) - Content-Disposition filename extraction - Partial read + resume: server drops connection mid-transfer, client resumes with Range from the offset of received bytes - Gzip partial read + resume: first response is gzip-encoded (Go auto- decompresses), connection drops, resume request gets decompressed data (Go doesn't add Accept-Encoding when Range is set, so the server decompresses), combined bytes match original * fix(replication): address PR review comments - Consolidate downloadWithRange into DownloadFile with optional offset parameter (variadic), eliminating code duplication (DRY) - Validate HTTP response status: require 206 + correct Content-Range when offset > 0, reject when server ignores Range header - Use if/else for fullData assignment for clarity - Add test for rejected Range (server returns 200 instead of 206) * refactor(replication): remove unused ReplicationSource interface The interface was never referenced and its signature didn't match the actual FilerSource.ReadPart method. --------- Co-authored-by: Copilot <copilot@github.com>	2026-03-11 22:38:22 -07:00
3 people	0647f66bb5	filer.sync: add exponential backoff on unexpected EOF during replication (#8557) ... * filer.sync: add exponential backoff on unexpected EOF during replication When the source volume server drops connections under high traffic, filer.sync retries aggressively (every 1-6s), hammering the already overloaded source. This adds a longer exponential backoff (10s to 2min) specifically for "unexpected EOF" errors, reducing pressure on the source while still retrying indefinitely until success. Also adds more logging throughout the replication path: - Log source URL and error at V(0) when ReadPart or io.ReadAll fails - Log content-length and byte counts at V(4) on success - Log backoff duration in retry messages Fixes #8542 * filer.sync: extract backoff helper and fix 2-minute cap - Extract nextEofBackoff() and isEofError() helpers to deduplicate the backoff logic between fetchAndWrite and uploadManifestChunk - Fix the cap: previously 80s would double to 160s and pass the < 2min check uncapped. Now doubles first, then clamps to 2min. * filer.sync: log source URL instead of empty upload URL on read errors UploadUrl is not populated until after the reader is consumed, so the V(0) and V(4) logs were printing an empty string. Add SourceUrl field to UploadOption and populate it from the HTTP response in fetchAndWrite. * filer.sync: guard isEofError against nil error * filer.sync: use errors.Is for EOF detection, fix log wording - Replace broad substring matching ("read input", "unexpected EOF") with errors.Is(err, io.ErrUnexpectedEOF) and errors.Is(err, io.EOF) so only actual EOF errors trigger the longer backoff - Fix awkward log phrasing: "interrupted replicate" → "interrupted while replicating" * filer.sync: remove EOF backoff from uploadManifestChunk uploadManifestChunk reads from an in-memory bytes.Reader, so any EOF errors there are from the destination side, not a broken source stream. The long source-oriented backoff is inappropriate; let RetryUntil handle destination retries at its normal cadence. --------- Co-authored-by: Copilot <copilot@github.com>	2026-03-08 14:33:37 -07:00
Chris Lu	e4b70c2521	go fix	2026-02-20 18:42:00 -08:00
Chris LuandGitHub	7fcbffed7f	filer.sync: support manifest chunks (#8299) ... * filer.sync support manifest chunks * filersink: address manifest sync review feedback	2026-02-10 20:18:35 -08:00
Chris LuandGitHub	be0379f6fd	Fix filer.sync retry on stale chunk (#8298) ... * Fix filer.sync stale chunk uploads * Tweak filersink stale logging	2026-02-10 19:06:35 -08:00
3 people	9012069bd7	chore: execute goimports to format the code (#7983) ... * chore: execute goimports to format the code Signed-off-by: promalert <promalert@outlook.com> * goimports -w . --------- Signed-off-by: promalert <promalert@outlook.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-01-07 13:06:08 -08:00
Chris LuandGitHub	25975bacfb	fix(gcs): resolve credential conflict and improve backup logging (#7951) ... * fix(gcs): resolve credential conflict and improve backup logging - Workaround GCS SDK's "multiple credential options" error by manually constructing an authenticated HTTP client. - Include source entry path in filer backup error logs for better visibility on missing volumes/404s. * fix: address PR review feedback - Add nil check for EventNotification in getSourceKey - Avoid reassigning google_application_credentials parameter in gcs_sink.go * fix(gcs): return errors instead of calling glog.Fatalf in initialize Adheres to Go best practices and allows for more graceful failure handling by callers. * read from bind ip	2026-01-03 14:41:25 -08:00
Chris LuandGitHub	2f6aa98221	Refactor: Replace removeDuplicateSlashes with NormalizeObjectKey (#7873) ... * Replace removeDuplicateSlashes with NormalizeObjectKey Use s3_constants.NormalizeObjectKey instead of removeDuplicateSlashes in most places for consistency. NormalizeObjectKey handles both duplicate slash removal and ensures the path starts with '/', providing more complete normalization. * Fix double slash issues after NormalizeObjectKey After using NormalizeObjectKey, object keys have a leading '/'. This commit ensures: - getVersionedObjectDir strips leading slash before concatenation - getEntry calls receive names without leading slash - String concatenation with '/' doesn't create '//' paths This prevents path construction errors like: /buckets/bucket//object (wrong) /buckets/bucket/object (correct) * ensure object key leading "/" * fix compilation * fix: Strip leading slash from object keys in S3 API responses After introducing NormalizeObjectKey, all internal object keys have a leading slash. However, S3 API responses must return keys without leading slashes to match AWS S3 behavior. Fixed in three functions: - addVersion: Strip slash for version list entries - processRegularFile: Strip slash for regular file entries - processExplicitDirectory: Strip slash for directory entries This ensures ListObjectVersions and similar APIs return keys like 'bar' instead of '/bar', matching S3 API specifications. * fix: Normalize keyMarker for consistent pagination comparison The S3 API provides keyMarker without a leading slash (e.g., 'object-001'), but after introducing NormalizeObjectKey, all internal object keys have leading slashes (e.g., '/object-001'). When comparing keyMarker < normalizedObjectKey in shouldSkipObjectForMarker, the ASCII value of '/' (47) is less than 'o' (111), causing all objects to be incorrectly skipped during pagination. This resulted in page 2 and beyond returning 0 results. Fix: Normalize the keyMarker when creating versionCollector so comparisons work correctly with normalized object keys. Fixes pagination tests: - TestVersioningPaginationOver1000Versions - TestVersioningPaginationMultipleObjectsManyVersions * refactor: Change NormalizeObjectKey to return keys without leading slash BREAKING STRATEGY CHANGE: Previously, NormalizeObjectKey added a leading slash to all object keys, which required stripping it when returning keys to S3 API clients and caused complexity in marker normalization for pagination. NEW STRATEGY: - NormalizeObjectKey now returns keys WITHOUT leading slash (e.g., 'foo/bar' not '/foo/bar') - This matches the S3 API format directly - All path concatenations now explicitly add '/' between bucket and object - No need to strip slashes in responses or normalize markers Changes: 1. Modified NormalizeObjectKey to strip leading slash instead of adding it 2. Fixed all path concatenations to use: - BucketsPath + '/' + bucket + '/' + object instead of: - BucketsPath + '/' + bucket + object 3. Reverted response key stripping in: - addVersion() - processRegularFile() - processExplicitDirectory() 4. Reverted keyMarker normalization in findVersionsRecursively() 5. Updated matchesPrefixFilter() to work with keys without leading slash 6. Fixed paths in handlers: - s3api_object_handlers.go (GetObject, HeadObject, cacheRemoteObjectForStreaming) - s3api_object_handlers_postpolicy.go - s3api_object_handlers_tagging.go - s3api_object_handlers_acl.go - s3api_version_id.go (getVersionedObjectDir, getVersionIdFormat) - s3api_object_versioning.go (getObjectVersionList, updateLatestVersionAfterDeletion) All versioning tests pass including pagination stress tests. * adjust format * Update post policy tests to match new NormalizeObjectKey behavior - Update TestPostPolicyKeyNormalization to expect keys without leading slashes - Update TestNormalizeObjectKey to expect keys without leading slashes - Update TestPostPolicyFilenameSubstitution to expect keys without leading slashes - Update path construction in tests to use new pattern: BucketsPath + '/' + bucket + '/' + object * Fix ListObjectVersions prefix filtering Remove leading slash addition to prefix parameter to allow correct filtering of .versions directories when listing object versions with a specific prefix. The prefix parameter should match entry paths relative to bucket root. Adding a leading slash was breaking the prefix filter for paginated requests. Fixes pagination issue where second page returned 0 versions instead of continuing with remaining versions. * no leading slash * Fix urlEscapeObject to add leading slash for filer paths NormalizeObjectKey now returns keys without leading slashes to match S3 API format. However, urlEscapeObject is used for filer paths which require leading slashes. Add leading slash back after normalization to ensure filer paths are correct. Fixes TestS3ApiServer_toFilerPath test failures. * adjust tests * normalize * Fix: Normalize prefixes and markers in LIST operations using NormalizeObjectKey Ensure consistent key normalization across all S3 operations (GET, PUT, LIST). Previously, LIST operations were not applying the same normalization rules (handling backslashes, duplicate slashes, leading slashes) as GET/PUT operations. Changes: - Updated normalizePrefixMarker() to call NormalizeObjectKey for both prefix and marker - This ensures prefixes with leading slashes, backslashes, or duplicate slashes are handled consistently with how object keys are normalized - Fixes Parquet test failures where pads.write_dataset creates implicit directory structures that couldn't be discovered by subsequent LIST operations - Added TestPrefixNormalizationInList and TestListPrefixConsistency tests All existing LIST tests continue to pass with the normalization improvements. * Add debugging logging to LIST operations to track prefix normalization * Fix: Remove leading slash addition from GetPrefix to work with NormalizeObjectKey The NormalizeObjectKey function removes leading slashes to match S3 API format (e.g., 'foo/bar' not '/foo/bar'). However, GetPrefix was adding a leading slash back, which caused LIST operations to fail with incorrect path handling. Now GetPrefix only normalizes duplicate slashes without adding a leading slash, which allows NormalizeObjectKey changes to work correctly for S3 LIST operations. All Parquet integration tests now pass (20/20). * Fix: Handle object paths without leading slash in checkDirectoryObject NormalizeObjectKey() removes the leading slash to match S3 API format. However, checkDirectoryObject() was assuming the object path has a leading slash when processing directory markers (paths ending with '/'). Now we ensure the object has a leading slash before processing it for filer operations. Fixes implicit directory marker test (explicit_dir/) while keeping Parquet integration tests passing (20/20). All tests pass: - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * Fix: Handle explicit directory markers with trailing slashes Explicit directory markers created with put_object(Key='dir/', ...) are stored in the filer with the trailing slash as part of the name. The checkDirectoryObject() function now checks for both: 1. Explicit directories: lookup with trailing slash preserved (e.g., 'explicit_dir/') 2. Implicit directories: lookup without trailing slash (e.g., 'implicit_dir') This ensures both types of directory markers are properly recognized. All tests pass: - Implicit directory tests: 6/6 (including explicit directory marker test) - Parquet integration tests: 20/20 * Fix: Preserve trailing slash in NormalizeObjectKey NormalizeObjectKey now preserves trailing slashes when normalizing object keys. This is important for explicit directory markers like 'explicit_dir/' which rely on the trailing slash to be recognized as directory objects. The normalization process: 1. Notes if trailing slash was present 2. Removes duplicate slashes and converts backslashes 3. Removes leading slash for S3 API format 4. Restores trailing slash if it was in the original This ensures explicit directory markers created with put_object(Key='dir/', ...) are properly normalized and can be looked up by their exact name. All tests pass: - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * clean object * Fix: Don't restore trailing slash if result is empty When normalizing paths that are only slashes (e.g., '///', '/'), the function should return an empty string, not a single slash. The fix ensures we only restore the trailing slash if the result is non-empty. This fixes the 'just_slashes' test case: - Input: '///' - Expected: '' - Previous: '/' - Fixed: '' All tests now pass: - Unit tests: TestNormalizeObjectKey (13/13) - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * prefixEndsOnDelimiter * Update s3api_object_handlers_list.go * Update s3api_object_handlers_list.go * handle create directory	2025-12-24 19:07:08 -08:00
Chris LuandGitHub	cc2edfaf68	fix: enable RetryForever for active-active cluster sync to prevent out-of-sync (#7840) ... Fixes #7230 When a cluster goes down during file replication, the chunk upload process would fail after a limited number of retries. Once the remote cluster came back online, those failed uploads were never retried, leaving the clusters out-of-sync. This change enables the RetryForever flag in the UploadOption when replicating chunks between filers. This ensures that upload operations will keep retrying indefinitely, and once the remote cluster comes back online, the pending uploads will automatically succeed. Users no longer need to manually run fs.meta.save and fs.meta.load as a workaround for out-of-sync clusters.	2025-12-22 00:58:23 -08:00
Chris LuandGitHub	ec4f7cf33c	Filer: Fixed critical bugs in the Azure SDK migration (PR #7310) (#7401) ... * Fixed critical bugs in the Azure SDK migration (PR #7310) fix https://github.com/seaweedfs/seaweedfs/issues/5044 * purge emojis * conditional delete * Update azure_sink_test.go * refactoring * refactor * add context to each call * refactor * address comments * refactor * defer * DeleteSnapshots The conditional delete in handleExistingBlob was missing DeleteSnapshots, which would cause the delete operation to fail on Azure storage accounts that have blob snapshots enabled. * ensure the expected size * adjust comment	2025-10-28 22:16:21 -07:00
4 people	263e891da0	Clients to volume server requires JWT tokens for all read operations (#7376) ... * [Admin UI] Login not possible due to securecookie error * avoid 404 favicon * Update weed/admin/dash/auth_middleware.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * address comments * avoid variable over shadowing * log session save error * When jwt.signing.read.key is enabled in security.toml, the volume server requires JWT tokens for all read operations. * reuse fileId * refactor --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	2025-10-24 17:09:58 -07:00
3 people	c5a9c27449	Migrate from deprecated azure-storage-blob-go to modern Azure SDK (#7310) ... * Migrate from deprecated azure-storage-blob-go to modern Azure SDK Migrates Azure Blob Storage integration from the deprecated github.com/Azure/azure-storage-blob-go to the modern github.com/Azure/azure-sdk-for-go/sdk/storage/azblob SDK. ## Changes ### Removed Files - weed/remote_storage/azure/azure_highlevel.go - Custom upload helper no longer needed with new SDK ### Updated Files - weed/remote_storage/azure/azure_storage_client.go - Migrated from ServiceURL/ContainerURL/BlobURL to Client-based API - Updated client creation using NewClientWithSharedKeyCredential - Replaced ListBlobsFlatSegment with NewListBlobsFlatPager - Updated Download to DownloadStream with proper HTTPRange - Replaced custom uploadReaderAtToBlockBlob with UploadStream - Updated GetProperties, SetMetadata, Delete to use new client methods - Fixed metadata conversion to return map[string]*string - weed/replication/sink/azuresink/azure_sink.go - Migrated from ContainerURL to Client-based API - Updated client initialization - Replaced AppendBlobURL with AppendBlobClient - Updated error handling to use azcore.ResponseError - Added streaming.NopCloser for AppendBlock ### New Test Files - weed/remote_storage/azure/azure_storage_client_test.go - Comprehensive unit tests for all client operations - Tests for Traverse, ReadFile, WriteFile, UpdateMetadata, Delete - Tests for metadata conversion function - Benchmark tests - Integration tests (skippable without credentials) - weed/replication/sink/azuresink/azure_sink_test.go - Unit tests for Azure sink operations - Tests for CreateEntry, UpdateEntry, DeleteEntry - Tests for cleanKey function - Tests for configuration-based initialization - Integration tests (skippable without credentials) - Benchmark tests ### Dependency Updates - go.mod: Removed github.com/Azure/azure-storage-blob-go v0.15.0 - go.mod: Made github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.2 direct dependency - All deprecated dependencies automatically cleaned up ## API Migration Summary Old SDK → New SDK mappings: - ServiceURL → Client (service-level operations) - ContainerURL → ContainerClient - BlobURL → BlobClient - BlockBlobURL → BlockBlobClient - AppendBlobURL → AppendBlobClient - ListBlobsFlatSegment() → NewListBlobsFlatPager() - Download() → DownloadStream() - Upload() → UploadStream() - Marker-based pagination → Pager-based pagination - azblob.ResponseError → azcore.ResponseError ## Testing All tests pass: - ✅ Unit tests for metadata conversion - ✅ Unit tests for helper functions (cleanKey) - ✅ Interface implementation tests - ✅ Build successful - ✅ No compilation errors - ✅ Integration tests available (require Azure credentials) ## Benefits - ✅ Uses actively maintained SDK - ✅ Better performance with modern API design - ✅ Improved error handling - ✅ Removes ~200 lines of custom upload code - ✅ Reduces dependency count - ✅ Better async/streaming support - ✅ Future-proof against SDK deprecation ## Backward Compatibility The changes are transparent to users: - Same configuration parameters (account name, account key) - Same functionality and behavior - No changes to SeaweedFS API or user-facing features - Existing Azure storage configurations continue to work ## Breaking Changes None - this is an internal implementation change only. * Address Gemini Code Assist review comments Fixed three issues identified by Gemini Code Assist: 1. HIGH: ReadFile now uses blob.CountToEnd when size is 0 - Old SDK: size=0 meant "read to end" - New SDK: size=0 means "read 0 bytes" - Fix: Use blob.CountToEnd (-1) to read entire blob from offset 2. MEDIUM: Use to.Ptr() instead of slice trick for DeleteSnapshots - Replaced &[]Type{value}[0] with to.Ptr(value) - Cleaner, more idiomatic Azure SDK pattern - Applied to both azure_storage_client.go and azure_sink.go 3. Added missing imports: - github.com/Azure/azure-sdk-for-go/sdk/azcore/to These changes improve code clarity and correctness while following Azure SDK best practices. * Address second round of Gemini Code Assist review comments Fixed all issues identified in the second review: 1. MEDIUM: Added constants for hardcoded values - Defined defaultBlockSize (4 MB) and defaultConcurrency (16) - Applied to WriteFile UploadStream options - Improves maintainability and readability 2. MEDIUM: Made DeleteFile idempotent - Now returns nil (no error) if blob doesn't exist - Uses bloberror.HasCode(err, bloberror.BlobNotFound) - Consistent with idempotent operation expectations 3. Fixed TestToMetadata test failures - Test was using lowercase 'x-amz-meta-' but constant is 'X-Amz-Meta-' - Updated test to use s3_constants.AmzUserMetaPrefix - All tests now pass Changes: - Added import: github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror - Added constants: defaultBlockSize, defaultConcurrency - Updated WriteFile to use constants - Updated DeleteFile to be idempotent - Fixed test to use correct S3 metadata prefix constant All tests pass. Build succeeds. Code follows Azure SDK best practices. * Address third round of Gemini Code Assist review comments Fixed all issues identified in the third review: 1. MEDIUM: Use bloberror.HasCode for ContainerAlreadyExists - Replaced fragile string check with bloberror.HasCode() - More robust and aligned with Azure SDK best practices - Applied to CreateBucket test 2. MEDIUM: Use bloberror.HasCode for BlobNotFound in test - Replaced generic error check with specific BlobNotFound check - Makes test more precise and verifies correct error returned - Applied to VerifyDeleted test 3. MEDIUM: Made DeleteEntry idempotent in azure_sink.go - Now returns nil (no error) if blob doesn't exist - Uses bloberror.HasCode(err, bloberror.BlobNotFound) - Consistent with DeleteFile implementation - Makes replication sink more robust to retries Changes: - Added import to azure_storage_client_test.go: bloberror - Added import to azure_sink.go: bloberror - Updated CreateBucket test to use bloberror.HasCode - Updated VerifyDeleted test to use bloberror.HasCode - Updated DeleteEntry to be idempotent All tests pass. Build succeeds. Code uses Azure SDK best practices. * Address fourth round of Gemini Code Assist review comments Fixed two critical issues identified in the fourth review: 1. HIGH: Handle BlobAlreadyExists in append blob creation - Problem: If append blob already exists, Create() fails causing replication failure - Fix: Added bloberror.HasCode(err, bloberror.BlobAlreadyExists) check - Behavior: Existing append blobs are now acceptable, appends can proceed - Impact: Makes replication sink more robust, prevents unnecessary failures - Location: azure_sink.go CreateEntry function 2. MEDIUM: Configure custom retry policy for download resiliency - Problem: Old SDK had MaxRetryRequests: 20, new SDK defaults to 3 retries - Fix: Configured policy.RetryOptions with MaxRetries: 10 - Settings: TryTimeout=1min, RetryDelay=2s, MaxRetryDelay=1min - Impact: Maintains similar resiliency in unreliable network conditions - Location: azure_storage_client.go client initialization Changes: - Added import: github.com/Azure/azure-sdk-for-go/sdk/azcore/policy - Updated NewClientWithSharedKeyCredential to include ClientOptions with retry policy - Updated CreateEntry error handling to allow BlobAlreadyExists Technical details: - Retry policy uses exponential backoff (default SDK behavior) - MaxRetries=10 provides good balance (was 20 in old SDK, default is 3) - TryTimeout prevents individual requests from hanging indefinitely - BlobAlreadyExists handling allows idempotent append operations All tests pass. Build succeeds. Code is more resilient and robust. * Update weed/replication/sink/azuresink/azure_sink.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Revert "Update weed/replication/sink/azuresink/azure_sink.go" This reverts commit 605e41cadf. * Address fifth round of Gemini Code Assist review comment Added retry policy to azure_sink.go for consistency and resiliency: 1. MEDIUM: Configure retry policy in azure_sink.go client - Problem: azure_sink.go was using default retry policy (3 retries) while azure_storage_client.go had custom policy (10 retries) - Fix: Added same retry policy configuration for consistency - Settings: MaxRetries=10, TryTimeout=1min, RetryDelay=2s, MaxRetryDelay=1min - Impact: Replication sink now has same resiliency as storage client - Rationale: Replication sink needs to be robust against transient network errors Changes: - Added import: github.com/Azure/azure-sdk-for-go/sdk/azcore/policy - Updated NewClientWithSharedKeyCredential call in initialize() function - Both azure_storage_client.go and azure_sink.go now have identical retry policies Benefits: - Consistency: Both Azure clients now use same retry configuration - Resiliency: Replication operations more robust to network issues - Best practices: Follows Azure SDK recommended patterns for production use All tests pass. Build succeeds. Code is consistent and production-ready. * fmt * Address sixth round of Gemini Code Assist review comment Fixed HIGH priority metadata key validation for Azure compliance: 1. HIGH: Handle metadata keys starting with digits - Problem: Azure Blob Storage requires metadata keys to be valid C# identifiers - Constraint: C# identifiers cannot start with a digit (0-9) - Issue: S3 metadata like 'x-amz-meta-123key' would fail with InvalidInput error - Fix: Prefix keys starting with digits with underscore '_' - Example: '123key' becomes '_123key', '456-test' becomes '_456_test' 2. Code improvement: Use strings.ReplaceAll for better readability - Changed from: strings.Replace(str, "-", "_", -1) - Changed to: strings.ReplaceAll(str, "-", "_") - Both are functionally equivalent, ReplaceAll is more readable Changes: - Updated toMetadata() function in azure_storage_client.go - Added digit prefix check: if key[0] >= '0' && key[0] <= '9' - Added comprehensive test case 'keys starting with digits' - Tests cover: '123key' -> '_123key', '456-test' -> '_456_test', '789' -> '_789' Technical details: - Azure SDK validates metadata keys as C# identifiers - C# identifier rules: must start with letter or underscore - Digits allowed in identifiers but not as first character - This prevents SetMetadata() and UploadStream() failures All tests pass including new test case. Build succeeds. Code is now fully compliant with Azure metadata requirements. * Address seventh round of Gemini Code Assist review comment Normalize metadata keys to lowercase for S3 compatibility: 1. MEDIUM: Convert metadata keys to lowercase - Rationale: S3 specification stores user-defined metadata keys in lowercase - Consistency: Azure Blob Storage metadata is case-insensitive - Best practice: Normalizing to lowercase ensures consistent behavior - Example: 'x-amz-meta-My-Key' -> 'my_key' (not 'My_Key') Changes: - Updated toMetadata() to apply strings.ToLower() to keys - Added comment explaining S3 lowercase normalization - Order of operations: strip prefix -> lowercase -> replace dashes -> check digits Test coverage: - Added new test case 'uppercase and mixed case keys' - Tests: 'My-Key' -> 'my_key', 'UPPERCASE' -> 'uppercase', 'MiXeD-CaSe' -> 'mixed_case' - All 6 test cases pass Benefits: - S3 compatibility: Matches S3 metadata key behavior - Azure consistency: Case-insensitive keys work predictably - Cross-platform: Same metadata keys work identically on both S3 and Azure - Prevents issues: No surprises from case-sensitive key handling Implementation: ```go key := strings.ReplaceAll(strings.ToLower(k[len(s3_constants.AmzUserMetaPrefix):]), "-", "_") ``` All tests pass. Build succeeds. Metadata handling is now fully S3-compatible. * Address eighth round of Gemini Code Assist review comments Use %w instead of %v for error wrapping across both files: 1. MEDIUM: Error wrapping in azure_storage_client.go - Problem: Using %v in fmt.Errorf loses error type information - Modern Go practice: Use %w to preserve error chains - Benefit: Enables errors.Is() and errors.As() for callers - Example: Can check for bloberror.BlobNotFound after wrapping 2. MEDIUM: Error wrapping in azure_sink.go - Applied same improvement for consistency - All error wrapping now preserves underlying errors - Improved debugging and error handling capabilities Changes applied to all fmt.Errorf calls: - azure_storage_client.go: 10 instances changed from %v to %w - Invalid credential error - Client creation error - Traverse errors - Download errors (2) - Upload error - Delete error - Create/Delete bucket errors (2) - azure_sink.go: 3 instances changed from %v to %w - Credential creation error - Client creation error - Delete entry error - Create append blob error Benefits: - Error inspection: Callers can use errors.Is(err, target) - Error unwrapping: Callers can use errors.As(err, &target) - Type preservation: Original error types maintained through wraps - Better debugging: Full error chain available for inspection - Modern Go: Follows Go 1.13+ error wrapping best practices Example usage after this change: ```go err := client.ReadFile(...) if errors.Is(err, bloberror.BlobNotFound) { // Can detect specific Azure errors even after wrapping } ``` All tests pass. Build succeeds. Error handling is now modern and robust. * Address ninth round of Gemini Code Assist review comment Improve metadata key sanitization with comprehensive character validation: 1. MEDIUM: Complete Azure C# identifier validation - Problem: Previous implementation only handled dashes, not all invalid chars - Issue: Keys like 'my.key', 'key+plus', 'key@symbol' would cause InvalidMetadata - Azure requirement: Metadata keys must be valid C# identifiers - Valid characters: letters (a-z, A-Z), digits (0-9), underscore (_) only 2. Implemented robust regex-based sanitization - Added package-level regex: `[^a-zA-Z0-9_]` - Matches ANY character that's not alphanumeric or underscore - Replaces all invalid characters with underscore - Compiled once at package init for performance Implementation details: - Regex declared at package level: var invalidMetadataChars = regexp.MustCompile(`[^a-zA-Z0-9_]`) - Avoids recompiling regex on every toMetadata() call - Efficient single-pass replacement of all invalid characters - Processing order: lowercase -> regex replace -> digit check Examples of character transformations: - Dots: 'my.key' -> 'my_key' - Plus: 'key+plus' -> 'key_plus' - At symbol: 'key@symbol' -> 'key_symbol' - Mixed: 'key-with.' -> 'key_with_' - Slash: 'key/slash' -> 'key_slash' - Combined: '123-key.value+test' -> '_123_key_value_test' Test coverage: - Added comprehensive test case 'keys with invalid characters' - Tests: dot, plus, at-symbol, dash+dot, slash - All 7 test cases pass (was 6, now 7) Benefits: - Complete Azure compliance: Handles ALL invalid characters - Robust: Works with any S3 metadata key format - Performant: Regex compiled once, reused efficiently - Maintainable: Single source of truth for valid characters - Prevents errors: No more InvalidMetadata errors during upload All tests pass. Build succeeds. Metadata sanitization is now bulletproof. * Address tenth round review - HIGH: Fix metadata key collision issue Prevent metadata loss by using hex encoding for invalid characters: 1. HIGH PRIORITY: Metadata key collision prevention - Critical Issue: Different S3 keys mapping to same Azure key causes data loss - Example collisions (BEFORE): * 'my-key' -> 'my_key' * 'my.key' -> 'my_key' ❌ COLLISION! Second overwrites first * 'my_key' -> 'my_key' ❌ All three map to same key! - Fixed with hex encoding (AFTER): * 'my-key' -> 'my_2d_key' (dash = 0x2d) * 'my.key' -> 'my_2e_key' (dot = 0x2e) * 'my_key' -> 'my_key' (underscore is valid) ✅ All three are now unique! 2. Implemented collision-proof hex encoding - Pattern: Invalid chars -> _XX_ where XX is hex code - Dash (0x2d): 'content-type' -> 'content_2d_type' - Dot (0x2e): 'my.key' -> 'my_2e_key' - Plus (0x2b): 'key+plus' -> 'key_2b_plus' - At (0x40): 'key@symbol' -> 'key_40_symbol' - Slash (0x2f): 'key/slash' -> 'key_2f_slash' 3. Created sanitizeMetadataKey() function - Encapsulates hex encoding logic - Uses ReplaceAllStringFunc for efficient transformation - Maintains digit prefix check for Azure C# identifier rules - Clear documentation with examples Implementation details: ```go func sanitizeMetadataKey(key string) string { // Replace each invalid character with _XX_ where XX is the hex code result := invalidMetadataChars.ReplaceAllStringFunc(key, func(s string) string { return fmt.Sprintf("_%02x_", s[0]) }) // Azure metadata keys cannot start with a digit if len(result) > 0 && result[0] >= '0' && result[0] <= '9' { result = "_" + result } return result } ``` Why hex encoding solves the collision problem: - Each invalid character gets unique hex representation - Two-digit hex ensures no confusion (always _XX_ format) - Preserves all information from original key - Reversible (though not needed for this use case) - Azure-compliant (hex codes don't introduce new invalid chars) Test coverage: - Updated all test expectations to match hex encoding - Added 'collision prevention' test case demonstrating uniqueness: * Tests my-key, my.key, my_key all produce different results * Proves metadata from different S3 keys won't collide - Total test cases: 8 (was 7, added collision prevention) Examples from tests: - 'content-type' -> 'content_2d_type' (0x2d = dash) - '456-test' -> '_456_2d_test' (digit prefix + dash) - 'My-Key' -> 'my_2d_key' (lowercase + hex encode dash) - 'key-with.' -> 'key_2d_with_2e_' (multiple chars: dash, dot, trailing dot) Benefits: - ✅ Zero collision risk: Every unique S3 key -> unique Azure key - ✅ Data integrity: No metadata loss from overwrites - ✅ Complete info preservation: Original key distinguishable - ✅ Azure compliant: Hex-encoded keys are valid C# identifiers - ✅ Maintainable: Clean function with clear purpose - ✅ Testable: Collision prevention explicitly tested All tests pass. Build succeeds. Metadata integrity is now guaranteed. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	2025-10-08 23:12:03 -07:00
Chris LuandGitHub	69553e5ba6	convert error fromating to %w everywhere (#6995)	2025-07-16 23:39:27 -07:00
3 people	4511c2cc1f	Changes logging function (#6919) ... * updated logging methods for stores * updated logging methods for stores * updated logging methods for filer * updated logging methods for uploader and http_util * updated logging methods for weed server --------- Co-authored-by: akosov <a.kosov@kryptonite.ru>	2025-06-24 08:44:06 -07:00
Aleksey KosovandGitHub	283d9e0079	Add context with request (#6824)	2025-05-28 11:34:02 -07:00
3 people	165af32d6b	added context to filer_client method calls (#6808) ... Co-authored-by: akosov <a.kosov@kryptonite.ru>	2025-05-22 09:46:49 -07:00
chrislu	33ba88df9c	fix from ensure() before actual deletion, within the b2 client library ... fix https://github.com/seaweedfs/seaweedfs/issues/6483	2025-01-28 23:59:42 -08:00
chrislu	ec155022e7	"golang.org/x/exp/slices" => "slices" and go fmt	2024-12-19 19:25:06 -08:00
chrislu	7a75928e7d	Azure sink: avoid overwriting existing files	2024-07-17 23:14:36 -07:00
vadimartynovandGitHub	86d92a42b4	Added tls for http clients (#5766) ... * Added global http client * Added Do func for global http client * Changed the code to use the global http client * Fix http client in volume uploader * Fixed pkg name * Fixed http util funcs * Fixed http client for bench_filer_upload * Fixed http client for stress_filer_upload * Fixed http client for filer_server_handlers_proxy * Fixed http client for command_fs_merge_volumes * Fixed http client for command_fs_merge_volumes and command_volume_fsck * Fixed http client for s3api_server * Added init global client for main funcs * Rename global_client to client * Changed: - fixed NewHttpClient; - added CheckIsHttpsClientEnabled func - updated security.toml in scaffold * Reduce the visibility of some functions in the util/http/client pkg * Added the loadSecurityConfig function * Use util.LoadSecurityConfiguration() in NewHttpClient func	2024-07-16 23:14:09 -07:00