mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-02 05:16:38 +00:00
d28cd946010a74f3de774174d2bcbef5fddcc96f
960
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
594fc667d5 |
Cut per-subscriber replay decode and widen replay concurrency (#9917)
* Filter metadata events before unmarshaling them per subscriber Every subscriber unmarshaled every log entry into a full event just to run the path filter, and entries carry complete chunk lists, so a fleet of path-filtered subscribers spends almost all replay CPU materializing events it then discards. A shallow wire scan now extracts just the directory, entry names and rename destination into a skeleton event, feeds the same matcher, and skips the decode for entries the subscriber cannot match. Any scan surprise (malformed bytes, merged duplicate message fields) falls back to the full decode, and the unsynced-events heartbeat keeps firing for skipped entries. * Raise the legacy replay cap The cap was sized when every replay pinned a private chunk reader per source filer. Replays now share decoded chunks, so sixteen needlessly serializes subscriber catch-up; the expensive part stays bounded by the cache's load gate. * Weight concurrent log-chunk loads by size The flat eight-load gate let eight tiny chunks through as reluctantly as eight full ones. Charge each load's chunk size against a 128MB in-flight budget instead: small chunks decode wide open while full-size ones still serialize enough to cap the transient peak. Oversized weights clamp to the budget so they can always acquire. * Propagate heartbeat send failures and reset the skip counter A failed heartbeat send means the stream is gone, so end the replay instead of scanning on. A delivered event also resets the skip counter, keeping the heartbeat cadence relative to the last thing the client actually received. * Share the unsynced-events counter across the prefilter and delivery Two independent counters could starve the heartbeat: alternating drops reset each side before either reached its threshold. One shared counter increments on every dropped entry, prefiltered or not, and only an actual delivery resets it, restoring the original cadence exactly. * Tighten comments * Benchmark the subscription match paths For a thousand-chunk event that the subscriber filters out, the shallow scan matches in 10us and 9 allocations against 175us and 4031 allocations for the full decode. |
||
|
Chris LuandGitHub
|
2ac5aa72c7 |
add elastic8 filer store for Elasticsearch 8 (#9916)
* elastic: fix listing against a missing or empty directory index
The refresh 404 leaked into the named return, so the first listing of a
directory whose index does not exist yet returned an error instead of an
empty result. Sorting also fails on an index with no documents
("No mapping found for [_id] in order to sort on"); unmapped_type
keeps the resumed-listing path working there.
* add elastic8 filer store for Elasticsearch 8
Elasticsearch 8 disables _id fielddata by default, so the elastic7
store's directory listings fail with "Fielddata access on the _id
field is disallowed". elastic8 uses the same client and configuration
options, but also indexes the document id as an Id field and sorts
listings on Id.keyword.
|
||
|
Chris LuandGitHub
|
bf76040046 |
Share metadata-log replays per chunk instead of per file (#9906)
* Share metadata-log replays per chunk instead of per file Log file chunks are immutable: each metadata-log flush uploads one whole buffer of complete records as a new chunk, and appends only add chunks. So cache decoded entries per chunk, with no age gate and no fingerprint revalidation. The per-file cache excluded files younger than two flush intervals, which is exactly the hot tail that every tailing or reconnecting subscriber replays — each through a private chunk reader holding an 8MB buffer and decoding the whole file from byte zero. A chunk's flush time also upper-bounds every record timestamp inside it, so a tail replay now skips cold chunks without reading them at all. If a chunk does not decode standalone (records spanning chunk boundaries, or a corrupt size prefix), fall back to streaming the whole file as one byte stream, resuming after the last yielded entry. * Evict idle metadata-log cache entries The replay cache only evicted on insert, so once filled it held its full budget forever. Stamp entries on use and sweep the LRU tail every minute, dropping anything untouched for five minutes; the cache now holds memory only while subscribers actually replay. * Reject implausible records when decoding log chunks proto.Unmarshal is permissive: empty payloads and unknown-field garbage parse without error, so a chunk starting mid-record could decode by coincidence and get cached instead of falling back to the byte stream. Enforce what the writer guarantees - records are never empty and carry strictly increasing positive timestamps within one flushed buffer. * Gate the singleflight test on an open flight The sleep alone only probabilistically created concurrent misses; a started channel now proves the loader holds the flight before callers are released. |
||
|
Chris LuandGitHub
|
9e98ec4b2e |
Share decoded metadata-log entries across subscriber replays (#9903)
perf(filer): share decoded log entries across metadata replays Concurrent SubscribeMetadata replays of the same persisted log history each opened a chunk reader per source filer and re-decoded the same files, so a reconnect storm multiplied into many GB of buffers. Cache the decoded entries of completed log files in a bounded LRU, coalescing concurrent loads with single-flight and bounding concurrent decodes. Each hit is validated against the file's current chunk set, so a file that received a late append is reloaded rather than served stale; reads that stop on an unreachable chunk are delivered but not cached so a transient outage re-probes on the next replay. |
||
|
Chris LuandGitHub
|
048f9ece2d |
Fix filer metadata-replay OOM under mount reconnect storms (#9901)
* fix(filer): propagate multi-filer metadata log read errors A genuine (non not-found) read error in one filer's log stream was logged and skipped, then the merged cursor advanced past the gap, silently dropping that file's events. Abort the whole replay so the subscriber re-reads from the unchanged position; chunk-not-found still skips. * perf(mount): read persisted metadata log chunks directly from volume servers Set LogFileReaderFn so the filer returns log file references and the mount reads the chunk data itself, instead of the filer reading, decoding, and streaming every persisted entry. Keeps a reconnect storm of many mounts from concentrating hundreds of concurrent log replays in filer memory. * perf(filer): pre-size chunk stream reader buffer to view size The chunk size is known up front, so grow the buffer once instead of letting bytes.Buffer double as the streamed pieces arrive (which transiently overshoots to ~2x per reader). * fix(filer): bound concurrent persisted-log replays Each server-side replay holds an open chunk reader per source filer plus a readahead buffer, so a reconnect storm of clients that predate the metadata-chunks offload multiplies into many GB. Gate replays with a semaphore; abort the acquire when the subscriber's stream is gone so cancelled clients do not pile up parked goroutines. |
||
|
Chris LuandGitHub
|
8776b9d311 |
feat(filer): object size distribution metric and dashboard panels (#9902)
* feat(filer): record object size distribution histogram Add SeaweedFS_filer_object_size_bytes, a histogram sampled when an object is first created in the filer namespace, covering every write protocol (S3, WebDAV, FUSE mount, direct HTTP). Buckets follow the 1KB/100KB/1MB/100MB/1GB ranges operators use to size collections. Directories, overwrites, and metadata-only updates are not sampled, so the bucket counts track the size distribution of distinct objects. * feat(metrics): add filer object size distribution dashboard panels Add a write-rate-by-size-range graph and a size-distribution bar gauge, driven by SeaweedFS_filer_object_size_bytes, to the standalone and Helm Grafana dashboards. Per-range subtractions are clamped at zero so transient negative rate() samples do not render below the axis. |
||
|
Chris LuandGitHub
|
7aba10fa1a |
fix(mongodb): merge URI auth fields with username/password override (#9889)
* fix(mongodb): merge URI auth fields with username/password override SetAuth replaced the whole Credential parsed from the URI, dropping AuthSource and AuthMechanism. Start from the URI-parsed Auth and only override the username and password so credentials scoped to a specific auth database keep working. * fix(mongodb): set PasswordSet for explicit credentials Required by GSSAPI auth when a password is supplied; ignored for other mechanisms. |
||
|
Chris LuandGitHub
|
d67fc48fbd |
fix(filer.sync): guard batched events against nil EventNotification (#9877)
* fix(filer.sync): guard batched events against nil EventNotification The server folds a backlog into one response: the first event in the top-level fields, the rest in resp.Events, and the pipelined sender can drain an idle heartbeat (nil EventNotification) into that tail. Only the envelope got the freshness-signal guard, so a batched heartbeat reached AddSyncJob and nil-derefed in IsEmpty while replaying a backlog buffered during a peer outage. Route every event, envelope and batched, through one handler that sends freshness signals (nil heartbeat, empty marker) to OnIdleHeartbeat. * fix(filer): guard MetaAggregator batched events against nil EventNotification The peer subscription's envelope is nil-guarded but its batched tail was not. The aggregator doesn't enable idle heartbeats today, so the server can't fold a nil EventNotification into the batch yet, but make the two loops consistent so it can't nil-deref if that changes. |
||
|
Chris LuandGitHub
|
a24f4844d3 |
filer: keep S3 list order byte-lexicographic regardless of SQL name column collation (#9824)
* mysql: keep S3 list order byte-lexicographic regardless of name column collation ORDER BY name and the name > ? pagination predicate follow the column collation, so a case-insensitive filemeta.name (e.g. utf8mb3_general_ci) returns S3 keys out of byte order and breaks clients that merge two sorted listings. Detect the live name collation at startup; only when it isn't binary, wrap the list comparison, prefix, and ORDER BY in BINARY name so order and pagination stay consistent. Correctly configured utf8mb4_bin tables keep their indexed range scan unchanged, and the operator gets a warning to convert the column. * postgres: keep S3 list order byte-lexicographic regardless of name column collation ORDER BY name and the name > $n pagination predicate follow the column or database collation, so a locale-aware filemeta.name (e.g. the en_US.UTF-8 database default) returns S3 keys out of byte order and breaks clients that merge two sorted listings. Detect the live name collation at startup; only when it isn't byte-ordered, wrap the list comparison, prefix, and ORDER BY in COLLATE "C" so order and pagination stay consistent. A byte-ordered (C/POSIX/C.UTF-8) column keeps its indexed range scan unchanged, and the operator gets a warning to declare the column COLLATE "C". |
||
|
Chris LuandGitHub
|
8c2d9f466f |
filer: stream persisted log files when serving metadata subscriptions (#9821)
* filer: stream persisted log files when serving metadata subscriptions readFileEntries buffered every LogEntry of a whole log file into memory before returning them one by one, making a subscription read O(entries in one log file) instead of O(one entry). On a filer with large per-entry metadata, many concurrent SubscribeMetadata streams each loading a full log file exhausted memory. Keep a current LogFileIterator and return one entry at a time, advancing files as each is exhausted. The deleted-volume skip is preserved. * filer: close the log file iterator on read errors too A genuine read error returned early without closing the current LogFileIterator, leaving its ChunkStreamReader alive until GC. Close on every exit path and propagate only a real error. * filer: close persisted-log iterators when a subscription stops early The streaming iterator keeps a log file reader open across calls, so a subscription that returns before EOF (early stop, cancellation) left the reader alive until GC. Add idempotent Close on LogFileQueueIterator and OrderedLogVisitor, and have ReadPersistedLogBuffer wait for the readahead goroutine and close the visitor on the way out. |
||
|
Chris LuandGitHub
|
8c60408bfb |
s3: auto-enforce bucket quota read-only both ways (#9774)
* s3: auto-enforce bucket quota read-only both ways Quota read-only only ever flipped when an admin re-ran s3.bucket.quota.enforce, so a bucket that went over quota stayed read-only forever even after usage dropped back under. Fold enforcement into the per-minute, leader-locked bucket-size loop the s3 gateway already runs for metrics: it now flips each bucket's read-only flag to match its quota in both directions, rewriting filer.conf only when a flag actually changes. The set/clear decision lives in one shared FilerConf.ApplyBucketQuotaReadOnly helper so the shell command and the gateway can't drift. * only manage read-only when a quota is set, never clobber manual locks * trim comments |
||
 7y-9andGitHub
|
fbcba51e73 | refactor: avoid unused sql insert result (#9734) | ||
|
Chris LuandGitHub
|
dfd05d14cb |
refactor(filer): remove the inode->path index and the NFS gateway (#9724)
* fix(filer): derive inodes by hash instead of a snowflake sequencer
Compute the same inode the FUSE mount would: non-hard-linked entries hash path + crtime, hard links hash their shared HardLinkId so every link resolves to one inode. Removes the snowflake inodeSequencer and the SEAWEEDFS_FILER_SNOWFLAKE_ID knob; inodes are now deterministic across filers.
* chore: remove the experimental NFS gateway
The NFS frontend ('weed nfs') was the only consumer of the inode->path index. Remove the weed/server/nfs package, the command and its registration, the integration test harness, and the CI workflow; go mod tidy drops the willscott/go-nfs and go-nfs-client dependencies.
* refactor(filer): drop the inode->path index
With the NFS gateway gone, nothing reads it. A regular file's inode is a pure hash of its path and a hard link's is a hash of its shared HardLinkId -- both derivable on demand -- so the secondary KV index and its write/remove hooks are dead. Removes filer_inode_index.go and the recordInodeIndex hooks from the store wrapper.
|
||
|
Chris LuandGitHub
|
21b4b81edb |
fix(filer/postgres): default to ON CONFLICT upsert to keep tx alive (#9709)
* fix(filer/postgres): default to ON CONFLICT upsert to keep tx alive A KvPut from the inode-index secondary write could fail with 23505 (duplicate key) inside a rename's transaction, after which the next statement returned 25P02 and rename surfaced to FUSE as EIO. Default the postgres upsert query when enableUpsert=true so INSERTs are idempotent; the enableUpsert=false escape hatch is preserved for non-PG-compatible backends. * fix(filer/mysql): default to ON DUPLICATE KEY UPDATE upsert Same shape as the postgres default: when enableUpsert=true but no upsertQuery is configured, install a sensible default so the inode-index KvPut does not waste a duplicate-key roundtrip on every entry write. Uses the VALUES() form so the default works on MariaDB and MySQL >=5.7; the MySQL 8.0.19 row-alias form is left to explicit config. * fix(filer): default enableUpsert=true for sql stores The default-template fallback only kicks in when enableUpsert=true, so minimal configs that omit the flag entirely were still exposed. Default it on for postgres/postgres2/mysql/mysql2; an explicit false in filer.toml still wins because SetDefault only fills absent keys. |
||
|
Chris LuandGitHub
|
8fd7c524c7 |
redis2: apply keyPrefix in KV methods (#9693)
KvPut/KvGet/KvDelete bypassed store.getKey(), so filer.store.id and other KV writes landed outside the configured prefix. With a Redis ACL restricted to the prefix this errored with NOPERM; without the ACL the keys silently lived in the wrong namespace. |
||
|
Chris LuandGitHub
|
c9868dcf2f |
filer/posixlock: remove the unused lock-set serde (#9676)
The codec (Set.Marshal/Unmarshal) and its posix_lock.proto were built to let the lock set ride in an inode's entry metadata, but the authority is in-memory and ownership handoff/restart is handled by mounts re-asserting their held locks over the RPC — neither serializes the set. Nothing calls the serde outside its own tests, so drop it (codec, proto, generated pb, Makefile). The in-memory Set/Manager are unchanged. |
||
|
Chris LuandGitHub
|
f8caaa4464 |
mount,filer: re-assert POSIX locks via keepalive (ownership migration + restart) (#9668)
* mount: renew POSIX lock leases via keepalive The mount tracks the inode keys it holds locks on and a background loop renews its session lease (KEEP_ALIVE) with each key's owner filer every 5s, within the filer's 15s TTL. A live mount is never reaped; a dead one stops renewing and owners reclaim its locks. Tracking is a superset: holds are added on grant and dropped only on owner release, so a still held lock is never under-renewed. * mount,filer: re-assert held POSIX locks via keepalive The owner filer holds POSIX advisory locks as in-memory soft state, so a key's owner change (ring rebalance) or an owner restart lost or stranded them: the new or restarted owner was blind to existing holders and would double-grant. Make the keepalive carry the mount's held lock ranges per key. The mount mirrors its own granted locks (posixOwn), and each tick re-asserts them to the key's current owner, which rebuilds that session's locks from the assertion — self -healing after a takeover or restart. The owner arbitrates re-asserted locks against other sessions so it never double-grants; a lock that lost a migration race is reported, not forced. A bare keepalive (no ranges) still just renews. |
||
|
Chris LuandGitHub
|
c97b69f8a4 |
filer: session lease + reaping for POSIX locks (#9666)
* filer: session lease + reaping for POSIX locks A mount renews its session lease by keepalive (new KEEP_ALIVE op); the owner filer records last-seen per session and a background sweeper reaps the locks of leased sessions that stop renewing — a dead or partitioned mount. Only sessions that have renewed are leased, so this is inert until mounts run with -posixLock. * mount: route POSIX advisory locks to the owner filer (-posixLock) (#9665) mount: route POSIX advisory locks to the owner filer under -dlm With -dlm, GetLk/SetLk/SetLkw and the flush/release cleanup paths go to the inode's owner filer via the PosixLock RPC instead of the local table, so flock/fcntl are honored across mounts. Advisory locking rides the same switch as whole-file write coordination — and is therefore off under writeback cache, which implies single-writer. The mount calls its filer and relies on filer-side forwarding to reach the owner. Keys are the inode identity (HardLinkId else path); SetLkw is client-side polling with the FUSE cancel channel (no server wait queue); a per-mount session id namespaces owners; a local hint avoids a release RPC on every close. * mount,filer: bound posix-lock release RPCs and stop the reaper on shutdown The unlock/release RPCs run off the syscall path (close/flush) and used context.Background() with no deadline, so a slow or unreachable filer could hang close() indefinitely; bound them to 5s (they still aren't cancelled by an interrupt). The lease-reaping sweeper now selects on a stop channel that FilerServer.Shutdown closes, instead of looping for the process lifetime. |
||
|
Chris LuandGitHub
|
564b94796a |
filer: in-memory POSIX lock authority (Manager) (#9663)
Concurrent multi-inode authority over the per-inode Set: a Set per opaque inode key (path, or hl:<HardLinkId>) plus a session->keys index so a dead mount's locks reap in O(locks held). Lock state stays in memory like the distributed lock manager's, off the replicated meta-log. TryLock/Unlock/ GetLk/ReleasePosixOwner/ReleaseFlockOwner/ReleaseSession; empty sets and stale index entries are pruned on release. |
||
|
Chris LuandGitHub
|
475ae2b443 |
filer: serialize the POSIX lock set for entry metadata (phase 2) (#9661)
* filer: POSIX advisory lock set primitive (phase 1) Pure per-inode conflict/coalesce/range-split logic for fcntl byte-range and flock whole-file locks, extracted from the mount's PosixLockTable without its wait queue or inode-map concurrency. Owner identity is (Sid, Owner) so the same FUSE owner on different mounts never aliases, and ReleaseSession reaps a dead mount's locks. The owner filer will hold one Set per inode under the per-path lock; no concurrency control here. * test: tolerate transient FUSE invisibility in ConcurrentReadWrite A concurrent truncating overwrite leaves a short-lived dentry/cache window where the file is momentarily ENOENT to another opener. Retry the reads and writes a few times before failing, as ConcurrentDirectoryOperations does. * filer: serialize the POSIX lock set for entry metadata Versioned fixed-width binary encoding of a Set, so an inode's held locks can ride in its entry metadata: a lock op materializes the Set from the blob, applies under the per-path lock, and writes it back. Empty set encodes to nil so a lock-free inode carries no blob. * filer: encode the POSIX lock set as protobuf Replace the hand-rolled fixed-width codec with a LockSetProto message, so the metadata blob can gain fields without a format-version migration. proto.Unmarshal already rejects a malformed blob, so the explicit version and length checks go away. Marshal now returns an error to match. |
||
|
Chris LuandGitHub
|
e8e7cd6fac |
filer: POSIX advisory lock set primitive (phase 1 of distributed FUSE locking) (#9660)
* filer: POSIX advisory lock set primitive (phase 1) Pure per-inode conflict/coalesce/range-split logic for fcntl byte-range and flock whole-file locks, extracted from the mount's PosixLockTable without its wait queue or inode-map concurrency. Owner identity is (Sid, Owner) so the same FUSE owner on different mounts never aliases, and ReleaseSession reaps a dead mount's locks. The owner filer will hold one Set per inode under the per-path lock; no concurrency control here. * test: tolerate transient FUSE invisibility in ConcurrentReadWrite A concurrent truncating overwrite leaves a short-lived dentry/cache window where the file is momentarily ENOENT to another opener. Retry the reads and writes a few times before failing, as ConcurrentDirectoryOperations does. |
||
|
Chris LuandGitHub
|
83195fc111 |
filer: reuse the caller's fetched entry in CreateEntry (#9645)
CreateEntry starts with a FindEntry to load the current entry. A conditional CreateEntry already fetched that entry to evaluate the precondition under the per-path lock, so the create repeated the lookup. Add an existing *Entry parameter: when non-nil it is used as the current entry and the internal lookup is skipped; nil keeps the lookup. The gRPC CreateEntry handler passes the entry it fetched for the precondition, removing the redundant read while the lock is held. All other callers pass nil. |
||
|
Chris LuandGitHub
|
3f6410fdc3 |
fix(redis3): prevent filer crash from inconsistent skiplist ends (#9602)
* fix(redis3): prevent filer crash from inconsistent skiplist ends DeleteByKey updated the two ends asymmetrically: the start side decided whether to clear StartLevels[index] by comparing a cached reference key, while the end side cleared EndLevels[index] structurally. The redis3 ItemList re-keys a node while keeping its id, so that cached key drifts. When such a node was the only one at a level and got deleted, the stale key comparison left StartLevels dangling while EndLevels was cleared to nil. The next InsertByKey then dereferenced a nil EndLevels[0] and took down the whole filer during a rename or delete. Match the deleted node by its unique id so both ends stay consistent, and guard each end in InsertByKey so an already-corrupted skiplist persisted in Redis self-heals instead of crashing on load. * fix(redis3): propagate errors from WriteName node split The case 2.3 split path returned nil instead of the error in seven branches. Because the split runs a multi-step sequence (DeleteByKey on the skiplist, ItemAdd, redis range-delete, ItemAdd), a swallowed failure let WriteName report success while the skiplist was half-updated, which the caller then persisted - silently corrupting the directory listing and setting up the very inconsistent-ends state that crashes the filer. |
||
|
Chris LuandGitHub
|
a17dca7009 |
fix(filer): don't disable the SQL idle connection pool when unconfigured (#9591)
* fix(filer): don't disable the SQL idle connection pool when unconfigured The mysql/mysql2/postgres stores called SetMaxIdleConns(maxIdle) unconditionally, so an unset connection_max_idle (0) actively kept zero idle connections - every query opened and closed a fresh connection instead of reusing the pool. Only apply the value when it's set; otherwise leave database/sql's default idle pool of 2 in place. * comments: shorten idle-pool note * fix(filer): default the SQL idle pool via config, keep explicit 0 honored Apply the idle-pool default at the config layer with SetDefault instead of guarding the SetMaxIdleConns call. An absent connection_max_idle now reads back as 2 (pool stays on), while an explicit 0 flows through to SetMaxIdleConns(0) so operators can still disable idle pooling on purpose. |
||
 
|
4476cb282b |
feat(filer): add atime to FuseAttributes + TouchAccessTime RPC (#9556)
* feat(filer): add atime field and TouchAccessTime RPC to filer proto
Introduce POSIX-style access-time tracking on the filer:
- FuseAttributes gains atime (field 22) and atime_ns (field 23).
- New TouchAccessTime RPC (and Touch{Access,Time}{Request,Response})
lets read paths bump atime without going through UpdateEntry's
chunk-rewrite/EqualEntry short-circuit.
Additive proto changes only; zero atime is treated as unset and
existing clients are unaffected. Java client proto is kept in lock
step.
Co-authored-by: Cursor <cursoragent@cursor.com>
* feat(filer): wire Atime through Attr codec with mtime fallback
Add Attr.Atime and round-trip it through EntryAttributeToPb /
EntryAttributeToExistingPb / PbToEntryAttribute. A zero proto atime
decodes as Mtime, so legacy entries report a sensible value and
freshly-created/updated entries default Atime to Mtime when callers
do not set it explicitly.
CreateEntry and UpdateEntry stamp Atime = Mtime (or Crtime) when it
is zero. TouchAccessTime later bypasses this path to write atime
alone via Store.UpdateEntry.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(filer): preserve atime in first epoch second on decode
The Atime decode branch previously treated any attr.Atime == 0 as
unset and overwrote it with Mtime, which drops valid timestamps in
the first second of the unix epoch where attr.Atime is 0 but
attr.AtimeNs > 0. Check both fields so we only fall back to Mtime
when both are zero.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Chris LuandGitHub
|
10cc06333b |
cluster: restrict Ping RPC to known peers of the requested type (#9445)
Ping previously dialled whatever host:port the caller asked for. Gate each server's Ping handler on cluster membership: masters check the topology, registered cluster nodes, and configured master peers; volume servers only accept their seed/current masters; filers accept tracked peer filers, the master-learned volume server set, and configured masters. Use address-indexed peer lookups to keep Ping target validation O(1): - topology maintains a pb.ServerAddress -> *DataNode index alongside the dc/rack/node tree, kept in sync from doLinkChildNode and UnlinkChildNode plus the ip/port-rewrite branch in GetOrCreateDataNode. GetTopology now returns nil on a detached subtree instead of panicking, so the linkage hooks can no-op safely. - vid_map tracks a refcount per volume-server address so hasVolumeServer answers without scanning every vid location. The add path skips empty-address entries the same way the delete path already does, so a zero-value Location cannot leak a permanent serverRefCount[""] bucket. - masters reuse a cached master-address set from MasterClient instead of walking the configured peer slice on every request. - volume servers compare against a pre-built seed-master set and protect currentMaster reads/writes with an RWMutex, fixing the data race with the heartbeat goroutine. The seed slice is copied on construction so external mutation cannot desync it from the frozen lookup set. - cluster.check drops the direct volume-to-volume sweep; volume servers no longer carry a peer-volume list, and the note next to the dropped probe is reworded to make clear that direct volume-to-volume reachability is intentionally not validated by this command. Update the volume-server integration tests that drove Ping through the new admission gate: success-path coverage now targets the master peer (the only type a volume server tracks), and the unknown/unreachable path asserts the InvalidArgument the gate now returns instead of the old downstream dial error. Mirror the same admission gate in the Rust volume server crate: a seed-master HashSet built once at startup plus a tokio RwLock over the heartbeat-tracked current master, both consulted in is_known_ping_target on every Ping, with InvalidArgument returned for any target that isn't a recognised master. |
||
|
Chris LuandGitHub
|
35e3fe89bc |
feat(s3/lifecycle): filer-backed cursor Persister + drop BlockerStore (#9358)
* feat(s3/lifecycle): filer-backed cursor Persister FilerPersister persists per-shard cursor maps as JSON to /etc/s3/lifecycle/cursors/shard-NN.json via filer.SaveInsideFiler. One file per shard keeps Save atomic — the filer writes the entry in a single mutation, so a crash mid-write doesn't leak partial state. Pipeline.Run loads on start; the periodic checkpoint and graceful-shutdown save go through this implementation. A small FilerStore interface wraps the SeaweedFilerClient surface the persister needs, so tests inject an in-memory fake instead of mocking the full gRPC client. * refactor(s3/lifecycle): drop BlockerStore — durable cursor IS the block A frozen cursor doesn't advance, so the durable cursor (FilerPersister) encodes the blocked state on its own. On worker restart the reader re-encounters the poison event at MinTsNs, the dispatcher walks the same retry budget to BLOCKED, and the cursor freezes at the same EventTs. Other in-flight events between freeze tsNs and prior cursor positions self-resolve via NOOP_RESOLVED (STALE_IDENTITY) since the underlying objects were already deleted on the prior pass. Removed: - BlockerStore interface + InMemoryBlockerStore + BlockerRecord - Dispatcher.Blockers + Dispatcher.ReplayBlockers - the BlockerStore.Put call in handleBlocked - Pipeline.Blockers field + the ReplayBlockers call on startup Added a TestDispatchRestartReFreezesNaturally that pins the self-recovery property: a fresh Dispatcher with a fresh Cursor, fed the same poison event, reaches the same frozen state at the same EventTs without any durable blocker store. Operator visibility: a cursor whose MinTsNs hasn't advanced is the signal — surfaced via the durable cursor file. * refactor(filer): SaveInsideFiler accepts ctx ReadInsideFiler already takes ctx; SaveInsideFiler used context.Background() internally and silently dropped the caller's ctx. Symmetric API now; cancellation/deadlines propagate through LookupEntry / CreateEntry / UpdateEntry. Mechanical update of all callers — most pass context.Background() since the existing call sites have no ctx in scope. * fix(s3/lifecycle): deterministic order in cursor save Iterating Go maps yields random order, so json.Encode produced a different byte sequence on each save even when the state hadn't changed. Sort entries by (Bucket, ActionKind, RuleHash) before encoding so the on-disk file diffs cleanly. New test pins byte-identical output across two saves of the same map. * fix(s3/lifecycle): log reason when freezing cursor in handleBlocked handleBlocked dropped the reason via _ = reason with a comment claiming the caller logged it; none of the three callers do. A frozen cursor is the only surface where the operator finds out something stuck, so the reason has to land somewhere. glog.Warningf with shard, key, eventTs, and the original reason — same shape the rest of the package uses. |
||
|
Chris LuandGitHub
|
0fa0a56a5a |
filer(mysql): TLS hostname/SNI knobs + MariaDB upsert documentation (#9260)
* refactor(filer/mysql): set tls.Config per-instance via Connector instead of global registry
Replace the use of `mysql.RegisterTLSConfig("mysql-tls", ...)` and the
`&tls=mysql-tls` DSN suffix with a per-instance setup that assigns the
`*tls.Config` directly to `mysql.Config.TLS` and opens the database via
`mysql.NewConnector` + `sql.OpenDB`.
The driver's TLS-config registry is process-wide; if a second `MysqlStore`
were ever initialized with different TLS settings (e.g., a filer plus a
separately configured store) the second registration would silently
overwrite the first. The connector pattern keeps the TLS configuration
attached to the connector and avoids that global side effect.
Behavior is otherwise unchanged: TLS is enabled when `enable_tls=true`,
the same `ca_crt`/`client_crt`/`client_key` knobs are honored, and the
TLS minimum version remains 1.2.
* filer(mysql): use system root CAs when ca_crt is empty
Previously, enabling `enable_tls=true` without setting `ca_crt` returned an
unhelpful empty-path read error. Many managed MySQL/MariaDB providers serve
certificates that chain to a public CA already in the host's trust store, so
requiring an explicit CA bundle adds friction with no security benefit.
Leave `RootCAs` unset when `ca_crt` is empty so Go's `tls.Config` falls back
to the system trust store, matching the standard behavior of `mysql --ssl`.
Existing setups with `ca_crt` configured are unaffected.
Also wraps the CA read/parse errors with the file path for easier diagnosis.
* filer(mysql): fail loudly when client_crt / client_key are unreadable
The previous implementation called `tls.LoadX509KeyPair` and silently
discarded any error, falling back to a non-mTLS connection. A typo or
permissions problem in `client_crt` / `client_key` therefore appeared as a
confusing server-side handshake error rather than as a config error,
because the server was expecting a client cert that the filer never sent.
Treat the keypair as required when either path is set, and surface the
underlying load error with both filenames so the misconfiguration is
obvious. The default (both paths empty) is unchanged: no client cert is
sent.
* filer(mysql): add tls_insecure_skip_verify and tls_server_name knobs
When the filer connects to a MySQL/MariaDB cluster whose server
certificate's SAN does not match the connection address (common with
internal load balancers, IP-only connection strings, or self-signed
cluster certs), the TLS handshake fails with `x509: certificate is valid
for X, not Y`. There was previously no way to fix this short of reissuing
the cert.
Expose two new optional knobs on `[mysql]`:
- `tls_server_name` overrides the SNI / cert hostname used for
verification — the standard fix when the cert SAN is correct but the
connection address is not.
- `tls_insecure_skip_verify` disables verification entirely as an escape
hatch for testing or for clusters with no usable SAN.
Both default to off, so existing configurations continue to verify the
server certificate against the connection address as before.
* docs(scaffold/filer.toml): document mysql TLS knobs and MariaDB upsert override
- Document the new `tls_insecure_skip_verify` and `tls_server_name` options.
- Update the `ca_crt` comment to reflect that it is optional and that the
system trust store is used when the path is empty (matches the runtime
behavior in mysql_store.go).
- Reword the client cert comments to make the mTLS pairing requirement
explicit (both `client_crt` and `client_key` must be set together).
- Add a commented-out MariaDB / MySQL 5.7 alternative for `upsertQuery`,
noting that the default (`AS new` row alias) requires MySQL 8.0.19+.
* filer(mysql): drop redundant blank import of go-sql-driver/mysql
The package was imported twice: once with the `mysql` alias (used for
`mysql.MySQLError`, `mysql.Config`, `mysql.NewConnector`, etc.) and once
as `_` to register the driver. The named import already triggers
`init()` and registers the driver, so the blank import is dead weight.
|
||
|
Chris LuandGitHub
|
c934b5dab6 |
fix(credential/postgres,s3api/iam): rename safety + pgxutil follow-ups to #9226 (#9230)
* refactor(util): extract pgx OpenDB + DSN builder into shared pgxutil
The postgres filer store had OpenPGXDB plus duplicated key=value DSN
assembly across postgres/ and postgres2/. Move the connection helper to
weed/util/pgxutil and add BuildDSN so the credential postgres store can
land on the same code path.
filer/postgres/pgx_conn.go keeps OpenPGXDB as a thin alias so postgres2
keeps building unchanged.
* refactor(credential/postgres): use shared pgxutil for connection setup
Replace the bespoke fmt.Sprintf DSN + sql.Open("pgx", ...) path with
pgxutil.BuildDSN + pgxutil.OpenDB so the credential store mirrors the
postgres filer store. This also drops the leaky RegisterConnConfig-style
init in favor of stdlib.OpenDB(*config), which doesn't accumulate
entries in the global pgx config map.
Adds parity knobs the filer store already exposes: sslcrl, and
configurable connection_max_idle / connection_max_open /
connection_max_lifetime_seconds (with the previous hardcoded 25/5/5min
as defaults). Also moves the jsonbParam helper here so other store
files can reuse it. (Helper is also referenced by postgres_identity.go,
which is migrated to it in the next commit.)
* refactor(credential/postgres): use jsonbParam helper across all writers
Consolidate JSONB write handling on the new pgxutil-adjacent helper
jsonbParam(b []byte) interface{}, which returns nil (driver writes SQL
NULL) when the marshaled JSON is empty and string(b) otherwise.
postgres_identity.go: replace the inline 'var fooParam any' /
'fooParam = string(b)' pattern with the helper. Same in CreateUser
and UpdateUser.
postgres_inline_policy.go, postgres_policy.go, postgres_service_account.go,
postgres_group.go: every JSONB writer was still passing []byte. Under
pgx simple_protocol (pgbouncer_compatible=true), []byte is encoded as
bytea and Postgres rejects that against a JSONB column with "invalid
input syntax for type json". Route them through jsonbParam too.
* fix(credential/postgres): rework SaveConfiguration to handle rename + UNIQUE access keys
The IAM rename path (s3api UpdateUser) renames an identity in place
and keeps its access keys. With the previous flow — upsert each user,
then per-user delete-and-insert credentials, then prune absent users —
the renamed user's access keys were still owned by the old row when
the INSERT for the new name ran, tripping credentials.access_key's
global UNIQUE constraint and failing every rename of a user with
credentials.
Reorder the SaveConfiguration body so the prune step runs BEFORE the
credential replace. CASCADE on the old user releases its access keys
in the same transaction, and the new name can then claim them.
While here:
- Replace the per-user loop DELETE FROM users WHERE username = $1 with
a single DELETE ... WHERE username = ANY($1), one round trip instead
of N inside the transaction.
- Surface inline-policy CASCADE losses: count user_inline_policies for
the prune set and emit a Warningf when the count is non-zero so
rename-driven drops are visible in operator logs (the structural
fix for renames lives at the IAM layer in a follow-up commit).
- Two-pass credential replace: clear credentials for every user we are
about to rewrite first, then insert, so an access key can be moved
between two users in the same SaveConfiguration call.
- credErr := credRows.Err() before credRows.Close() in
LoadConfiguration — Err() is documented as safe after Close, but
the leading-capture pattern matches the rest of the file.
* fix(s3api/iam): preserve inline policies when renaming a user
EmbeddedIamApi.UpdateUser renames an identity in place and the caller
persists via SaveConfiguration, which prunes the old username and
CASCADE-drops its rows from user_inline_policies. GetUserPolicy and
ListUserPolicies then return nothing under the new name even though
the API reported success — silent data loss.
Before flipping sourceIdent.Name, list the user's stored inline
policies and re-attach each one under the new name. The subsequent
SaveConfiguration prune still CASCADE-removes the old-name rows; only
the duplicates we just wrote under the new name survive. Adds a
regression test that puts a policy on the old name, renames, and
asserts the policy is readable under the new name.
* perf(credential/postgres): batch the credential clear in SaveConfiguration
The two-pass credential replace was clearing each incoming user's
credentials with its own DELETE statement — N round-trips inside the
transaction. Match the pattern already used for the user prune and
issue a single DELETE FROM credentials WHERE username = ANY($1)
instead.
* refactor(s3api/iam): plumb context through UpdateUser
UpdateUser was synthesizing a fresh context.Background() inside the
inline-policy migration block, which discards the request deadline,
cancellation, and tracing carried by the caller. Add ctx as the first
parameter and pass r.Context() in via the ExecuteAction dispatcher,
mirroring the signature already used by CreatePolicy /
AttachUserPolicy / DetachUserPolicy.
* fix(util/pgxutil): quote DSN values per libpq rules
BuildDSN was concatenating values directly, so any password / cert path
/ database name with a space, single quote, or backslash produced a
malformed connection string and pgx.ParseConfig either errored or
mis-parsed the remainder. Critical now that the helper is shared with
the credential store: mTLS deployments routinely sourcing passwords or
secret-mounted cert paths from a vault are exactly the case where
spaces and quotes show up.
Add quoteDSNValue: empty values and values containing whitespace, `'`,
or `\` are wrapped in single quotes with `'` and `\` escaped per
PostgreSQL libpq rules; plain alphanumeric values pass through
unchanged. Apply it to every variable field in BuildDSN.
Adds a test that round-trips a password containing spaces, quotes and
backslashes through pgx.ParseConfig and confirms the parsed Config
matches the input.
* fix(credential,s3api/iam): atomic UserRenamer to avoid FK violation on rename
The previous IAM rename path called PutUserInlinePolicy(newName, ...)
before SaveConfiguration created the new users row. user_inline_policies
has a non-deferrable FOREIGN KEY (username) REFERENCES users(username),
which Postgres validates at statement time, so every rename of a user
that owned at least one inline policy failed with an FK violation. The
existing memory-store regression test missed it because the memory
backend has no FK enforcement.
Add an optional credential.UserRenamer interface plus a
CredentialManager.RenameUser thin shim that returns (supported, err).
Implement it on PostgresStore as an atomic in-transaction migration:
INSERT the new users row by SELECT-copying from the old, UPDATE
credentials.username and user_inline_policies.username to the new
name (FK satisfied because both rows now exist), then DELETE the old
row. ErrUserNotFound / ErrUserAlreadyExists are surfaced cleanly.
Implement it on MemoryStore by re-binding store.users / store.accessKeys
/ store.inlinePolicies under the new name. Also fixes a small leak in
DeleteUser, which was forgetting to drop the user's inline-policy
bucket.
EmbeddedIamApi.UpdateUser now calls RenameUser first; if the store
implements the interface, that's the whole migration. If it doesn't
(stores without FK enforcement), fall back to the previous
list / get / put copy.
Adds a focused test for MemoryStore.RenameUser that asserts the
identity, the access-key index, and the inline policies all land
under the new name.
|
||
 Lars LehtonenandGitHub
|
8ae07e2a3f | chore(weed/filer/redis3): prune unused test functions (#9192) | ||
|
Chris LuandGitHub
|
e24a443b17 |
peer chunk sharing 2/8: filer mount registry (#9131)
* proto: define MountRegister/MountList and MountPeer service Adds the wire types for peer chunk sharing between weed mount clients: * filer.proto: MountRegister / MountList RPCs so each mount can heartbeat its peer-serve address into a filer-hosted registry, and refresh the list of peers. Tiny payload; the filer stores only O(fleet_size) state. * mount_peer.proto (new): ChunkAnnounce / ChunkLookup RPCs for the mount-to-mount chunk directory. Each fid's directory entry lives on an HRW-assigned mount; announces and lookups route to that mount. No behavior yet — later PRs wire the RPCs into the filer and mount. See design-weed-mount-peer-chunk-sharing.md for the full design. * filer: add mount-server registry behind -peer.registry.enable Implements tier 1 of the peer chunk sharing design: an in-memory registry of live weed mount servers, keyed by peer address, refreshed by MountRegister heartbeats and served by MountList. * weed/filer/peer_registry.go: thread-safe map with TTL eviction; lazy sweep on List plus a background sweeper goroutine for bounded memory. * weed/server/filer_grpc_server_peer.go: MountRegister / MountList RPC handlers. When -peer.registry.enable is false (the default), both RPCs are silent no-ops so probing older filers is harmless. * -peer.registry.enable flag on weed filer; FilerOption.PeerRegistryEnabled wires it through. Phase 1 is single-filer (no cross-filer replication of the registry); mounts that fail over to another filer will re-register on the next heartbeat, so the registry self-heals within one TTL cycle. Part of the peer-chunk-sharing design; no behavior change at runtime until a later PR enables the flag on both filer and mount. * filer: nil-safe peerRegistryEnable + registry hardening Addresses review feedback on PR #9131. * Fix: nil pointer deref in the mini cluster. FilerOptions instances constructed outside weed/command/filer.go (e.g. miniFilerOptions in mini.go) do not populate peerRegistryEnable, so dereferencing the pointer panics at Filer startup. Use the same `nil && deref` idiom already used for distributedLock / writebackCache. * Hardening (gemini review): registry now enforces three invariants: - empty peer_addr is silently rejected (no client-controlled sentinel mass-inserts) - TTL is capped at 1 hour so a runaway client cannot pin entries - new-entry count is capped at 10000 to bound memory; renewals of existing entries are always honored, so a full registry still heartbeats its existing members correctly Covered by new unit tests. * filer: rename -peer.registry.enable flag to -mount.p2p Per review feedback: the old name "peer.registry.enable" leaked the implementation ("registry") into the CLI surface. "mount.p2p" is shorter and describes what it actually controls — whether this filer participates in mount-to-mount peer chunk sharing. Flag renames (all three keep default=true, idle cost is near-zero): -peer.registry.enable -> -mount.p2p (weed filer) -filer.peer.registry.enable -> -filer.mount.p2p (weed mini, weed server) Internal variable names (mountPeerRegistryEnable, MountPeerRegistry) keep their longer form — they describe the component, not the knob. * filer: MountList returns DataCenter + List uses RLock Two review follow-ups on the mount peer registry: * weed/server/filer_grpc_server_mount_peer.go: MountList was dropping the DataCenter on the wire. The whole point of carrying DC separately from Rack is letting the mount-side fetcher re-rank peers by the two-level locality hierarchy (same-rack > same-DC > cross-DC); without DC in the response every remote peer collapsed to "unknown locality." * weed/filer/mount_peer_registry.go: List() was taking a write lock so it could lazy-delete expired entries inline. But MountList is a read-heavy RPC hit on every mount's 30 s refresh loop, and Sweep is already wired as the sole reclamation path (same pattern as the mount-side PeerDirectory). Switch List to RLock + filter, let Sweep do the map mutation, so concurrent MountList callers don't serialize on each other. Test updated to reflect the new contract (List no longer mutates the map; Sweep is what drops expired entries). |
||
|
Chris LuandGitHub
|
00a2e22478 |
fix(mount): remove fid pool to stop master over-allocating volumes (#9111)
* fix(mount): remove fid pool to stop master over-allocating volumes
The writeback-cache fid pool pre-allocated file IDs with
ExpectedDataSize = ChunkSizeLimit (typically 8+ MB). The master's
PickForWrite charges count * expectedDataSize against the volume's
effectiveSize, so a full pool refill could charge hundreds of MB
against a single volume before any bytes were actually written.
That tripped RecordAssign's hard-limit path and eagerly removed
volumes from writable, causing the master to grow new volumes
even when the real data being written was tiny.
Drop the pool entirely. Every chunk upload goes through
UploadWithRetry -> AssignVolume with no ExpectedDataSize hint,
letting the master fall back to the 1 MB default estimate. The
mount->filer grpc connection is already cached in pb.WithGrpcClient
(non-streaming mode), so per-chunk AssignVolume is a unary RPC
over an existing HTTP/2 stream, not a full dial. Path-based
filer.conf storage rules now apply to mount chunk assigns again,
which the pool had to skip.
Also remove the now-unused operation.UploadWithAssignFunc and its
AssignFunc type.
* fix(upload): populate ExpectedDataSize from actual chunk bytes
UploadWithRetry already buffers the full chunk into `data` before
calling AssignVolume, so the real size is known. Previously the
assign request went out with ExpectedDataSize=0, making the master
fall back to the 1 MB DefaultNeedleSizeEstimate per fid — same
over-reservation symptom the pool had, just smaller per call.
Stamp ExpectedDataSize = len(data) before the assign RPC when the
caller hasn't already set it. This covers mount chunk uploads,
filer_copy, filersink, mq/logstore, broker_write, gateway_upload,
and nfs — all the UploadWithRetry paths.
* fix(assign): pass real ExpectedDataSize at every assign call site
After removing the mount fid pool, per-chunk AssignVolume calls went
out with ExpectedDataSize=0, making the master fall back to its 1 MB
DefaultNeedleSizeEstimate. That's still an over-estimate for small
writes. Thread the real payload size through every remaining assign
site so RecordAssign charges effectiveSize accurately and stops
prematurely marking volumes full.
- filer: assignNewFileInfo now takes expectedDataSize and stamps it
on both primary and alternate VolumeAssignRequests. Callers pass:
- SSE data-to-chunk: len(data)
- copy manifest save: len(data)
- streamCopyChunk: srcChunk.Size
- TUS sub-chunk: bytes read
- saveAsChunk (autochunk/manifestize): 0 (small, size unknown
until the reader is drained; master uses 1 MB default)
- filer gRPC remote fetch-and-write: ExpectedDataSize = chunkSize
after the adaptive chunkSize is computed.
- ChunkedUploadOption.AssignFunc gains an expectedDataSize parameter;
upload_chunked.go passes the buffered dataSize at the call site.
S3 PUT assignFunc stamps it on the AssignVolumeRequest.
- S3 copy: assignNewVolume / prepareChunkCopy take expectedDataSize;
all seven call sites pass the source chunk's Size.
- operation.SubmitFiles / FilePart.Upload: derive per-fid size from
FileSize (average for batched requests, real per-chunk size for
sequential chunk assigns).
- benchmark: pass fileSize.
- filer append-to-file: pass len(data).
* fix(assign): thread size through SaveDataAsChunkFunctionType
The saveAsChunk path (autochunk, filer_copy, webdav, mount) ran
AssignVolume before the reader was drained, so it had to pass
ExpectedDataSize=0 and fall back to the master's 1 MB default.
Add an expectedDataSize parameter to SaveDataAsChunkFunctionType.
- mergeIntoManifest already has the serialized manifest bytes, so
it passes uint64(len(data)) directly.
- Mount's saveDataAsChunk ignores the parameter because it uses
UploadWithRetry, which already stamps len(data) on the assign
after reading the payload.
- webdav and filer_copy saveDataAsChunk follow the same UploadWithRetry
path and also ignore the hint.
- Filer's saveAsChunk (used for manifestize) plumbs the value to
assignNewFileInfo so manifest-chunk assigns get a real size.
Callers of saveFunc-as-value (weedfs_file_sync, dirty_pages_chunked)
pass the chunk size they're about to upload.
|
||
|
Chris LuandGitHub
|
886d50a6a5 |
feat(mount): singleflight dedup for concurrent chunk reads (#9100)
* feat(mount): add singleflight deduplication for concurrent chunk reads When multiple FUSE readers request the same uncached chunk concurrently, only one network fetch is performed. Other readers wait and share the downloaded data, reducing redundant volume server traffic under parallel read workloads. * fix(util): make singleflight panic-safe with defer cleanup If the provided function panics, the WaitGroup and map entry are now cleaned up via defer, preventing other waiters from hanging forever. * fix(filer): remove singleflight from reader_cache to fix buffer ownership The singleflight wrapper around chunk fetches returned the same []byte buffer to concurrent callers. Since each SingleChunkCacher owns and frees its data buffer in destroy(), sharing the same slice would cause a use-after-free or double-free with the mem allocator. The downloaders map already deduplicates in-flight downloads for the same fileId, so the singleflight was redundant at this layer. The SingleFlightGroup utility is retained for use elsewhere. |
||
|
Chris LuandGitHub
|
213b6c3107 |
fix(mount): count manifest sizes in merge condition to prevent accumulation (#9101)
* fix(mount): count manifest sizes in merge condition to prevent manifest accumulation shouldMergeChunks only counted non-manifest chunk sizes toward the bloat threshold, so overlapping manifests accumulated undetected across flush cycles. During sustained random writes (e.g. fio), each metadata flush compacts non-manifest chunks and may group them into a new manifest via MaybeManifestize, while carrying forward all existing manifests. Since the merge condition only checked non-manifest totals against 2x file size, the growing pile of redundant manifests never triggered a merge. In a real workload: 4 GB file, 25 manifests each covering ~4 GB (107 GB manifest data on volume servers), but shouldMergeChunks saw only 4.2 GB of non-manifest chunks vs the 8.6 GB threshold -- no merge. Fix: include manifest coverage sizes in totalChunkSize. This correctly detects the 111 GB total vs 8.6 GB threshold and triggers the merge, which re-reads the file as clean chunks and lets the filer's MinusChunks (which resolves manifests) garbage-collect all redundant sub-chunks. * test(mount,filer): add manifest chunk correctness tests Filer-level tests (filechunk_manifest_test.go): - TestManifestRoundTripPreservesChunks: create -> serialize -> deserialize preserves fileId, offset, size, and timestamp for all sub-chunks - TestCompactResolvedOverlappingManifests: two manifests covering the same range, older sub-chunks correctly identified as garbage after compaction - TestDoMinusChunksWithResolvedManifests: old manifest sub-chunks detected as garbage when compared against new clean chunks (mirrors filer cleanup) - TestManifestizeSmallBatchWithRemainder: batch=3 with 7 chunks produces 2 manifests + 1 remainder, all data recoverable after resolve - TestCompactMultipleOverlappingManifestGenerations: 5 generations of full-file manifests, compaction keeps only the newest generation - TestManifestBloatDetection: with N overlapping manifests, merge triggers at N >= 2 (stored > 2x file size) Mount-level test (weedfs_file_sync_test.go): - TestFlushCycleManifestAccumulation: simulates the flushMetadataToFiler pipeline over multiple cycles with a small manifest batch (5 chunks). Verifies merge triggers at cycle 2 when accumulated manifests exceed the 2x threshold. Uses SeparateManifestChunks + CompactFileChunks + shouldMergeChunks to mirror the real code path. |
||
|
Chris LuandGitHub
|
979c54f693 |
fix(wdclient,volume): compare master leader with ServerAddress.Equals (#9089)
* fix(wdclient,volume): compare master leader with ServerAddress.Equals Raft leader is advertised as host:httpPort.grpcPort, but clients dial host:httpPort. Raw string comparison against VolumeLocation.Leader / HeartbeatResponse.Leader therefore never matches, causing the masterclient and the volume server heartbeat loop to continuously "redirect" to the already-connected master, tearing down the stream and reconnecting. Use ServerAddress.Equals, which normalizes the grpc-port suffix. * fix(filer,mq): compare ServerAddress via Equals in two more sites filer bootstrap skip (MaybeBootstrapFromOnePeer) and the broker's local partition assignment check both compared a wire-supplied address string against the local self ServerAddress with raw string equality. Both are vulnerable to the same plain-vs-host:port.grpcPort mismatch as the masterclient/volume heartbeat sites: filer would bootstrap from itself, and the broker would fail to claim a partition it was actually assigned. Route both through ServerAddress.Equals. * fix(master,shell): more ServerAddress comparisons via Equals - raft_server_handlers.go HealthzHandler: s.serverAddr == leader would skip the child-lock check on the real leader when the two carry different plain/grpc-suffix forms, returning 200 OK instead of 423. - master_server.go SetRaftServer leader-change callback: the Leader() == Name() guard for ensureTopologyId could disagree with topology.IsLeader() (which already uses Equals), so leader-only initialization could be skipped after an election. - command_volume_merge.go isReplicaServer: the -target guard compared user-supplied host:port against NewServerAddressFromDataNode(...) with ==, letting an existing replica slip through when topology carries the embedded gRPC port. All routed through pb.ServerAddress.Equals. * fix(mq,cluster): more ServerAddress comparisons via Equals - broker_grpc_lookup.go GetTopicPublishers/GetTopicSubscribers: the partition ownership check gated listing on raw LeaderBroker == BrokerAddress().String(), so listings silently omitted partitions hosted locally when the assignment carried the other host:port / host:port.grpcPort form. - lock_client.go: LockHostMovedTo comparison and the seedFiler fallback guard both used raw string equality against configured filer addresses (which may be plain host:port while LockHostMovedTo comes back suffixed), causing spurious host-change churn and blocking the seed-filer fallback. * fix(mq): more ServerAddress comparisons via Equals - pub_balancer/allocate.go EnsureAssignmentsToActiveBrokers: direct activeBrokers.Get() lookup missed brokers when a persisted assignment carried a different address encoding than the registered broker key, triggering a bogus reassignment on every read/write cycle. Added a findActiveBroker helper that falls back to an Equals-based scan and canonicalizes the assignment in place so later writes are stable. - broker_grpc_lookup.go isLockOwner: used raw string equality between LockOwner() and BrokerAddress().String(), so a lock owner could fail to recognize itself and proxy local lookup/config/admin RPCs away. - pub_client/scheduler.go onEachAssignments: reused publisher jobs only on exact LeaderBroker match, so an encoding flip in lookup results tore down and recreated a stream to the same broker. |
||
|
Chris LuandGitHub
|
08d9193fe1 |
[nfs] Add NFS (#9067)
* add filer inode foundation for nfs
* nfs command skeleton
* add filer inode index foundation for nfs
* make nfs inode index hardlink aware
* add nfs filehandle and inode lookup plumbing
* add read-only nfs frontend foundation
* add nfs namespace mutation support
* add chunk-backed nfs write path
* add nfs protocol integration tests
* add stale handle nfs coverage
* complete nfs hardlink and failover coverage
* add nfs export access controls
* add nfs metadata cache invalidation
* fix nfs chunk read lookup routing
* fix nfs review findings and rename regression
* address pr 9067 review comments
- filer_inode: fail fast if the snowflake sequencer cannot start, and let
operators override the 10-bit node id via SEAWEEDFS_FILER_SNOWFLAKE_ID
to avoid multi-filer collisions
- filer_inode: drop the redundant retry loop in nextInode
- filerstore_wrapper: treat inode-index writes/removals as best-effort so
a primary store success no longer surfaces as an operation failure
- filer_grpc_server_rename: defer overwritten-target chunk deletion until
after CommitTransaction so a rolled-back rename does not strand live
metadata pointing at freshly deleted chunks
- command/nfs: default ip.bind to loopback and require an explicit
filer.path, so the experimental server does not expose the entire
filer namespace on first run
- nfs integration_test: document why LinkArgs matches go-nfs's on-the-wire
layout rather than RFC 1813 LINK3args
* mount: pre-allocate inode in Mkdir and Symlink
Mkdir and Symlink used to send filer_pb.CreateEntryRequest with
Attributes.Inode = 0. After PR 9067, the filer's CreateEntry now assigns
its own inode in that case, so the filer-side entry ends up with a
different inode than the one the mount allocates via inodeToPath.Lookup
and returns to the kernel. Once applyLocalMetadataEvent stores the
filer's entry in the meta cache, subsequent GetAttr calls read the
cached entry and hit the setAttrByPbEntry override at line 197 of
weedfs_attr.go, returning the filer-assigned inode instead of the
mount's local one. pjdfstest tests/rename/00.t (subtests 81/87/91)
caught this — it lstat'd a freshly-created directory/symlink, renamed
it, lstat'd again, and saw a different inode the second time.
createRegularFile already pre-allocates via inodeToPath.AllocateInode
and stamps it into the create request. Do the same thing in Mkdir and
Symlink so both sides agree on the object identity from the very first
request, and so GetAttr's cache path returns the same value as Mkdir /
Symlink's initial response.
* sequence: mask snowflake node id on int→uint32 conversion
CodeQL flagged the unchecked uint32(snowflakeId) cast in
NewSnowflakeSequencer as a potential truncation bug when snowflakeId is
sourced from user input (e.g. via SEAWEEDFS_FILER_SNOWFLAKE_ID). Mask
to the 10 bits the snowflake library actually uses so any caller-
supplied int is safely clamped into range.
* add test/nfs integration suite
Boots a real SeaweedFS cluster (master + volume + filer) plus the
experimental `weed nfs` frontend as subprocesses and drives it through
the NFSv3 wire protocol via go-nfs-client, mirroring the layout of
test/sftp. The tests run without a kernel NFS mount, privileged ports,
or any platform-specific tooling.
Coverage includes read/write round-trip, mkdir/rmdir, nested
directories, rename content preservation, overwrite + explicit
truncate, 3 MiB binary file, all-byte binary and empty files, symlink
round-trip, ReadDirPlus listing, missing-path remove, FSInfo sanity,
sequential appends, and readdir-after-remove.
Framework notes:
- Picks ephemeral ports with net.Listen("127.0.0.1:0") and passes
-port.grpc explicitly so the default port+10000 convention cannot
overflow uint16 on macOS.
- Pre-creates the /nfs_export directory via the filer HTTP API before
starting the NFS server — the NFS server's ensureIndexedEntry check
requires the export root to exist with a real entry, which filer.Root
does not satisfy when the export path is "/".
- Reuses the same rpc.Client for mount and target so go-nfs-client does
not try to re-dial via portmapper (which concatenates ":111" onto the
address).
* ci: add NFS integration test workflow
Mirror test/sftp's workflow for the new test/nfs suite so PRs that touch
the NFS server, the inode filer plumbing it depends on, or the test
harness itself run the 14 NFSv3-over-RPC integration tests on Ubuntu
22.04 via `make test`.
* nfs: use append for buffer growth in Write and Truncate
The previous make+copy pattern reallocated the full buffer on every
extending write or truncate, giving O(N^2) behaviour for sequential
write loops. Switching to `append(f.content, make([]byte, delta)...)`
lets Go's amortized growth strategy absorb the repeated extensions.
Called out by gemini-code-assist on PR 9067.
* filer: honor caller cancellation in collectInodeIndexEntries
Dropping the WithoutCancel wrapper lets DeleteFolderChildren bail out of
the inode-index scan if the client disconnects mid-walk. The cleanup is
already treated as best-effort by the caller (it logs on error and
continues), so a cancelled walk just means the partial index rebuild is
skipped — the same failure mode as any other index write error.
Flagged as a DoS concern by gemini-code-assist on PR 9067.
* nfs: skip filer read on open when O_TRUNC is set
openFile used to unconditionally loadWritableContent for every writable
open and then discard the buffer if O_TRUNC was set. For large files
that is a pointless 64 MiB round-trip. Reorder the branches so we only
fetch existing content when the caller intends to keep it, and mark the
file dirty right away so the subsequent Close still issues the
truncating write. Called out by gemini-code-assist on PR 9067.
* nfs: allow Seek on O_APPEND files and document buffered write cap
Two related cleanups on filesystem.go:
- POSIX only restricts Write on an O_APPEND fd, not lseek. The existing
Seek error ("append-only file descriptors may only seek to EOF")
prevented read-and-write workloads that legitimately reposition the
read cursor. Write already snaps the offset to EOF before persisting
(see seaweedFile Write), so Seek can unconditionally accept any
offset. Update the unit test that was asserting the old behaviour.
- Add a doc comment on maxBufferedWriteSize explaining that it is a
per-file ceiling, the memory footprint it implies, and that the real
fix for larger whole-file rewrites is streaming / multi-chunk support.
Both changes flagged by gemini-code-assist on PR 9067.
* nfs: guard offset before casting to int in Write
CodeQL flagged `int(f.offset) + len(p)` inside the Write growth path as
a potential overflow on architectures where `int` is 32-bit. The
existing check only bounded the post-cast value, which is too late.
Clamp f.offset against maxBufferedWriteSize before the cast and also
reject negative/overflowed endOffset results. Both branches fall
through to billy.ErrNotSupported, the same behaviour the caller gets
today for any out-of-range buffered write.
* nfs: compute Write endOffset in int64 to satisfy CodeQL
The previous guard bounded f.offset but left len(p) unchecked, so
CodeQL still flagged `int(f.offset) + len(p)` as a possible int-width
overflow path. Bound len(p) against maxBufferedWriteSize first, do the
addition in int64, and only cast down after the total has been clamped
against the buffer ceiling. Behaviour is unchanged: any out-of-range
write still returns billy.ErrNotSupported.
* ci: drop emojis from nfs-tests workflow summary
Plain-text step summary per user preference — no decorative glyphs in
the NFS CI output or checklist.
* nfs: annotate remaining DEV_PLAN TODOs with status
Three of the unchecked items are genuine follow-up PRs rather than
missing work in this one, and one was actually already done:
- Reuse chunk cache and mutation stream helpers without FUSE deps:
checked off — the NFS server imports weed/filer.ReaderCache and
weed/util/chunk_cache directly with no weed/mount or go-fuse imports.
- Extract shared read/write helpers from mount/WebDAV/SFTP: annotated
as deferred to a separate refactor PR (touches four packages).
- Expand direct data-path writes beyond the 64 MiB buffered fallback:
annotated as deferred — requires a streaming WRITE path.
- Shared lock state + lock tests: annotated as blocked upstream on
go-nfs's missing NLM/NFSv4 lock state RPCs, matching the existing
"Current Blockers" note.
* test/nfs: share port+readiness helpers with test/testutil
Drop the per-suite mustPickFreePort and waitForService re-implementations
in favor of testutil.MustAllocatePorts (atomic batch allocation; no
close-then-hope race) and testutil.WaitForPort / SeaweedMiniStartupTimeout.
Pull testutil in via a local replace directive so this standalone
seaweedfs-nfs-tests module can import the in-repo package without a
separate release.
Subprocess startup is still master + volume + filer + nfs — no switch to
weed mini yet, since mini does not know about the nfs frontend.
* nfs: stream writes to volume servers instead of buffering the whole file
Before this change the NFS write path held the full contents of every
writable open in memory:
- OpenFile(write) called loadWritableContent which read the existing
file into seaweedFile.content up to maxBufferedWriteSize (64 MiB)
- each Write() extended content in-place
- Close() uploaded the whole buffer as a single chunk via
persistContent + AssignVolume
The 64 MiB ceiling made large NFS writes return NFS3ERR_NOTSUPP, and
even below the cap every Write paid a whole-file-in-memory cost. This
PR rewrites the write path to match how `weed filer` and the S3 gateway
persist data:
- openFile(write) no longer loads the existing content at all; it
only issues an UpdateEntry when O_TRUNC is set *and* the file is
non-empty (so a fresh create+trunc is still zero-RPC)
- Write() streams the caller's bytes straight to a volume server via
one AssignVolume + one chunk upload, then atomically appends the
resulting chunk to the filer entry through mutateEntry. Any
previously inlined entry.Content is migrated to a chunk in the same
update so the chunk list becomes the authoritative representation.
- Truncate() becomes a direct mutateEntry (drop chunks past the new
size, clip inline content, update FileSize) instead of resizing an
in-memory buffer.
- Close() is a no-op because everything was flushed inline.
The small-file fast path that the filer HTTP handler uses is preserved:
if the post-write size still fits in maxInlineWriteSize (4 MiB) and
the file has no existing chunks, we rewrite entry.Content directly and
skip the volume-server round-trip. This keeps single-shot tiny writes
(echo, small edits) cheap while completely removing the 64 MiB cap on
larger files. Read() now always reads through the chunk reader instead
of a local byte slice, so reads inside the same session see the freshly
appended data.
Drops the unused seaweedFile.content / dirty fields, the
maxBufferedWriteSize constant, and the loadWritableContent helper.
Updates TestSeaweedFileSystemSupportsNamespaceMutations expectations
to match the new "no extra O_TRUNC UpdateEntry on an empty file"
behavior (still 3 updates: Write + Chmod + Truncate).
* filer: extract shared gateway upload helper for NFS and WebDAV
Three filer-backed gateways (NFS, WebDAV, and mount) each had a local
saveDataAsChunk that wrapped operation.NewUploader().UploadWithRetry
with near-identical bodies: build AssignVolumeRequest, build
UploadOption, build genFileUrlFn with optional filerProxy rewriting,
call UploadWithRetry, validate the result, and call ToPbFileChunk.
Pull that body into filer.SaveGatewayDataAsChunk with a
GatewayChunkUploadRequest struct so both NFS and WebDAV can delegate
to one implementation.
- NFS's saveDataAsChunk is now a thin adapter that assembles the
GatewayChunkUploadRequest from server options and calls the helper.
The chunkUploader interface keeps working for test injection because
the new GatewayChunkUploader interface is structurally identical.
- WebDAV's saveDataAsChunk is similarly a thin adapter — it drops the
local operation.NewUploader call plus the AssignVolume/UploadOption
scaffolding.
- mount is intentionally left alone. mount's saveDataAsChunk has two
features that do not fit the shared helper (a pre-allocated file-id
pool used to skip AssignVolume entirely, and a chunkCache
write-through at offset 0 so future reads hit the mount's local
cache), both of which are mount-specific.
Marks the Phase 2 "extract shared read/write helpers from mount,
WebDAV, and SFTP" DEV_PLAN item as done. The filer-level chunk read
path (NonOverlappingVisibleIntervals + ViewFromVisibleIntervals +
NewChunkReaderAtFromClient) was already shared.
* nfs: remove DESIGN.md and DEV_PLAN.md
The planning documents have served their purpose — all phase 1 and
phase 2 items are landed, phase 3 streaming writes are landed, phase 2
shared helpers are extracted, and the two remaining phase 4 items
(shared lock state + lock tests) are blocked upstream on
github.com/willscott/go-nfs which exposes no NLM or NFSv4 lock state
RPCs. The running decision log no longer reflects current code and
would just drift. The NFS wiki page
(https://github.com/seaweedfs/seaweedfs/wiki/NFS-Server) now carries
the overview, configuration surface, architecture notes, and known
limitations; the source is the source of truth for the rest.
|
||
|
Chris LuandGitHub
|
edf7d2a074 |
fix(filer): eliminate redundant disk reads causing memory/CPU regression (#9039)
* fix(filer): eliminate redundant disk reads causing memory/CPU regression (#9035) Since 4.18, LocalMetaLogBuffer's ReadFromDiskFn was set to readPersistedLogBufferPosition, causing LoopProcessLogData to call ReadPersistedLogBuffer on every 250ms health-check tick when a subscriber encounters ResumeFromDiskError. Each call creates an OrderedLogVisitor (ListDirectoryEntries on the filer store), spawns a readahead goroutine with a 1024-element channel, finds no data, and returns — 4 times per second even on an idle filer. This is redundant because SubscribeLocalMetadata already manages disk reads explicitly with its own shouldReadFromDisk / lastCheckedFlushTsNs tracking in the outer loop. Set ReadFromDiskFn back to nil for LocalMetaLogBuffer. When LoopProcessLogData encounters ResumeFromDiskError with nil ReadFromDiskFn, the HasData() guard returns ResumeFromDiskError to the caller (SubscribeLocalMetadata), which blocks efficiently on listenersCond.Wait() instead of polling. * fix(filer): add gap detection for slow consumers after disk-read stall When a slow consumer falls behind and LoopProcessLogData returns ResumeFromDiskError with no flush or read-position progress, there may be a gap between persisted data and in-memory data (e.g. writes stopped while consumer was still catching up). Without this, the consumer would block on listenersCond.Wait() forever. Skip forward to the earliest in-memory time to resume progress, matching the gap-handling pattern already used in the shouldReadFromDisk path. * fix(filer): clear stale ResumeFromDiskError after gap-skip to avoid stall The gap-detection block added in the previous commit skips lastReadTime forward to GetEarliestTime() and continues the outer loop. On the next iteration, shouldReadFromDisk becomes true (currentReadTsNs > lastDiskReadTsNs), the disk read returns processedTsNs == 0, and the existing gap handler at the top of the loop runs its own gap check. That check uses readInMemoryLogErr == ResumeFromDiskError as the entry condition — but readInMemoryLogErr is still the stale error from two iterations ago. GetEarliestTime() now equals lastReadTime.Time (we already advanced to it), so earliestTime.After(lastReadTime.Time) is false and the handler falls into listenersCond.Wait() — stuck. Clear readInMemoryLogErr at the gap-skip point, matching the existing pattern at the earlier gap handler that already clears it for the same reason. * fix(log_buffer): GetEarliestTime must include sealed prev buffers GetEarliestTime previously returned only logBuffer.startTime (the active buffer's first timestamp). That is narrower than ReadFromBuffer's tsMemory, which is the min across active + prev buffers. Callers using GetEarliestTime for gap detection after ResumeFromDiskError (the SubscribeLocalMetadata outer loop's disk-read path, the new gap-skip in the in-memory ResumeFromDiskError handler, and MQ HasData) saw a time that was *newer* than the real earliest in-memory data. Impact in SubscribeLocalMetadata's slow-consumer path: - tsMemory = earliest prev buffer time (T_prev) - GetEarliestTime() = active startTime (T_active, later than T_prev) - Consumer position = T1, with T_prev < T1 < T_active - ReadFromBuffer returns ResumeFromDiskError (T1 < tsMemory) - Gap detect: GetEarliestTime().After(T1) = T_active.After(T1) = true - Skip forward to T_active -- silently drops the prev-buffer data - And when T_active happens to equal the stuck position, gap detect evaluates false, and the subscriber stalls on listenersCond.Wait() This reproduces the TestMetadataSubscribeSlowConsumerKeepsProgressing failure in CI where the consumer stalled at 10220/20000 after writing stopped -- the buffer still had data in prev[0..3], but gap detection was comparing against the active buffer's startTime. Fix: scan all sealed prev buffers under RLock, return the true minimum startTime. Matches the min-of-buffers logic in ReadFromBuffer. * test(log_buffer): make DiskReadRetry test deterministic The previous test added the message via AddToBuffer + ForceFlush and relied on a race: the second disk read had to happen before the data was delivered through the in-memory path. Under the race detector or on a slow CI runner, the reader is woken by AddToBuffer's notification, finds the data in the active buffer or its prev slot, and returns after exactly one disk read — failing the >= 2 disk reads assertion even though the loop behaved correctly. Reproduced on master with race detector (2/5 failures). Rewrite the test to deliver the data exclusively through the disk-read path: no AddToBuffer, no ForceFlush. The test waits until the reader has issued at least one no-op disk read, then atomically flips a "dataReady" flag. The reader's next iteration through readFromDiskFn returns the entry. This deterministically exercises the retry-loop behavior the test was originally written to protect, and removes the in-memory delivery race entirely. |
||
 Chris Lu
|
e648c76bcf | go fmt | ||
|
Chris LuandGitHub
|
2a7ec8d033 |
fix(filer): do not abort entry deletion when hard link cleanup fails (#9022)
When unlinking a hard-linked file, DeleteOneEntry and DeleteEntry both called DeleteHardLink before removing the directory entry from the store. If DeleteHardLink returned an error (e.g. KV storage issue, decode failure), the function returned early without deleting the directory entry itself. This left a stale entry in the filer store, causing subsequent rmdir to fail with ENOTEMPTY. Change both functions to log the hard link cleanup error and continue to delete the directory entry regardless. This ensures the parent directory can always be removed after all its children are unlinked. Remove tests/unlink/14.t from the pjdfstest known failures list since this fix addresses the root cause. |
||
|
Chris LuandGitHub
|
de5b6f2120 |
fix(filer,mount): add nanosecond timestamp precision (#9019)
* fix(filer,mount): add nanosecond timestamp precision Add mtime_ns and ctime_ns fields to the FuseAttributes protobuf message to store the nanosecond component of timestamps (0-999999999). Previously timestamps were truncated to whole seconds. - Update EntryAttributeToPb/PbToEntryAttribute to encode/decode ns - Update setAttrByPbEntry/setAttrByFilerEntry to set Mtimensec/Ctimensec - Update in-memory atime map to store time.Time (preserves nanoseconds) - Remove tests/utimensat/08.t from known_failures.txt (all 9 subtests pass) * fix: sync nanosecond fields on all mtime/ctime write paths Ensure MtimeNs/CtimeNs are updated alongside Mtime/Ctime in all code paths: truncate, flush, link, copy_range, metadata flush, and directory touch. * fix: set ctime/ctime_ns in copy_range and metadata flush paths |
||
|
Chris LuandGitHub
|
3f36846642 |
fix(filer): update hard link ctime when nlink changes on unlink (#9018)
* fix(filer): update hard link ctime when nlink changes on unlink When a hard link is unlinked, POSIX requires that the remaining links' ctime is updated because the inode's nlink count changed. The filer's DeleteHardLink() decremented the counter in the KV store but did not update the ctime field. Set ctime to time.Now() on the KV entry before writing it back when the hard link counter is decremented but still > 0. Remove tests/unlink/00.t from known_failures.txt (all 112 subtests now pass). * style: use time.Now().UTC() for ctime in DeleteHardLink |
||
|
Chris LuandGitHub
|
bf31f404bc |
test: add pjdfstest POSIX compliance suite (#9013)
* test: add pjdfstest POSIX compliance suite Adds a script and CI workflow that runs the upstream pjdfstest POSIX compliance test suite against a SeaweedFS FUSE mount. The script starts a self-contained `weed mini` server, mounts the filesystem with `weed mount`, builds pjdfstest from source, and runs it under prove(1). * fix: address review feedback on pjdfstest setup - Use github.ref instead of github.head_ref in concurrency group so push events get a stable group key - Add explicit timeout check after filer readiness polling loop - Refresh pjdfstest checkout when PJDFSTEST_REPO or PJDFSTEST_REF are overridden instead of silently reusing stale sources * test: add Docker-based pjdfstest for faster iteration Adds a docker-compose setup that reuses the existing e2e image pattern: - master, volume, filer services from chrislusf/seaweedfs:e2e - mount service extended with pjdfstest baked in (Dockerfile extends e2e) - Tests run via `docker compose exec mount /run.sh` - CI workflow gains a parallel `pjdfstest (docker)` job This avoids building Go from scratch on each iteration — just rebuild the e2e image once and iterate on the compose stack. * fix: address second round of review feedback - Use mktemp for WORK_DIR so each run starts with a clean filer state - Pin PJDFSTEST_REF to immutable commit (03eb257) instead of master - Use cp -r instead of cp -a to avoid preserving ownership during setup * fix: address CI failure and third round of review feedback - Fix docker job: fall back to plain docker build when buildx cache export is not supported (default docker driver in some CI runners) - Use /healthz endpoint for filer healthcheck in docker-compose - Copy logs to a fixed path (/tmp/seaweedfs-pjdfstest-logs/) for reliable CI artifact upload when WORK_DIR is a mktemp path * fix(mount): improve POSIX compliance for FUSE mount Address several POSIX compliance gaps surfaced by the pjdfstest suite: 1. Filename length limit: reduce from 4096 to 255 bytes (NAME_MAX), returning ENAMETOOLONG for longer names. 2. SUID/SGID clearing on write: clear setuid/setgid bits when a non-root user writes to a file (POSIX requirement). 3. SUID/SGID clearing on chown: clear setuid/setgid bits when file ownership changes by a non-root user. 4. Sticky bit enforcement: add checkStickyBit helper and enforce it in Unlink, Rmdir, and Rename — only file owner, directory owner, or root may delete entries in sticky directories. 5. ctime (inode change time) tracking: add ctime field to the FuseAttributes protobuf message and filer.Attr struct. Update ctime on all metadata-modifying operations (SetAttr, Write/flush, Link, Create, Mkdir, Mknod, Symlink, Truncate). Fall back to mtime for backward compatibility when ctime is 0. * fix: add -T flag to docker compose exec for CI Disable TTY allocation in the pjdfstest docker job since GitHub Actions runners have no interactive TTY. * fix(mount): update parent directory mtime/ctime on entry changes POSIX requires that a directory's st_mtime and st_ctime be updated whenever entries are created or removed within it. Add touchDirMtimeCtime() helper and call it after: - mkdir, rmdir - create (including deferred creates), mknod, unlink - symlink, link - rename (both source and destination directories) This fixes pjdfstest failures in mkdir/00, mkfifo/00, mknod/00, mknod/11, open/00, symlink/00, link/00, and rmdir/00. * fix(mount): enforce sticky bit on destination directory during rename POSIX requires sticky-bit enforcement on both source and destination directories during rename. When the destination directory has the sticky bit set and a target entry already exists, only the file owner, directory owner, or root may replace it. * fix(mount): add in-memory atime tracking for POSIX compliance Track atime separately from mtime using a bounded in-memory map (capped at 8192 entries with random eviction). atime is not persisted to the filer — it's only kept in mount memory to satisfy POSIX stat requirements for utimensat and related syscalls. This fixes utimensat/00, utimensat/02, utimensat/04, utimensat/05, and utimensat/09 pjdfstest failures where atime was incorrectly aliased to mtime. * fix(mount): restore long filename support, fix permission checks - Restore 4096-byte filename limit (was incorrectly reduced to 255). SeaweedFS stores names as protobuf strings with no ext4-style constraint — the 255 limit is not applicable. - Fix AcquireHandle permission check to map filer uid/gid to local space before calling hasAccess, matching the pattern used in Access(). - Fix hasAccess fallback when supplementary group lookup fails: fall through to "other" permissions instead of requiring both group AND other to match, which was overly restrictive for non-existent UIDs. * fix(mount): fix permission checks and enforce NAME_MAX=255 - Fix AcquireHandle to map uid/gid from filer-space to local-space before calling hasAccess, consistent with the Access handler. - Fix hasAccess fallback when supplementary group lookup fails: use "other" permissions only instead of requiring both group AND other. - Enforce NAME_MAX=255 with a comment explaining the Linux FUSE kernel module's VFS-layer limit. Files >255 bytes can be created via direct FUSE protocol calls but can't be stat'd/chmod'd via normal syscalls. - Don't call touchDirMtimeCtime for deferred creates to avoid invalidating the just-cached entry via filer metadata events. * ci: mark pjdfstest steps as continue-on-error The pjdfstest suite has known failures (Linux FUSE NAME_MAX=255 limitation, hard link nlink/ctime tracking, nanosecond precision) that cannot be fixed in the mount layer. Mark the test steps as continue-on-error so the CI job reports results without blocking. * ci: increase pjdfstest bare metal timeout to 90 minutes * fix: use full commit hash for PJDFSTEST_REF in run.sh Short hashes cannot be resolved by git fetch --depth 1 on shallow clones. Use the full 40-char SHA. * test: add pjdfstest known failures skip list Add known_failures.txt listing 33 test files that cannot pass due to: - Linux FUSE kernel NAME_MAX=255 (26 files) - Hard link nlink/ctime tracking requiring filer changes (3 files) - Parent dir mtime on deferred create (1 file) - Directory rename permission edge case (1 file) - rmdir after hard link unlink (1 file) - Nanosecond timestamp precision (1 file) Both run.sh and run_inside_container.sh now skip these tests when running the full suite. Any failure in a non-skipped test will cause CI to fail, catching regressions immediately. Remove continue-on-error from CI steps since the skip list handles known failures. Result: 204 test files, 8380 tests, all passing. * ci: remove bare metal pjdfstest job, keep Docker only The bare metal job consistently gets stuck past its timeout due to weed processes not exiting cleanly. The Docker job covers the same tests reliably and runs faster. |
||
|
Chris LuandGitHub
|
546f255b46 |
fix(filer/postgres): use pgx v5 API for PgBouncer simple protocol (#9010)
* fix(filer/postgres): use pgx v5 API for PgBouncer simple protocol
In pgx/v5 the `prefer_simple_protocol` DSN parameter was removed, so
appending it to the connection string caused PgBouncer/PostgreSQL to
reject it as an unknown startup parameter:
FATAL: unsupported startup parameter: prefer_simple_protocol (SQLSTATE 08P01)
Parse the DSN with pgx.ParseConfig and, when pgbouncer_compatible is
set, configure DefaultQueryExecMode = QueryExecModeSimpleProtocol and
disable the statement/description caches. Register the config via
stdlib.RegisterConnConfig before sql.Open.
Fixes #9005
* refactor(filer/postgres): extract shared OpenPGXDB helper with cleanup
Extract the pgx v5 ParseConfig/RegisterConnConfig/sql.Open/Ping logic
into a shared postgres.OpenPGXDB helper used by both postgres and
postgres2 filer stores, eliminating ~60 lines of duplication.
The helper also unregisters the conn config via stdlib.UnregisterConnConfig
on every failure path (sql.Open error, Ping error) so we do not leak
entries in stdlib's global connection config map when initialization
fails.
* refactor(filer/postgres): use stdlib.OpenDB to avoid conn config leak
Switch OpenPGXDB from RegisterConnConfig + sql.Open("pgx", connStr) to
stdlib.OpenDB(*connConfig). The former leaks an entry in stdlib's global
conn config map on every successful initialization; stdlib.OpenDB takes
the config directly and keeps no global registration.
Addresses CodeRabbit review feedback on #9010.
|
||
|
Chris LuandGitHub
|
75dcb97187 |
filer: bootstrap pre-existing metadata when a new filer joins (#8979)
* filer: bootstrap pre-existing metadata when a new filer joins a cluster When a filer connects to a peer for the first time (no stored sync offset), it now does a full BFS traversal of the peer's metadata via TraverseBfsMetadata before starting the incremental change stream. This ensures filer2 sees all data that existed before it started, fixing the issue where only post-startup changes were synced. Closes #8961 * filer: upsert during bootstrap and persist offset immediately - Use upsert (insert, then update on conflict) during metadata traversal so the bootstrap doesn't fail on the root directory or after a partial previous attempt. - Persist the sync offset right after a successful traversal so a retry doesn't redo the full BFS. * filer: address review feedback on metadata bootstrap - Use peer-side max Mtime as the streaming cursor instead of local time.Now() to avoid missing events due to clock skew between filers. traversePeerMetadata now returns the high-water Mtime (nanoseconds) observed during BFS traversal. - Compare Mtime before overwriting during bootstrap: if a local entry is newer than the peer's version, skip the update instead of clobbering it. - Only trigger full BFS traversal on ErrKvNotFound (key genuinely missing). Transient KvGet errors (connection issues, etc.) are now propagated instead of silently falling through to a full re-sync. Changed readOffset to use %w so errors.Is works through the chain. * filer: address review findings on bootstrap sync - Use wall-clock time with safety margin for stream cursor instead of entry Mtime. Mtime is file modification time (can be arbitrary), while the metadata stream uses TsNs (event log time). Using time.Now() minus 1 minute before traversal ensures no events are missed even with clock skew, matching the proven filer.meta.backup pattern. - Pass ExcludedPrefixes=[SystemLogDir] to TraverseBfsMetadata so the server prunes internal log entries server-side instead of transferring them over the network only to be filtered client-side. - Fail fast if updateOffset fails after bootstrap. If we can't persist the offset, bail out rather than proceeding and potentially losing the expensive BFS work on the next retry. |
||
|
|
df619ec3f6 |
fix(weed/filer/redis2): fix dropped error (#8952)
* fix(weed/filer/redis2): fix dropped error * fix(weed/filer/redis2): break on non-ErrNotFound errors in ListDirectoryEntries Without the break, a hard FindEntry error gets overwritten by subsequent iterations and the function may return nil, silently losing the error. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
a4753b6a3b |
S3: delay empty folder cleanup to prevent Spark write failures (#8970)
* S3: delay empty folder cleanup to prevent Spark write failures (#8963) Empty folders were being cleaned up within seconds, causing Apache Spark (s3a) writes to fail when temporary directories like _temporary/0/task_xxx/ were briefly empty. - Increase default cleanup delay from 5s to 2 minutes - Only process queue items that have individually aged past the delay (previously the entire queue was drained once any item triggered) - Make the delay configurable via filer.toml: [filer.options] s3.empty_folder_cleanup_delay = "2m" * test: increase cleanup wait timeout to match 2m delay The empty folder cleanup delay was increased to 2 minutes, so the Spark integration test needs to wait longer for temporary directories to disappear. * fix: eagerly clean parent directories after empty folder deletion After deleting an empty folder, immediately try to clean its parent rather than relying on cascading metadata events that each re-enter the 2-minute delay queue. This prevents multi-minute waits when cleaning nested temporary directory trees (e.g. Spark's _temporary hierarchy with 3+ levels would take 6m+ vs near-instant). Fixes the CI failure where lingering _temporary parent directories were not cleaned within the test's 3-minute timeout. |
||
|
Chris LuandGitHub
|
0798b274dd |
feat(s3): add concurrent chunk prefetch for large file downloads (#8917)
* feat(s3): add concurrent chunk prefetch for large file downloads Add a pipe-based prefetch pipeline that overlaps chunk fetching with response writing during S3 GetObject, SSE downloads, and filer proxy. While chunk N streams to the HTTP response, fetch goroutines for the next K chunks establish HTTP connections to volume servers ahead of time, eliminating the RTT gap between sequential chunk fetches. Uses io.Pipe for minimal memory overhead (~1MB per download regardless of chunk size, vs buffering entire chunks). Also increases the streaming read buffer from 64KB to 256KB to reduce syscall overhead. Benchmark results (64KB chunks, prefetch=4): - 0ms latency: 1058 → 2362 MB/s (2.2× faster) - 5ms latency: 11.0 → 41.7 MB/s (3.8× faster) - 10ms latency: 5.9 → 23.3 MB/s (4.0× faster) - 20ms latency: 3.1 → 12.1 MB/s (3.9× faster) * fix: address review feedback for prefetch pipeline - Fix data race: use *chunkPipeResult (pointer) on channel to avoid copying struct while fetch goroutines write to it. Confirmed clean with -race detector. - Remove concurrent map write: retryWithCacheInvalidation no longer updates fileId2Url map. Producer only reads it; consumer never writes. - Use mem.Allocate/mem.Free for copy buffer to reduce GC pressure. - Add local cancellable context so consumer errors (client disconnect) immediately stop the producer and all in-flight fetch goroutines. * fix(test): remove dead code and add Range header support in test server - Remove unused allData variable in makeChunksAndServer - Add Range header handling to createTestServer for partial chunk read coverage (206 Partial Content, 416 Range Not Satisfiable) * fix: correct retry condition and goroutine leak in prefetch pipeline - Fix retry condition: use result.fetchErr/result.written instead of copied to decide cache-invalidation retry. The old condition wrongly triggered retry when the fetch succeeded but the response writer failed on the first write (copied==0 despite fetcher having data). Now matches the sequential path (stream.go:197) which checks whether the fetcher itself wrote zero bytes. - Fix goroutine leak: when the producer's send to the results channel is interrupted by context cancellation, the fetch goroutine was already launched but the result was never sent to the channel. The drain loop couldn't handle it. Now waits on result.done before returning so every fetch goroutine is properly awaited. |
||
|
Chris LuandGitHub
|
36f37b9b6a |
fix(filer): remove cancellation guard from RollbackTransaction and clean up #8909 (#8916)
* fix(filer): remove cancellation guard from RollbackTransaction and clean up #8909 RollbackTransaction is a cleanup operation that must succeed even when the context is cancelled — guarding it causes the exact orphaned state that #8909 was trying to prevent. Also: - Use single-evaluation `if err := ctx.Err(); err != nil` pattern instead of double-calling ctx.Err() - Remove spurious blank lines before guards - Add context.DeadlineExceeded test coverage - Simplify tests from ~230 lines to ~130 lines * fix(filer): call cancel() in expiredCtx and test rollback with expired context - Call cancel() instead of suppressing it to avoid leaking timer resources - Test RollbackTransaction with both cancelled and expired contexts |
||
 os-pradipbabarandGitHub
|
d5128f00f1 |
fix: Prevent orphaned metadata from cancelled S3 operations (Issue #8908) (#8909)
fix(filer): check if context was already cancelled before ignoring cancellation |
||
|
Chris LuandGitHub
|
995dfc4d5d |
chore: remove ~50k lines of unreachable dead code (#8913)
* chore: remove unreachable dead code across the codebase Remove ~50,000 lines of unreachable code identified by static analysis. Major removals: - weed/filer/redis_lua: entire unused Redis Lua filer store implementation - weed/wdclient/net2, resource_pool: unused connection/resource pool packages - weed/plugin/worker/lifecycle: unused lifecycle plugin worker - weed/s3api: unused S3 policy templates, presigned URL IAM, streaming copy, multipart IAM, key rotation, and various SSE helper functions - weed/mq/kafka: unused partition mapping, compression, schema, and protocol functions - weed/mq/offset: unused SQL storage and migration code - weed/worker: unused registry, task, and monitoring functions - weed/query: unused SQL engine, parquet scanner, and type functions - weed/shell: unused EC proportional rebalance functions - weed/storage/erasure_coding/distribution: unused distribution analysis functions - Individual unreachable functions removed from 150+ files across admin, credential, filer, iam, kms, mount, mq, operation, pb, s3api, server, shell, storage, topology, and util packages * fix(s3): reset shared memory store in IAM test to prevent flaky failure TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey was flaky because the MemoryStore credential backend is a singleton registered via init(). Earlier tests that create anonymous identities pollute the shared store, causing LookupAnonymous() to unexpectedly return true. Fix by calling Reset() on the memory store before the test runs. * style: run gofmt on changed files * fix: restore KMS functions used by integration tests * fix(plugin): prevent panic on send to closed worker session channel The Plugin.sendToWorker method could panic with "send on closed channel" when a worker disconnected while a message was being sent. The race was between streamSession.close() closing the outgoing channel and sendToWorker writing to it concurrently. Add a done channel to streamSession that is closed before the outgoing channel, and check it in sendToWorker's select to safely detect closed sessions without panicking. |