7 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Junker der Provinz	78f79a3919	master: honour -volume.fileSizeLimitMB on the master's /submit (#11176) ... * fix(master): honour -volume.fileSizeLimitMB on the master's /submit - #6748 `weed server -volume.fileSizeLimitMB=2048` still refused anything over 256MB, and the reason is not the one the report assumes: the option does reach the volume server. The master does not use it. Uploads through the master's /submit are buffered by submitForClientHandler, which passed a hardcoded 256MB to needle.ParseUpload, so the master rejected what the volume server it started would have accepted. The limit is now passed in. `weed master` gains its own -fileSizeLimitMB with the same 256 default, so a standalone master behaves exactly as before, and `weed server` and `weed mini` hand it the value their volume server already got. * master.follower: take the same upload limit, and say which flag to match Review found the follower left behind. It serves /submit like the leader and buffers uploads under the same limit, but kept the fixed 256MB, so a cluster raised above that would accept an upload through the leader and refuse the identical one through a follower. Two smaller points from the same review: the master's flag description named only the standalone volume server's spelling, and now names the weed server and weed mini form too; and the under-limit test asserted on the error message alone, so it would have passed had the limit rejected that payload with different wording. It now requires the request to get past parsing.	2026-09-05 12:58:17 -07:00
4 people	f99c4a1f14	s3: make RenameObject idempotent for a retried request (#11178) ... * feat(s3): make RenameObject idempotent for a retried request - #10661 A rename that succeeds but whose response is lost leaves the client with no safe move: retrying returned 404, because the source is already gone, so a retry was indistinguishable from a rename that never happened. The destination now carries what the rename that created it was, under x-seaweedfs-rename-token: the client's token, the source key and the time. A retry that names the same token and the same source and destination is answered 200 without touching anything. The same token sent for a different rename is refused with 409 rather than silently answered, and a token older than 24 hours is treated as unrelated so a key cannot answer for a request indefinitely. Requests without the header behave exactly as before. * s3: answer a reused rename token with 409, not 400 The PR promised Conflict and the code returned Bad Request. 400 tells a client its request was malformed and invites it to give up; this request is well formed and resending it unchanged will not help, because what it collides with is a rename the same token already stands for. The status code is now asserted in a test, since it is the part of this behaviour a client actually acts on. * Update weed/s3api/s3err/s3api_errors.go Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: fix rename token review notes - ErrIdempotentParameterMismatch returns 409 Conflict, not 400. The comment and TestRenameTokenReuseAnswersConflict both expect 409; the code regressed to 400 in a later commit. - stampRenameToken: clarify that markRenameToken mutates srcEntry in place, so the token reaches the destination via the move regardless of whether the UpdateEntry succeeds. The precondition only guards the pre-move write, not the move itself. - Extract the handler retry branch into retryRenameDecision and add TestRetryRenameDecision, covering the source-still-exists fallthrough that was previously reasoned about but not tested. * s3: IdempotentParameterMismatch returns 400, matching AWS docs The AWS S3 RenameObject API documentation specifies HTTP Status Code: 400 for IdempotencyParameterMismatch. Revert the previous 409 change and align the comment and test with the documented behavior. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-05 11:48:42 -07:00
Junker der Provinz	fa3bd5b5a7	mount: use the kernel-resolved node id in Link, not the persisted attribute (#10885) ... * fix(mount): reply to LINK with the kernel node id, not the stored inode Link() answered the kernel with out.NodeId = oldEntry.Attributes.Inode. That attribute is a mount-runtime number and only entries created through a mount carry one. An entry written by the S3 API, WebDAV or a direct filer call persists inode 0, so the LINK reply named node id 0, which the kernel rejects as invalid_nodeid and reports as EIO. The hard link itself had already been written to the filer, which is why it looked correct again after a mount restart. The same stale number was also used as an inodeToPath key. AddPath(0, path) filed the new link under inode 0, so a later Lookup on that name handed the kernel node id 0 as well, and a LOOKUP reply carrying node id 0 means no such entry. in.Oldnodeid is the node id the kernel already holds for the source, and it is the key inodeToPath is indexed by, so use it for the reply, for AddPath and for the sibling sync. Fixes #8404 * test(mount): cover the sibling sync in Link with a third hard link The two existing cases never reach the body of syncHardLinkSiblings: with two links the source alias and the name just created are both in skipPaths, so the loop iterates over nothing and a change to that site goes unnoticed. A third link leaves one name that no other part of Link() writes. The new case drives three links off one source. It guards against covering nothing (it fails if every path turns out to be a skipPath), checks that every name of the file reports nlink 3, and then drives the sync with both candidate keys to pin down which one it has to be: keyed by the source's persisted Attributes.Inode, which is 0 for an entry written outside a mount, GetAllPaths has no path to walk, while the kernel node id reaches the sibling. That second half is driven directly because Link() alone cannot tell the two keys apart. The meta cache keeps one blob per hard link id (FilerStoreWrapper setHardLink/maybeReadHardLink), so a read of any sibling returns the attributes of the last write to any of them whether or not the sync ran.	2026-08-23 10:43:22 -07:00
Junker der Provinz	5ebc9c9f4b	server: reject a Range start offset equal to the file size (#10898)	2026-08-23 08:20:25 -07:00
Junker der Provinz	8d2c0273bd	admin: stop the maintenance scanner pinning itself to one scan per second after a transient failure (#10887) ... * admin: honour persisted task configs when building the maintenance policy buildPolicyFromTaskConfigs passed a literal nil to vacuum, erasure_coding and balance LoadConfigFromPersistence. Those functions look for their LoadXTaskPolicy() accessor via a type assertion, which a nil interface can never satisfy, so every call fell through to NewDefaultConfig() and the policy came back with the compiled-in defaults - Enabled: true among them. A task disabled on disk was therefore still scheduled, and the only trace was a glog.V(1) "Using default ... configuration" line. Thread the real ConfigPersistence through instead. There are two copies of this function: the one in weed/admin/dash builds config.Policy on the normal admin startup path and can simply take cp as its receiver, and the one in weed/admin/maintenance is the fallback used when the config carries no policy yet, which now receives the store from NewMaintenanceManager. weed/admin/dash already imports weed/admin/maintenance, so the maintenance side has to keep the duck-typed interface{} parameter that the task loaders already use rather than importing the concrete type back. The store is only handed over when a data directory is configured: an unconfigured one has nothing to read, and a typed nil pointer would pass the loaders' type assertion and then panic on first use. Fixes #10874 * admin: restore the maintenance scan cadence after an error backoff scanLoop shortens its ticker to the error backoff delay after a failed scan, but it decided whether to replace the ticker by comparing the target interval against the configured scan interval instead of against the interval the ticker was actually running at. Once the errors stopped, getScanInterval returned the configured interval again, the comparison came out false, and the ticker was left at the backoff delay - so a single transient scan failure pinned the scanner to one scan per second for the rest of the process lifetime. That is the ~1/second cadence in issue #10874: 658 KB/s of "Cancelled N stale pending balance tasks before re-detection" and 193k orphaned task files over two days. Track the interval the ticker is running at and compare against that, so both entering the backoff and returning to the normal cadence replace the ticker. While in here: - defer ticker.Stop() bound the ticker that was current when the defer was registered, so every replacement ticker leaked on return. Wrap it in a closure. - running was written by Start/Stop and read by all three background loops without synchronisation. Guard it with the existing mutex, fold the running check in triggerScanInternal into the lock it already takes, and make Stop a no-op when not running so a second call cannot close the stop channel twice. Refs #10874 * admin: make the maintenance policy actually reach the task detectors Loading the persisted task configs into the maintenance policy only matters if something reads that policy, and nothing did. MaintenanceIntegration pushes the policy into every registered detector and scheduler through interface{ SetEnabled(bool) } and interface{ SetMaxConcurrent(int) } type assertions. Every task registered through base.RegisterTask is backed by base.GenericDetector and base.GenericScheduler, and neither implemented either method, so all four assertions failed silently for every task on every startup. The policy's enabled flag reached nothing: ScanWithTaskDetectors gates on detector.IsEnabled(), and the queue's policy lookups for max concurrent and repeat interval are fallbacks that only fire when the scheduler reports zero, which the generic scheduler never does. Add the setters, delegating to the TaskConfig.SetEnabled the interface already declares and to TaskDefinition.MaxConcurrent, which is what GetMaxConcurrent returns. Applying the policy required three more fixes, because with the assertions working the policy could now do damage as well as good: - IsTaskEnabled reports false for a task type the policy has no entry for, so applying it unconditionally would have disabled every task the policy does not list. Skip task types with no policy entry: no entry means no opinion, not disabled. - ec_balance was exactly such a task. It is registered like the other three but had no entry in the policy builder and no accessor on ConfigPersistence at all, so its configuration could never be persisted. Add SaveEcBalanceTaskPolicy/LoadEcBalanceTaskPolicy, the task_ec_balance.pb file, the SaveTaskPolicy dispatcher case, and the policy entry. - InitMaintenanceManager ran before loadTaskConfigurationsFromPersistence, which replaces each task's whole config object, so the policy was applied and then immediately thrown away. Swap the order. Both read the same files, so the policy is now the last writer and stays authoritative. MaintenanceManager.UpdateConfig also updated the queue's and the scanner's policy but not the integration's, so a policy changed at runtime never reached the detectors. Add MaintenanceIntegration.SetPolicy and call it. While building the policy, stop hand-copying each task's fields and use the task's own ToTaskPolicy(). The hand-written version was a second definition of every task's policy and had already lost the erasure coding preferred tags and replica placement and the balance IO rate limit. For the same reason, the "nothing persisted yet" branches of LoadVacuumTaskPolicy, LoadErasureCodingTaskPolicy and LoadBalanceTaskPolicy now derive from each task's NewDefaultConfig() instead of a third hand-written copy. Those copies had drifted, so with a data directory but no config file on disk the effective defaults differed from what the task and the admin UI schema both advertise: vacuum scan interval 24h -> 2h balance scan interval 6h -> 30m balance imbalance 0.1 -> 0.2 erasure coding scan interval 168h -> 1h erasure coding fullness 0.90 -> 0.95 erasure coding min volume 1024MB -> 30MB Finally, weed/admin/dash and weed/admin/maintenance each carried a copy of the policy builder and they had already diverged. Export the maintenance one as BuildPolicyFromTaskConfigs and have dash call it. Refs #10874 * worker: warn when a config store cannot supply a task's persisted config LoadConfigFromPersistence logged a single glog.V(1) "Using default X configuration" for every way of not loading anything, so the bug in issue #10874 - a store handed in that the type assertion rejects, leaving a task running on compiled-in defaults - looked exactly like the normal "no data directory configured" case. The reporter had to read the source to work out why their disabled task kept running, and asked for this specifically. Separate the cases. A non-nil store that does not provide the accessor is always a wiring bug and is now logged at warning level, naming the type and the missing method. A read error or a policy that will not apply is also a warning. No persistence configured, and a store with nothing saved yet, stay at V(1): those are normal. Refs #10874 * admin: stop GetTaskPolicy panicking on a maintenance policy that is nil GetTaskPolicy dereferenced its MaintenancePolicy argument to look at TaskPolicies, so IsTaskEnabled, GetMaxConcurrent and GetRepeatInterval all took the admin process down when handed a nil policy. A nil policy is not a programming error here: MaintenanceConfig.Policy is unset until something builds one, DefaultMaintenanceConfig returns a config with no policy at all, and UpdateConfig installs whatever config it is given. Found by calling IsTaskEnabled with the policy from a freshly defaulted MaintenanceConfig. Treat a nil policy as "no entry": no task enabled, the safe concurrency default of 1, and a repeat interval of 0 so callers fall back to their own default instead of reading DefaultRepeatIntervalSeconds off nil. Also add the startup test this was found with. It walks the admin server's startup sequence over a data directory that has balance saved as disabled and checks the state that decides whether issue #10874 happens: the balance detector reports disabled, vacuum stays enabled, and tasks whose config was never saved keep their compiled-in default. Refs #10874 * admin: document the synchronisation SetPolicy would need beyond startup ConfigureTasksFromPolicy now really writes TaskDefinition.Config and TaskDefinition.MaxConcurrent, which the scan loop reads through detector.IsEnabled() with nothing synchronising the two. Every caller runs during admin server startup today, before the scan loop exists, so there is no live race - but the next caller has to add the locking, and the same already applies to UpdateAllConfigs replacing the whole config object. Write it down at the seam instead of leaving it to be rediscovered. Refs #10874	2026-08-22 23:41:45 -07:00
Junker der Provinz	f710b6003a	s3api: retry a transient failure when listing multipart uploads/parts (RFC on layering) (#10886) ... s3api: retry a transient failure when listing multipart uploads/parts A blip on the way to the filer failed the whole ListMultipartUploads or ListParts request. Both reported failure points sit inside one streaming listing: the ListEntries call that opens the stream, and the stream.Recv calls that drain it. Neither retried, so a single Unavailable answer from a filer that was restarting turned into a 500 for the S3 client. Replay the listing instead, bounded to three attempts with a 100ms backoff that doubles. Only a transient failure is replayed. A not-found answer stays authoritative so the empty-list branch still works, and every other error still reaches the client on the first attempt. This is scoped to (*S3ApiServer).list rather than added inside DoSeaweedListWithSnapshot, which mount, the shell and the other object listings share, and where a retry after a partial stream would re-deliver entries the callback had already seen. Within one call to list, a replay is safe: it collects into a fresh slice each time, so it can neither duplicate nor drop entries. That guarantee does not extend past this function. withFilerClientFailover already re-runs its callback against the next filer on any non-NotFound error without resetting the caller's accumulator, so on a multi-filer gateway a mid-listing failover can itself produce a duplicated result with err == nil, independent of this change and not fixed by it. Noted in the PR rather than silently left for someone to rediscover. Fixes #7221 References #7235	2026-08-22 23:00:50 -07:00
Junker der Provinz	f3caf6e7da	admin: count plugin-runtime workers in worker metrics (#10884) ... * admin: count plugin-runtime workers in worker metrics The admin server keeps two worker registries: the legacy maintenance-worker map, filled by workers registering over the worker gRPC stream, and the plugin worker registry, filled by workers started as `weed worker`. Both the SeaweedFS_admin_workers_connected / SeaweedFS_admin_worker_slots gauges and the dashboard's Workers card read only the legacy map, so a cluster that runs the admin and its workers as separate components reported 0 workers even while its workers showed up on the plugin pages and ran scheduled jobs. Aggregate both registries instead. The two are merged by worker ID: `weed mini` starts both runtimes out of one working directory, so they share the persisted worker ID and must not be counted twice. For such a worker the slot numbers still come from the legacy registry, which keeps mini's existing readings. Plugin workers report their slots in the heartbeat, so detection and execution slots are summed from there; a worker that has connected but not yet sent a heartbeat counts as connected with zero slots. Fixes #10525 * admin: clamp negative worker-reported slot values in metrics merge A plugin worker's self-reported heartbeat slot counts are untrusted input; clamp them to 0 before summing so a stale or misbehaving worker can't drive the aggregate gauge negative, matching the same defensiveness already used in registry.go's own slot arithmetic.	2026-08-22 22:54:39 -07:00