mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-30 03:53:18 +00:00
cb882ced468b7c658db847ef1820304ea346bb40
13571
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
cb882ced46 |
fix(test): retry ENOENT in fcntl lock subprocess helper
TestPosixFileLocking/FcntlReleaseOnClose was flaky because the subprocess spawned by startLockHolder occasionally saw ENOENT when opening a file the parent had just created on the FUSE mount. Retry on ENOENT (matching the existing openWithRetry pattern used in testConcurrentLockContention) so the subprocess waits for the mount's dentry state to propagate before reporting the lock acquire as failed. |
||
  Chris LuandGitHub
|
c4e1885053 |
fix(ec): honor disk_id in ReceiveFile so EC shards respect admin placement (#9184) (#9185)
* test(volume_server): reproduce #9184 EC ReceiveFile disk-placement bug The plugin-worker EC task sends shards via ReceiveFile, which picks Locations[0] as the target directory regardless of the admin planner's TargetDisk assignment. ReceiveFileInfo has no disk_id field, so there is no wire channel to honor the plan. Adds StartSingleVolumeClusterWithDataDirs to the integration framework so tests can launch a volume server with N data directories. The new repro asserts the current (buggy) behavior: sending three distinct EC shards via ReceiveFile leaves all three files in dir[0] and the other dirs empty. When the fix adds disk_id to ReceiveFileInfo, this assertion must flip to verify the planned placement is respected. * fix(ec): honor disk_id in ReceiveFile so EC shards respect admin placement Before this change, VolumeServer.ReceiveFile for EC shards always selected the first HDD location (Locations[0]). The plugin-worker EC task had no way to pass the admin planner's per-shard disk assignment — ReceiveFileInfo carried no disk_id field — so every received EC shard piled onto a single disk per destination server. On multi-disk servers this caused uneven load (one disk absorbing all EC shard I/O), frequent ENOSPC retries, and a growing EC backlog under sustained ingest (see issue #9184). Changes: - proto: add disk_id to ReceiveFileInfo, mirroring VolumeEcShardsCopyRequest.disk_id. - worker: DistributeEcShards tracks the planner-assigned disk per shard; sendShardFileToDestination forwards that disk id. Metadata files (ecx/ecj/vif) inherit the disk of the first data shard targeting the same node so they land next to the shards. - server: ReceiveFile honors disk_id when > 0 with bounds validation; disk_id=0 (unset) falls back to the same auto-selection pattern as VolumeEcShardsCopy (prefer disk that already has shards for this volume, then any HDD with free space, then any location with free space). Tests updated: - TestReceiveFileEcShardHonorsDiskID asserts three shards sent with disk_id={1,2,0} land on data dirs 1, 2, and 0 respectively. - TestReceiveFileEcShardRejectsInvalidDiskID pins the out-of-range disk_id rejection path. * fix(volume-rust): honor disk_id in ReceiveFile for EC shards Mirror the Go-side change: when disk_id > 0 place the EC shard on the requested disk; when unset, auto-select with the same preference order as volume_ec_shards_copy (disk already holding shards, then any HDD, then any disk). * fix(volume): compare disk_id as uint32 to avoid 32-bit overflow On 32-bit Go builds `int(fileInfo.DiskId) >= len(Locations)` can wrap a high-bit uint32 to a negative int, bypassing the bounds check before the index operation. Compare in the uint32 domain instead. * test(ec): fail invalid-disk_id test on transport error Previously a transport-level error from CloseAndRecv silently passed the test by returning early, masking any real gRPC failure. Fail loudly so only the structured ReceiveFileResponse rejection path counts as a pass. * docs(test): explain why DiskId=0 auto-selects dir 0 in EC placement test Documents the load-bearing assumption that shards are never mounted in this test, so loc.FindEcVolume always returns false and auto-select falls through to the first HDD. Saves future readers from re-deriving the expected directory for the DiskId=0 case. * fix(test): preserve baseDir/volume path for single-dir clusters StartSingleVolumeClusterWithDataDirs started naming the data directory volume0 even in the dataDirCount=1 case, which broke Scrub tests that reach into baseDir/volume via CorruptDatFile / CorruptEcShardFile / CorruptEcxFile. Keep the legacy name for single-dir clusters; only use the indexed "volumeN" layout when multiple disks are requested. |
||
|
Chris LuandGitHub
|
0f5e99f423 |
fix(filer.meta.tail): fail fast when -es is used without elastic build tag (#9191)
fix(filer.meta.tail): error instead of silently dropping events when -es is used without elastic build tag The default chrislusf/seaweedfs image builds without the `elastic` build tag, so sendToElasticSearchFunc was a no-op that returned a function discarding every event. Users passing -es saw the subscription wire up in filer logs but nothing ever reached Elasticsearch. Return an error explaining the binary wasn't built with ES support and pointing at the build flag. The caller already prints the error and exits, so users now get an immediate, actionable message. Fixes #9190 |
||
  |
1220468a33 |
build(deps): bump github.com/rclone/rclone from 1.73.1 to 1.73.5 in /test/kafka (#9189)
build(deps): bump github.com/rclone/rclone in /test/kafka Bumps [github.com/rclone/rclone](https://github.com/rclone/rclone) from 1.73.1 to 1.73.5. - [Release notes](https://github.com/rclone/rclone/releases) - [Changelog](https://github.com/rclone/rclone/blob/master/RELEASE.md) - [Commits](https://github.com/rclone/rclone/compare/v1.73.1...v1.73.5) --- updated-dependencies: - dependency-name: github.com/rclone/rclone dependency-version: 1.73.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
be9996962d |
fix(test): avoid port collision between master gRPC and volume ports
AllocateMiniPorts(1) reserved masterPort and masterPort+GrpcPortOffset by holding listeners open, but closed them on return. The subsequent AllocatePorts call bound 127.0.0.1:0, so the OS could immediately reuse the just-released mini gRPC port as a volume port — causing the volume server to fail at bind time with "address already in use". Introduce AllocatePortSet(miniCount, regularCount) that holds every listener open until the full set is chosen, and route the five volume test cluster builders through it. |
||
|
Chris Lu
|
96e5fea08e |
test(catalog_spark): bound weed shell invocation with 30s timeout
createTableBucket ran `weed shell` via exec.Command with no deadline. When the shell's first command retries on a transient master connection blip, the trailing `exit` on stdin never gets processed and the subprocess blocks until the outer 20m `go test` timeout fires — the surfacing symptom is a flaky 20m panic with no diagnostic output. Wrap the invocation in exec.CommandContext with a 30s timeout, matching the existing pattern in test/s3tables/catalog_risingwave/setup_test.go. |
||
|
Chris LuandGitHub
|
7f67995c24 |
chore(filer): remove -mount.p2p flag; registry is always on (#9183)
The filer-side mount peer registry (tier 1 of peer chunk sharing) was gated behind -mount.p2p (default true). Idle cost is negligible — a tiny in-memory map plus a 60s sweeper — so the opt-out is not worth the surface area. Removes the flag from weed filer, weed server (-filer.mount.p2p), and weed mini, and always constructs the registry in NewFilerServer. Also drops the now-dead nil guards in MountRegister/MountList/sweeper and the TestMountRegister_DisabledIsNoOp case. |
||
|
Chris LuandGitHub
|
9ae905e456 |
feat(security): hot-reload HTTPS certs without restart (k8s cert-manager) (#9181)
* feat(security): hot-reload HTTPS certs for master/volume/filer/webdav/admin S3 and filer already use a refreshing pemfile provider for their HTTPS cert, so rotated certificates (e.g. from k8s cert-manager) are picked up without a restart. Master, volume, webdav, and admin, however, passed cert/key paths straight to ServeTLS/ListenAndServeTLS and loaded once at startup — rotating those certs required a pod restart. Add a small helper NewReloadingServerCertificate in weed/security that wraps pemfile.Provider and returns a tls.Config.GetCertificate closure, then wire it into the four remaining HTTPS entry points. httpdown now also calls ServeTLS when TLSConfig carries a GetCertificate/Certificates but CertFile/KeyFile are empty, so volume server can pre-populate TLSConfig. A unit test exercises the rotation path (write cert, rotate on disk, assert the callback returns the new cert) with a short refresh window. * refactor(security): route filer/s3 HTTPS through the shared cert reloader Before: filer.go and s3.go each kept a *certprovider.Provider on the options struct plus a duplicated GetCertificateWithUpdate method. Both were loading pemfile themselves. Behaviorally they already reloaded, but the logic was duplicated two ways and neither path was shared with the newly-added master/volume/webdav/admin wiring. After: both use security.NewReloadingServerCertificate like the other servers. The per-struct certProvider field and GetCertificateWithUpdate method are removed, along with the now-unused certprovider and pemfile imports. Net: -32 lines, one code path for all HTTPS cert reloading. No behavior change — the refresh window, cache, and handshake contract are identical (the helper wraps the same pemfile.NewProvider). * feat(security): hot-reload HTTPS client certs for mount/backup/upload/etc The HTTP client in weed/util/http/client loaded the mTLS client cert once at startup via tls.LoadX509KeyPair. That left every long-lived HTTPS client process (weed mount, backup, filer.copy, filer→volume, s3→filer/volume) unable to pick up a rotated client cert without a restart — even though the same cert-manager setup was already rotating the server side fine. Swap the client cert loader for a tls.Config.GetClientCertificate callback backed by the same refreshing pemfile provider. New TLS handshakes pick up the rotated cert; in-flight pooled connections keep their old cert and drop as normal transport churn happens. To keep this reusable from both server and client TLS code without an import cycle (weed/security already imports weed/util/http/client for LoadHTTPClientFromFile), extract the pemfile wrapper into a new weed/security/certreload subpackage. weed/security keeps its thin NewReloadingServerCertificate wrapper. The existing unit test moves with the implementation. gRPC mTLS was already handled by security.LoadServerTLS / LoadClientTLS; this PR does not change any gRPC paths. MQ broker, MQ agent, Kafka gateway, and FUSE mount control plane are gRPC-only and therefore already rotate. CA bundles (ClientCAs / RootCAs / grpc.ca) are still loaded once — noted as a known limitation in the wiki. * fix(security): address PR review feedback on cert reloader Bots (gemini-code-assist + coderabbit) flagged three real issues and a couple of nits. Addressing them here: 1. KeyMaterial used context.Background(). The grpc pemfile provider's KeyMaterial blocks until material arrives or the context deadline expires; with Background() a slow disk could hang the TLS handshake indefinitely. Switched both the server and client callbacks to use hello.Context() / cri.Context() so a stuck read is bounded by the handshake timeout. 2. Admin server loaded TLS inside the serve goroutine. If the cert was bad, the goroutine returned but startAdminServer kept blocking on <-ctx.Done() with no listener, making the process look healthy with nothing bound. Moved TLS setup to run before the goroutine starts and propagate errors via fmt.Errorf; also captures the provider and defers Close(). 3. HTTP client discarded the certprovider.Provider from NewClientGetCertificate. That leaked the refresh goroutine, and NewHttpClientWithTLS had a worse case where a CA-file failure after provider creation orphaned the provider entirely. Added a certProvider field and a Close() method on HTTPClient, and made the constructors close the provider on subsequent error paths. 4. Server-side paths (master/volume/filer/s3/webdav/admin) now retain the provider. filer and webdav run ServeTLS synchronously, so a plain defer works. master/volume/s3 dispatch goroutines and return while the server keeps running, so they hook Close() into grace.OnInterrupt. 5. Test: certreload_test now tolerates transient read/parse errors during file rotation (writeSelfSigned rewrites cert before key) and reports the last error only if the deadline expires. No user-visible behavior change for the happy path. * test(tls): add end-to-end HTTPS cert rotation integration test Boots a real `weed master` with HTTPS enabled, captures the leaf cert served at TLS handshake time, atomically rewrites the cert/key files on disk (the same rename-in-place pattern kubelet does when it swaps a cert-manager Secret), and asserts that a subsequent TLS handshake observes the rotated leaf — with no process restart, no SIGHUP, no reloader sidecar. Verifies the full path: on-disk change → pemfile refresh tick → provider.KeyMaterial → tls.Config.GetCertificate → server TLS handshake. Runtime is ~1s by exposing the reloader's refresh window as an env var (WEED_TLS_CERT_REFRESH_INTERVAL) and setting it to 500ms for the test. The same env var is user-facing — documented in the wiki — so operators running short-lived certs (Vault, cert-manager with duration: 24h, etc.) can tighten the rotation-pickup window without a rebuild. Defaults to 5h to preserve prior behavior. security.CredRefreshingInterval is kept for API compatibility but now aliases certreload.DefaultRefreshInterval so the same env controls both gRPC mTLS and HTTPS reload. * ci(tls): wire the TLS rotation integration test into GitHub Actions Mirrors the existing vacuum-integration-tests.yml shape: Ubuntu runner, Go 1.25, build weed, run `go test` in test/tls_rotation, upload master logs on failure. 10-minute job timeout; the test itself finishes in about a second because WEED_TLS_CERT_REFRESH_INTERVAL is set to 500ms inside the test. Runs on every push to master and on every PR to master. * fix(tls): address follow-up PR review comments Three new comments on the integration test + volume shutdown path: 1. Test: peekServerCert was swallowing every dial/handshake error, which meant waitForCert's "last err: <nil>" fatal message lost all diagnostic value. Thread errors back through: peekServerCert now returns (*x509.Certificate, error), and waitForCert records the latest error so a CI flake points at the actual cause (master didn't come up, handshake rejected, CA pool mismatch, etc.). 2. Test: set HOME=<tempdir> on the master subprocess. Viper today registers the literal path "$HOME/.seaweedfs" without env expansion, so a developer's ~/.seaweedfs/security.toml is accidentally invisible — the test was relying on that. Pinning HOME is belt-and-braces against a future viper upgrade that does expand env vars. 3. volume.go: startClusterHttpService's provider close was registered via grace.OnInterrupt, which fires on SIGTERM but NOT on the v.shutdownCtx.Done() path used by mini / integration tests. The pemfile refresh goroutine leaked in that shutdown path. Now the helper returns a close func and the caller invokes it on BOTH shutdown paths for parity. Also add MinVersion: TLS 1.2 to the test's tls.Config to quiet the ast-grep static-analysis nit — zero-risk since the pool only trusts our in-memory CA. Test runs clean 3/3. |
||
|
Chris LuandGitHub
|
7364f148bd |
fix(s3/shell): factor EC volumes into bucket size metrics and collection.list (#9182)
* fix(s3/shell): include EC volumes in bucket size metrics and collection.list S3 bucket size metrics exported to Prometheus (and fed through stats.UpdateBucketSizeMetrics) are computed by collectCollectionInfoFromTopology, which only walked diskInfo.VolumeInfos. As soon as a volume was encoded to EC it dropped out of every aggregate, so Grafana showed bucket sizes shrinking while physical disk usage kept climbing. The shell helper collectCollectionInfo — used by collection.list and s3.bucket.quota.enforce — had the same gap, with the EC branch left as a commented-out TODO. Fold EC shards into both paths using the same approach the admin dashboard already uses (PR #9093): - PhysicalSize / Size sum across shard holders: EC shards are node-local (not replicas), so per-node TotalSize() and MinusParityShards().TotalSize() sum to the whole-volume physical and logical sizes respectively. - FileCount is deduped via max across reporters (every shard holder reports the same .ecx count; a slow node with a not-yet-loaded .ecx reports 0 and must not pin the aggregate). - DeleteCount is summed (each delete tombstones exactly one node's .ecj). - VolumeCount increments once per unique EC volume id. Adds regression tests covering pure-EC, mixed regular+EC, and the slow-reporter FileCount dedupe case. Refs #9086 * Address PR review feedback: EC size helpers, composite key, VolumeCount dedupe - Add EcShardsTotalSize / EcShardsDataSize helpers in the erasure_coding package that walk the shard bitmap directly instead of materializing a ShardsInfo and copying it via MinusParityShards(). Keeps the DataShardsCount dependency encapsulated in one place and avoids the per-shard allocation/copy overhead in the metrics hot path. - Switch shell collectCollectionInfo ecVolumes map to a composite {collection, volumeId} key, matching the bucket_size_metrics collector and defending against any cross-collection volume id aliasing. - Dedupe VolumeCount in shell addToCollection by volume id so regular volumes aren't counted once per replica presence. Aligns the shell's collection.list output with the S3 metrics collector and the EC branch, all of which now report logical volume counts. - Add unit tests for the new helpers and for the regular-volume VolumeCount dedupe. * Parameterize EcShardsDataSize with dataShards for custom EC ratios Add a dataShards parameter to EcShardsDataSize so forks with per-volume ratio metadata (e.g. the enterprise data_shards field carried on an extended VolumeEcShardInformationMessage) can pass the configured value and get accurate logical sizes under custom EC policies like 6+3 or 16+6. Passing 0 or a negative value falls back to the upstream DataShardsCount default, which is correct for the fixed 10+4 layout — so OSS callers in s3api and shell pass 0 and keep their current behavior. Added table cases covering the custom 6+3 and 16+6 paths so the parameterization is pinned by tests. |
||
|
Chris LuandGitHub
|
05f0f7e1c9 |
fix(remote-storage/azure): fix re-cache of large remote blobs (#9174) (#9179)
* fix(remote-storage/azure): fix re-cache of large remote blobs (#9174) ReadFile issued a single DownloadStream for the entire requested byte range, so a large re-cache (e.g. a 2 GB blob re-fetched on S3 GET after eviction) had to move the whole range over one HTTP connection within the SDK's per-try TryTimeout. TryTimeout was set to 10s "to fail faster on auth issues", which silently broke large reads: every attempt hit context deadline, the filer's CacheRemoteObjectToLocalCluster returned an error, and the S3 gateway surfaced it to clients as an ETag-mismatch on the partial response. Switch ReadFile to the SDK's parallel block downloader (DownloadBuffer with 4 MiB blocks) so each individual HTTP GET is small enough to complete well inside TryTimeout. Expose the parallelism through the RemoteStorageConcurrentReader interface so callers (FetchAndWriteNeedle) can tune it per request, matching the S3 backend. Also restore TryTimeout to 60s. With parallel block transfers it is no longer on the critical path for large-blob bodies, but it gives metadata operations and any non-parallel paths more headroom on slow links. * fix(remote-storage/azure): guard ReadFileWithConcurrency inputs Addresses review feedback on PR #9179: - Reject negative size up front instead of panicking inside make([]byte, size). - Clamp concurrency to math.MaxUint16 before casting to uint16 so an oversized caller value can't silently wrap to a small number. * fix(remote-storage/azure): reject negative offset in ReadFileWithConcurrency Addresses review feedback on PR #9179. Without this guard, a negative offset combined with size == 0 would compute `size = ContentLength - offset` -> a value larger than the blob, then attempt to allocate and download past the end. |
||
|
Chris LuandGitHub
|
45ba71a189 |
fix(volume): write state.pb into a real dir when -dir.idx is unset (#9178)
* fix(volume): write state.pb into a real dir when -dir.idx is unset When -dir.idx is not set, NewStore passed the empty default to NewState, making the state.pb path resolve to a relative "state.pb" against the process CWD. Under systemd (where CWD is typically /), this caused "open state.pb: permission denied" for operations such as `volumeServer.state -maintenanceOn`, even though the configured user owned the data dirs. Fall back to the first disk location's IdxDirectory so state.pb lives next to the volume data, consistent with other per-server artifacts. Fixes #9173 * fix(volume): always resolve state.pb dir via first disk location Use s.Locations[0].IdxDirectory unconditionally when a location exists so state.pb inherits the same resolution (~ expansion and empty-idxFolder fallback) already applied for the .idx files. Fall back to util.ResolvePath(idxFolder) in the location-less case so a relative or tilde-prefixed -dir.idx is still normalized. Addresses PR feedback on #9178. |
||
|
Chris LuandGitHub
|
c40db5a52d |
perf(filer): parallelize StreamMutateEntry with path-keyed scheduler (#9171)
* perf(filer): parallelize StreamMutateEntry with path-keyed scheduler The server handler processed one mutation at a time per stream, capping a mount's aggregate throughput at ~1/filer_store_service_time regardless of client concurrency (see issue #9138). With 12 rclone processes this showed as a ~500 QPS ceiling on a filer that previously served ~1000+ QPS via unary CreateEntry. Replace the serial for-loop with a per-request goroutine admitted by a path-keyed scheduler, adapted directly from filer.sync's MetadataProcessor (weed/command/filer_sync_jobs.go). Same four conflict indexes, same kind taxonomy (file / barrier-dir / non-barrier-dir), same ancestor-barrier and descendant-barrier rules. Cross-path mutations run in parallel; same- path mutations serialize on arrival order; recursive delete and directory rename act as subtree barriers; directory attribute bumps stay non-barrier so they do not serialize file writes under them. Correctness and safety: - Per-stream goroutine cap (streamMutateConcurrency = 64) bounds resource use from a single noisy mount. - syncStream wrapper serializes stream.Send across worker goroutines (gRPC Send is not concurrent-safe). - Handler waits on in-flight workers before returning on recv EOF/error so no worker writes to a torn-down stream. - First fatal Send error from any worker propagates as the handler's return, causing the stream to tear down. Benchmark (2 ms simulated filer-store service delay, 12 client workers): serial : 440 QPS sem only : 4902 QPS (unsafe — reorders same-path ops) scheduler : 4934 QPS on distinct paths, 439 QPS on same path (correct) The sem-only number shows the upper bound of raw parallelism; the scheduler matches it on distinct paths (the realistic 12-rclone case) and correctly falls back to serial when the workload demands ordering. Peak concurrent mutations at the handler equals client worker count on the distinct-path workload and pins to 1 on the same-path workload, as the scheduler intends. * perf(filer): decouple StreamMutateEntry admission from receive loop The previous StreamMutateEntry handler called sched.Admit directly in the Recv loop. A single request conflicting on path /hot would head-of-line block stream.Recv, so later requests targeting unrelated paths could not be received or admitted until /hot drained — cross-path parallelism then depended on request ordering instead of being a property of the scheduler. Spawn the worker goroutine immediately on Recv and move sched.Admit into that goroutine. A new streamMutatePendingLimit (1024) caps total per- stream outstanding goroutines (pending + active) so a client flooding a conflicted path cannot explode goroutine count without bound. Addresses #9171 review comment (coderabbitai, Major). * fix(filer): reply with EINVAL on unknown StreamMutateEntry request type Returning nil when req.Request is a future oneof variant or a malformed request left the client's per-RequestId waiter blocked forever, because no response was ever sent for that id. Reply with IsLast=true and EINVAL so the waiter completes with a well-formed error. Addresses #9171 review comments (gemini-code-assist, coderabbitai). * fix(filer): make classifyMutation crash-free and correct for deletes Two issues addressed together because they share one function: 1. Nil-entry panic. classifyMutation dereferenced req.Entry.Name without a nil guard; an empty create_request / update_request / rename_request from a misbehaving client crashed the scheduler. Guard each oneof variant and fall back to a "/" barrier; the handler then sends EINVAL via the unknown-request path. 2. Non-recursive delete vs concurrent dir attribute update. DeleteEntry- Request does not carry IsDirectory, so the previous kindMutateFile classification for non-recursive deletes did not conflict with an in- flight kindMutateNonBarrierDir (chmod / xattr / mtime) at the same path — a race in scheduler terms. Classify every delete as kindMutateBarrierDir regardless of IsRecursive. The incremental cost of a descendant-wait for a non-recursive delete of a non-empty dir is negligible since that call fails at the store anyway. Adds classifyMutation tests for malformed create/update, empty oneof, and updates the delete-non-recursive case to the new expected kind. Addresses #9171 review comments (coderabbitai Critical, Major). * fix(filer): route renameStreamProxy.SendMsg through the wrapping Send The default pass-through SendMsg on renameStreamProxy bypassed the syncStream mutex and the StreamMutateEntryResponse wrapping: anything the rename helpers happened to push via SendMsg would have been emitted on the wire as the wrong protobuf type and could interleave with other workers' Sends. RecvMsg similarly raced with the outer StreamMutateEntry Recv loop and could steal unrelated mutation requests. Route SendMsg through the wrapping Send (rejecting other payload types) and fail RecvMsg explicitly — the rename logic is a strictly server-push stream and never calls RecvMsg, so loud failure is safer than silent stealing. Addresses #9171 review comment (coderabbitai, Major). * test(filer): run exactly ops in stream-mutate workloads perGoroutine := ops / concurrency silently truncated the total when the values were not divisible — e.g. 2400 ops with 64 workers actually ran 2368 and with 256 workers ran 2304, making the logged "ops per run" inaccurate and introducing measurement noise that varied across the concurrency sweep. Introduce opsForWorker(g, concurrency, ops) which distributes the remainder to the first (ops % concurrency) workers so the three workloads (unary, stream sync, stream async) each dispatch exactly `ops` operations. No changes to the timing methodology. Addresses #9171 review comment (coderabbitai, Minor). * fix(filer): enforce per-path FIFO admission in mutateScheduler sync.Cond.Broadcast wakes every waiter; the first to re-acquire the mutex wins, so two conflicting same-path admissions could be reordered by the Go runtime even though they arrived serially on the stream. A single stream is supposed to carry ordered mutations — the PR's original #8770 claim — so admission must be FIFO per path. Replace the single cond with a per-path FIFO queue. Each Admit enqueues a waiter on every path it touches (primary, and on rename the secondary too) and blocks on a ready channel. tryPromoteLocked admits any waiter that is at the head of every queue it joined, passes pathConflictsLocked against the active-state indexes, and is under concurrencyLimit. Done removes the heads and re-runs tryPromoteLocked so waiters freed by the completion move in arrival order. Side effect: two non-barrier directory updates on the same path now serialize instead of overlapping. filer.sync's MetadataProcessor intentionally allows them to overlap because its events come from a committed log where last-writer-wins coalescing is safe; streamed mutations carry client operations whose order matters, so we drop that optimization here. Added TestAdmitSamePathFIFO (20-waiter barrier release) and TestAdmitSamePathNonBarrierSerializes to cover both. Also refreshed the kindMutateFile doc comment that still referenced the pre-#1ecf805f5 "non-recursive delete" classification. Addresses #9171 review comments (coderabbitai Critical, Minor). * test(filer): make TestAdmitSamePathFIFO deterministic without sleeps The previous arrival-ordering sync (send to `started` before calling Admit, plus a 1 ms sleep) relied on the goroutine actually entering Admit and reaching the per-path queue during that sleep. Under -race on a loaded CI that is a real flake source, which is ironic for a test whose job is catching non-deterministic wake-ups. Observe the scheduler's own pathQueue length between spawns instead — waitQueueLen polls s.pathQueue["/a"] under s.mu until the expected number of waiters (1 barrier holder + i+1 file waiters) is enqueued. That's the exact event the test wants to synchronise on, so there is no fudge factor. Verified by `go test -race -count=5`. Addresses #9171 review comment (coderabbitai, Minor). |
||
|
Chris LuandGitHub
|
141413ad76 |
fix(tests): make tests pass on 32-bit architectures (#9168) (#9170)
Two separate failures reported on 32-bit builds (void-linux 4.21): - weed/server: errorStreamImpl.count (and the same pattern in slowStream plus local totalEventsSent/totalSends) was a bare int64 sitting after smaller fields, so on 386/ARMv7/mips32 it landed at a 4-byte-aligned offset and atomic.AddInt64 panicked with "unaligned 64-bit atomic operation". Switched the counters to atomic.Int64, which Go guarantees is 8-byte aligned on every architecture. - weed/plugin/worker/iceberg: three equality-delete tests fail on 32-bit because the upstream github.com/apache/iceberg-go declares manifestEntry.EqualityIDs as *[]int while the Iceberg Avro schema defines equality_ids as long, and hamba/avro refuses to map Go int onto Avro long when int is 32-bit. Not fixable in seaweedfs, so guard the affected tests with a t.Skip() when unsafe.Sizeof(int) < 8 until the upstream type is changed to []int32/[]int64. |
||
|
Chris LuandGitHub
|
0a5c22b57e |
chore(upload): log offset/bytes-read context on chunk ReadFrom errors (#9169)
chore(upload): add offset/bytes-read context to chunk ReadFrom errors Wrap io.ErrUnexpectedEOF (and siblings) from bytesBuffer.ReadFrom so the log shows "read chunk at offset N (got M bytes): ..." instead of a bare "unexpected EOF". The context distinguishes a client disconnect before any data arrived (offset=0, got=0) from a mid-stream truncation (offset>0, got<chunkSize) — diagnosing #9149 in the wild. Verified at this stage that expectedDataSize is already threaded correctly from upload_chunked.go (actual ReadFrom bytes) through s3api assignFunc → filer AssignVolume → master PickForWrite. No behavioral change to what the master receives. |
||
|
Chris LuandGitHub
|
e77f8ae204 |
fix(s3api): route STS GetFederationToken to STS handler (#9157) (#9167)
* fix(s3api): route STS GetFederationToken requests to STS handler (#9157) The STS GetFederationToken handler was implemented but never reachable. Three routing gaps sent requests to the S3/IAM path instead of STS: - No explicit mux route for Action=GetFederationToken in the URL query - iamMatcher did not exclude GetFederationToken, so authenticated POSTs with Action in the form body were matched and dispatched to IAM - UnifiedPostHandler only dispatched AssumeRole* and GetCallerIdentity to STS, leaving GetFederationToken to fall through to DoActions and return NotImplemented Add the missing route, the matcher exclusion, and the dispatch branch. Also wire TestSTS, TestAssumeRoleWithWebIdentity, and TestServiceAccount into the s3-iam-tests workflow as a new "sts" matrix entry. Before this change, none of test/s3/iam/s3_sts_get_federation_token_test.go's four test functions ran in CI, which is why this regression shipped. * test(iam): make orphaned STS/service-account tests pass under auth-enabled CI Follow-up to wiring STS tests into CI: fixes several pre-existing issues that made the newly-included tests fail locally. Server fixes: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer 500s when the caller is a legacy S3-config identity (not in the IAM user store). Previously any GetPoliciesForUser error short-circuited to InternalError, which hard-failed every SigV4 caller using keys from -s3.config. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now generates IDs in the sa:<parent>:<uuid> format required by credential.ValidateServiceAccountId. The old "sa-XXXXXXXX" format failed the persistence-layer regex and caused every CreateServiceAccount call to return 500 once a filer-backed credential store validated the ID. Test helpers: - test/s3/iam/s3_sts_assume_role_test.go: callSTSAPIWithSigV4 no longer sets req.Header["Host"]. aws-sdk-go v1 v4.Signer already signs Host from req.URL.Host, and a manual Host header made the signer emit host;host in SignedHeaders, producing SignatureDoesNotMatch. Updated missing_role_arn subtest to match the existing SeaweedFS behavior (user-context assumption). - test/s3/iam/s3_service_account_test.go: callIAMAPI now SigV4-signs requests when STS_TEST_{ACCESS,SECRET}_KEY env vars are set. Unsigned IAM writes otherwise fall through to the STS fallback and return InvalidAction. CI matrix: - .github/workflows/s3-iam-tests.yml: skip TestServiceAccountLifecycle/use_service_account_credentials only. The rest of the service-account suite passes; that one subtest depends on a separate credential-reload issue where new ABIA keys briefly register into accessKeyIdent but aren't persisted to the filer, so they vanish on the next reload. Out of scope for the #9157 GetFederationToken fix. * fix(credential): accept AWS IAM username chars in service-account IDs Gemini review on #9167 pointed out that ServiceAccountIdPattern's parent-user segment was more restrictive than an AWS IAM username: `[A-Za-z0-9_-]` vs. IAM's `[\w+=,.@-]`. Realistic usernames with `@`, `.`, `+`, `=`, or `,` (e.g. email-style principals) would fail validation at the filer store even though the embedded IAM API happily created them. Broaden the regex to `[A-Za-z0-9_+=,.@-]` (matching the AWS IAM spec at https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html) and add a table-driven test that locks the expansion in. * address PR review feedback on #9167 All five review items were valid; changes keyed to review bullets: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer swallows arbitrary policy-lookup failures. Only credential.ErrUserNotFound is treated leniently (the legacy-config SigV4 path); any other error now returns InternalError so we don't mint tokens with an incomplete policy set. - weed/credential/grpc/grpc_identity.go: GetUser translates gRPC NotFound back to credential.ErrUserNotFound so errors.Is(...) above matches for gRPC-backed stores, not just memory/filer-direct. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now validates the generated saId against credential.ValidateServiceAccountId before returning. Surfaces a client 400 with the offending ID instead of the opaque 500 that used to bubble up from the persistence layer. - weed/s3api/s3api_server_routing_test.go: seed a routing-test identity with a known AK/SK, sign TestRouting_GetFederationTokenAuthenticatedBody with aws-sdk-go v4.Signer so the request actually passes AuthSignatureOnly. Assert 503 ServiceUnavailable (from STSHandlers with no stsService) instead of just NotEqual(501) — 503 proves the dispatch reached STSHandlers.HandleSTSRequest. - test/s3/iam/s3_service_account_test.go: callIAMAPI signs with service="iam" instead of "s3" (SeaweedFS verifies against whichever service the client signed with, but "iam" is semantically correct). - weed/credential/validation_test.go: add positive rows for an uppercase parent (sa:ALICE:...) and a canonical hyphenated UUID suffix (sa:alice:123e4567-e89b-12d3-a456-426614174000). |
||
|
Chris LuandGitHub
|
0007245c7f |
fix(kafka): close late-joiner orphan race in consumer-group rebalance (#9162)
* fix(kafka): close late-joiner orphan race in consumer-group rebalance
The CI-observed orphan (one consumer with empty assignment after
`TestConsumerGroups/Rebalancing/MultipleConsumersJoin`) came from a
race in the group coordinator: once the leader had taken its member-
list snapshot in its JoinGroup response, a new member could still
arrive before the leader's SyncGroup landed. The gateway accepted the
stale SyncGroup, moved to Stable, and the late joiner's own SyncGroup
then served an empty Assignment from the Stable-state path — leaving
it silently unassigned with no further rebalance to fix it.
Three changes in `handleJoinGroup` / `handleSyncGroup` close the race:
- Late join during `CompletingRebalance` bumps the generation and
resets to `PreparingRebalance`, so the leader's in-flight SyncGroup
fails its generation check and the round restarts with the new
member in the snapshot.
- SyncGroup generation-mismatch returns `REBALANCE_IN_PROGRESS` (not
`ILLEGAL_GENERATION`) while the group is rebalancing, mirroring the
existing heartbeat fix — otherwise Sarama's `Consume()` tears down
on the stale SyncGroup instead of retrying.
- Leader SyncGroup verifies its assignment covers every current
member and rejects with `REBALANCE_IN_PROGRESS` otherwise, as a
belt-and-suspenders catch for joins that slip in between the
leader's JoinGroup reply and its SyncGroup without going through
`CompletingRebalance` state.
Verified: baseline reliably reproduces the orphan locally; with the
fix `TestConsumerGroups` passes end-to-end (53s total,
`MultipleConsumersJoin` 15-17s) and a 10-iteration stress loop against
the same gateway is 10/10 green with every consumer getting exactly
one partition.
* fix(kafka): clear stale Assignment when restarting a rebalance round
Review spot: the two restart paths added in the previous commit bumped
group.Generation and reset each member's State to Pending but left
member.Assignment populated with the prior generation's partitions.
The non-leader SyncGroup path only returns REBALANCE_IN_PROGRESS when
`member.Assignment` is empty (handleSyncGroup ~line 982). Leaving the
stale assignment in place means a member rejoining at the new
generation — before the leader's SyncGroup has published fresh
assignments — falls through that guard and is served its old
partitions from the pre-rebalance state.
Clear m.Assignment alongside m.State in both restart sites so the
guard fires and the member correctly re-enters the join/sync cycle.
Verified with a fresh-broker TestConsumerGroups run: 50.99s total,
MultipleConsumersJoin 15.25s, all four consumers each get exactly one
partition.
* fix(kafka): don't let empty leader assignments bypass coverage check
Review spot: the leader-assignment branch was gated on
`len(request.GroupAssignments) > 0`, so a leader SyncGroup that omitted
every current member (empty array with a non-empty group) fell through
to the server-side-assignment `else` branch and could move the group
Stable without the intended rebalance retry.
Drop the length guard. Whenever the caller is the leader, build the
assigned-member map and run the coverage check; if the assignment
omits any current member (including the all-empty case against a
non-empty group), bump the generation, reset to PreparingRebalance,
clear each member's Assignment, and return REBALANCE_IN_PROGRESS so
the leader rebuilds its snapshot and sends a complete assignment on
retry. The server-side-assignment branch (documented as "should not
happen with Sarama") is now only reachable for non-leader+non-empty
SyncGroups — a genuinely unexpected case — and keeps its existing
warning.
* revert: keep len(GroupAssignments) > 0 gate on leader-assign branch
The previous commit (
|
||
|
Chris LuandGitHub
|
49f62df2cf |
fix(shell): mergeVolumes hard-link safety and cleaner cleanup logging (#9163)
* fix(shell): skip hard-linked entries in fs.mergeVolumes Hard-linked entries share a chunk list with their siblings, but the filer's UpdateEntry only rewrites one entry at a time. Moving a chunk here leaves every other hard-linked sibling pointing at a fid that either gets deleted by the filer's own garbage step after UpdateEntry or by deleteMovedSourceNeedles (#9160) — either way, the siblings end up with dangling references. Skip the entry with a visible log line so operators know the file was bypassed and can handle it explicitly (copy-then-unlink, or dedup before merge). Detected via entry.HardLinkId being non-empty, which is the same signal the filer itself uses (weed/pb/filer_pb/filer.pb.go:451). Flagged by coderabbit on #9160 post-merge. * fix(shell): mergeVolumes suppresses 404 alongside 304 in source cleanup BatchDelete also returns StatusNotFound (404) for an already-deleted needle when ReadVolumeNeedle can't find it in the cookie-check path, not only StatusNotModified (304) from DeleteVolumeNeedle returning size 0. Both are benign races against a concurrent fsck purge or a replica that already reconciled, so don't clutter the output with "delete ... not found" warnings for them. Flagged by coderabbit on #9160 post-merge. * fix(shell): mergeVolumes merges hard-linked files via dedup on HardLinkId Hard-linked siblings share one chunk list through a KV blob keyed by HardLinkId (see weed/filer/filerstore_hardlink.go). UpdateEntry's setHardLink rewrites that blob and maybeReadHardLink overrides per-entry chunks with the blob's on every read, so a single UpdateEntry propagates new fids to every sibling automatically — the previous skip-hardlinks bailout was overly conservative and left hard-linked files stuck on merge-source volumes forever. Process each HardLinkId exactly once per run with a sync.Map so BFS workers in different directories synchronize without a global lock. First sibling carries the chunk move + UpdateEntry; later siblings find the id in the map and return — preventing the real race, which is two siblings trying to re-download an already-moved source needle or double-queue the same fid for deletion. Also address the log-spam review on deleteMovedSourceNeedles: an unreachable volume server returns one error per needle, so collapse multiple failures into a single per-server line with the first error as an example. |
||
|
Chris LuandGitHub
|
06ccd0e9fc |
fix(mount): flush dirty handles on Release when kernel skipped Flush (#9165)
* fix(mount): flush dirty handles on Release when kernel skipped Flush The FUSE protocol allows the kernel to send Release without a preceding Flush; file handles that reach Release with dirtyMetadata=true (notably deferred creates that never saw any write) would then have their pending filer CreateEntry dropped on the floor, leaving the mount and filer out of sync. Detect dirty handles in Release and call doFlush before tearing the handle down. Skip the fallback when an async flush is already pending so we don't double-submit. Flock-unlock Releases stay on the synchronous path so close()-time serialization is preserved. Adds TestReleaseFlushesDirtyCreateIfFlushWasSkipped covering the create-without-flush path. * address review: drop racy dirty-flag peek, let doFlush self-gate fh.dirtyMetadata / fh.asyncFlushPending are written from the periodic metadata flusher and async flush worker under fhLockTable, so the unsynchronized read in Release was a data race per the reviewer. Just call doFlush unconditionally on every Release; it already fast- paths the clean case (dirtyPages.FlushData early-returns when hasWrites is false, and the dirty-metadata branch short-circuits), so the extra call after a normal Flush is cheap while the no-Flush-before-Release path still recovers a deferred create. |
||
|
Chris LuandGitHub
|
eaab3ec5d0 |
fix(topology): drop per-disk task-type conflict map (#9147) (#9166)
* fix(topology): drop per-disk task-type conflict map (#9147) Different job types (Balance, ErasureCoding, Vacuum) operate on different volumes, so a per-disk cross-type exclusion adds no correctness guarantee beyond what HasAnyTask already enforces at task detection time. The conflict map turned this into a deadlock on small clusters: a single in-flight (or retrying) balance task would prune the source/destination disks from EC placement, dropping the candidate count below MinTotalDisks and permanently blocking auto-EC. Removing the map lets EC see all eligible disks. Per-volume safety is still guaranteed by HasAnyTask, and per-disk load shaping remains available via MaxConcurrentTasksPerDisk. * chore(topology): trim verbose comments from #9147 fix |
||
|
Chris LuandGitHub
|
caaa53aee3 |
fix(shell): ec.encode health-check key mismatch on K8s deployments (#9164)
Build freeVolumeCountMap using dn.Address so the key matches wdclient.Location.Url during the subsequent lookup. Keying by dn.Id silently filtered out every replica in deployments where dn.Id is a short name (e.g. Kubernetes StatefulSet pod name) while the location Url is a FQDN:port, causing "no healthy replicas" even with ample free capacity. Also filter replicas before marking volumes readonly so that a failed health check no longer strands volumes in readonly state. Fixes #9145 |
||
|
Chris LuandGitHub
|
e725eb4079 |
refactor(shell): run volume.fsck purge once per volume, after all replicas (#9159)
* refactor(shell): run volume.fsck purge once per volume, after all replicas The purge step in findExtraChunksInVolumeServers was nested inside the outer `for dataNodeId` loop, so it fired once per data-node iteration rather than once total. Two consequences: 1. The replica-intersection safety net was broken. The code marks a fid "found in all replicas" only after every replica has reported its orphans, but the purge ran after the first data node already, so fids contributed only by later replicas never got the `true` flag in time. Without `-forcePurging` that meant some legitimate orphans were never purged; with `-forcePurging` the flag was ignored so the bug was hidden. 2. Visible output got noisy: "purging orphan data for volume X..." printed 2-3 times per volume (N_datanodes * N_replicas RPCs to the same locations) since purgeFileIdsForOneVolume already fans out to every replica location via MasterClient.GetLocations. Split the work into two explicit phases: collect orphans from every replica first, then purge each volume once. Drop the per-replica loop around purgeFileIdsForOneVolume since it already handles all replicas internally. Keep the per-replica mark-writable loop (each replica's readonly bit has to be flipped before the purge RPC fans out to it). Also simplify the gating expression — `isSeveralReplicas && foundInAllReplicas` is redundant given the preceding `!isSeveralReplicas` branch — and replace `!(X > 0)` with the more idiomatic `len(X) == 0`. Related to #9116 follow-up on multiple fsck passes needed to fully clean a volume. * address review: per-replica readonly tracking, count-based intersection, defer-per-volume Three issues raised on the v1: 1. The readonly cleanup stored a single isReadOnlyReplicas[volumeId]=bool that flipped true if any replica was read-only, then the defer marked every replica in serverReplicas[volumeId] read-only on exit. If a volume had mixed replica modes (one RO, one RW), the originally-RW replica ended up RO after fsck returned. Track read-only state per replica in readOnlyServerReplicas[volumeId] and revert only those. 2. The defer inside the volumeId loop accumulated for the entire fsck run, so every volume we processed stayed writable until the whole command returned. Split the per-volume logic into purgeOneVolume so the defers unwind between volumes. 3. The intersection logic used a sticky bool that treated "seen on any 2 of 3 replicas" as "seen on all replicas" — a 3+-replica volume would get purged for fids only 2 replicas agreed on, which is what -forcePurging is supposed to opt into. Switch to a count-based map[fid]int compared against volumeReplicaCounts[volumeId], so we only purge without -forcePurging when every replica agrees. Also drop the now-unused serverReplicas map. |
||
|
Chris LuandGitHub
|
08a7502b2c |
fix(shell): error on missing volume id in fsck, mergeVolumes, vacuum (#9158)
* fix(shell): error on missing volume id in fsck, mergeVolumes, vacuum Three shell commands silently report success when -volumeId / -fromVolumeId / -toVolumeId names a volume the master doesn't know about: typos, already-deleted volumes, and stale scripts all look identical to a clean no-op, which is what made the confusion in #9116 take as long as it did to diagnose. - volume.fsck: filter at the per-datanode loop drops unknown ids and findExtraChunksInVolumeServers ends with totalOrphanChunkCount==0, printing "no orphan data". - fs.mergeVolumes: createMergePlan iterates only known volumes, so an unknown -fromVolumeId produces an empty plan and we print just the "max volume size: N MB" header (indistinguishable from "nothing to merge"). - volume.vacuum: the master's VacuumVolume RPC silently iterates matching volumes; a missing id returns success having done nothing. Validate the requested ids against the current topology up front and return an explicit "volume(s) not found on master: [X Y]" error. Also drop a stale duplicate `if err != nil` in volume.fsck.Do left over from a prior refactor. Surfaces #9116 follow-up from madalee-com. * address review: propagate reloadVolumesInfo error; dedupe vacuum missing ids - fs.mergeVolumes: c.reloadVolumesInfo's return was ignored. If the master is unreachable or VolumeList fails, c.volumes stays empty and the new validation block reports "fromVolumeId X not found on master" — masking the real connection/RPC failure. Return the wrapped error instead. - volume.vacuum: "volume.vacuum -volumeId 5,5,5" on a missing volume 5 listed [5 5 5] in the error. Collect missing ids in a set so each missing id appears once. * address review: reject fromVolumeId/toVolumeId values that overflow uint32 flag.Uint produces a uint (64-bit on amd64), and the existing cast to needle.VolumeId silently truncates to uint32. A typo like `-fromVolumeId=4294967297` would wrap to volume 1 and slip past every other validation, so the merge would run against a completely different volume than the operator intended. Bail out with an explicit error when the raw flag value exceeds the uint32 range, before the cast. |
||
|
Chris LuandGitHub
|
6bdd775963 |
feat(shell): fs.mergeVolumes deletes source needles after filer update (#9160)
* feat(shell): fs.mergeVolumes deletes source needles after filer update Before this change, mergeVolumes only copied chunks to the destination volume and updated the filer — the source needle sat untouched on its original volume as a silent orphan. Operators had to run a separate volume.fsck + volume.vacuum pass to actually reclaim the space, and #9116 (comment 4282692876) showed how that pipeline can look exactly like "mergeVolumes did nothing": the source volume keeps reporting its original size even though every chunk has been logically moved out. Clean up the source inline. For each entry, track the pre-move fids as they're captured, and after the UpdateEntry RPC commits, issue BatchDelete on every replica of each source volume. Key invariants: - Source fids are only deleted AFTER UpdateEntry succeeds; if the filer write fails we skip the cleanup for that entry so we never delete data the filer still references. - rewriteManifestChunk grew a fourth return value so nested manifest and sub-chunk moves propagate their moved-source list back to the top-level callsite. The outer manifest itself is recorded at the callsite, since only the callsite sees the pre-rewrite fid. - deleteMovedSourceNeedles logs errors but never returns them. Propagating would abort TraverseBfs mid-merge, stranding remaining entries; logging leaves the fallback path (fsck reconciles later) intact. - StatusNotModified from the volume server is expected whenever a concurrent fsck purge beat us to the delete or a replica already reconciled — don't warn on it. Readonly source volumes are already rejected up front by createMergePlan, so by the time we reach the delete the source is writable. If a replica's readonly bit has flipped since then the delete will fail and get logged; the user can re-run once they've fixed the replica (same failure mode as today's fsck purge). Fixes the space-not-reclaimed half of #9116. Related design discussion: #8589. * address review: cast r.Status to int in StatusNotModified compare http.StatusNotModified is an untyped constant so the compare works as written, but the int32/int mixed-type signal trips static analyzers and PR tooling. Cast explicitly and note why. |
||
|
Chris LuandGitHub
|
d2e64e85ce |
fix(log_buffer): back off disk-poll cadence when caught up to disk head (#9161)
Idle subscribers parked in the ResumeFromDiskError path were re-probing the in-memory buffer and disk every 250ms, emitting a "Notification timeout after ResumeFromDiskError, rechecking state" log line per tick even when nothing had changed. Once ReadFromDiskFn returns without advancing lastReadPosition, we know the disk has no data past the subscriber's current position. Switch to a 2s poll in that state so external disk writers (e.g. Schema Registry) are still re-detected on a bounded cadence, but idle CPU and log noise drop ~8x. Any progress (disk advance or notifyChan wakeup) clears the flag and restores the responsive 250ms tick for active readers. The per-timeout V(4) log is replaced by a single "Caught up to disk head" transition log. |
||
|
|
61c1735cdd |
build(deps): bump modernc.org/sqlite from 1.46.1 to 1.49.1 (#9155)
Bumps [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) from 1.46.1 to 1.49.1. - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.46.1...v1.49.1) --- updated-dependencies: - dependency-name: modernc.org/sqlite dependency-version: 1.49.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
49ce2b7326 |
build(deps): bump github.com/rclone/rclone from 1.73.1 to 1.73.5 (#9156)
Bumps [github.com/rclone/rclone](https://github.com/rclone/rclone) from 1.73.1 to 1.73.5. - [Release notes](https://github.com/rclone/rclone/releases) - [Changelog](https://github.com/rclone/rclone/blob/master/RELEASE.md) - [Commits](https://github.com/rclone/rclone/compare/v1.73.1...v1.73.5) --- updated-dependencies: - dependency-name: github.com/rclone/rclone dependency-version: 1.73.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
a890300eaf |
build(deps): bump cloud.google.com/go/pubsub from 1.50.1 to 1.50.2 (#9154)
Bumps [cloud.google.com/go/pubsub](https://github.com/googleapis/google-cloud-go) from 1.50.1 to 1.50.2. - [Release notes](https://github.com/googleapis/google-cloud-go/releases) - [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-cloud-go/compare/pubsub/v1.50.1...pubsub/v1.50.2) --- updated-dependencies: - dependency-name: cloud.google.com/go/pubsub dependency-version: 1.50.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
6642a64d2b |
build(deps): bump github.com/go-git/go-billy/v5 from 5.6.2 to 5.8.0 (#9152)
Bumps [github.com/go-git/go-billy/v5](https://github.com/go-git/go-billy) from 5.6.2 to 5.8.0. - [Release notes](https://github.com/go-git/go-billy/releases) - [Commits](https://github.com/go-git/go-billy/compare/v5.6.2...v5.8.0) --- updated-dependencies: - dependency-name: github.com/go-git/go-billy/v5 dependency-version: 5.8.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
 Lars LehtonenandGitHub
|
93d604d799 | chore(weed/s3api/policy): prune unused test functions (#9150) | ||
|
|
dec09d1484 |
build(deps): bump github.com/aws/aws-sdk-go-v2 from 1.41.5 to 1.41.6 (#9153)
Bumps [github.com/aws/aws-sdk-go-v2](https://github.com/aws/aws-sdk-go-v2) from 1.41.5 to 1.41.6. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](https://github.com/aws/aws-sdk-go-v2/compare/v1.41.5...v1.41.6) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2 dependency-version: 1.41.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
25d7f2c569 |
build(deps): bump docker/build-push-action from 6 to 7 (#9151)
Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 6 to 7. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](https://github.com/docker/build-push-action/compare/v6...v7) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
86c5e815d2 |
fix(kafka): make consumer-group rebalancing work end-to-end (#9143)
* fix(kafka): make consumer-group rebalancing work end-to-end
TestConsumerGroups was failing every run since the job was added
(2026-04-17) but the failures were masked by a `|| echo ...` trailer on
the go test invocation, so the CI reported green. Removing the mask
exposes several real bugs in the gateway's group-coordinator code:
1. JoinGroup deduplicated members by ClientID, which collapsed two
Sarama consumers that share the default ClientID ("sarama") into a
single member slot and broke rebalancing. Key dedup off the TCP
ConnectionID instead; keep ClientID on the member for DescribeGroup
fidelity.
2. Every JoinGroup replaced the *GroupMember struct, wiping the
Assignment the leader had just published in its SyncGroup and leaving
non-leader consumers with 0 partitions after a rebalance. Update the
existing member in place on rejoin.
3. Non-leader SyncGroup returned an empty assignment while the leader
was mid-rebalance, so consumers silently came up with no partitions.
Return REBALANCE_IN_PROGRESS when the group is not Stable so Sarama
retries the join/sync cycle (4 retries x 2s backoff by default).
4. Heartbeat returned ILLEGAL_GENERATION on a gen mismatch even when
the group was in PreparingRebalance/CompletingRebalance. Return
REBALANCE_IN_PROGRESS in that case so the heartbeat loop cleanly
cancels the session instead of tearing it down on a fatal error.
5. LeaveGroup parser only handled v0-v2. Sarama at V2_8_0_0 sends v3
(Members array) by default, so the gateway silently rejected the
request as InvalidGroupID and dead consumers stayed in the group as
phantom leaders. Added v3 (Members array) and v4+ (flexible/compact/
tagged-fields) parsing.
The rebalancing integration tests called Consume() once per consumer,
which cannot survive a rebalance (heartbeat RBIP cancels the session
and Consume() returns - this is documented Sarama behaviour; callers
are expected to loop). Added a runConsumeLoop helper and used it in the
four affected sub-tests. RebalanceTestHandler.Setup now overwrites
stale entries in its assignments channel so the test observes the
settled post-rebalance snapshot rather than whatever arrived first.
* fix(kafka): address PR review feedback
- JoinGroup now snapshots existing members before mutating and restores
the snapshot on INCONSISTENT_GROUP_PROTOCOL rollback. Previously the
rollback path always deleted the entry, corrupting group state when
an existing member rejoined with an incompatible protocol.
- handleLeaveGroup iterates request.Members instead of processing only
the first entry, so v3+ batch departures (KIP-345 style) correctly
remove every listed member and build a per-member response. A single
group-state transition runs after the loop, with leader election
only triggered if the actual group leader was among the departures.
- Added buildLeaveGroupFlexibleResponse for v4+ clients. The parser
already decoded flexible versions, but the response still went out in
non-flexible encoding (4-byte array lengths, 2-byte strings, no
tagged fields), which v4+ clients could not parse. Route flexible
versions through the new builder; v1-v3 keep buildLeaveGroupFullResponse.
- BasicFunctionality gives each consumer its own
ConsumerGroupHandler/ready channel. The previous shared handler
closed ready once, so readyCount advanced to numConsumers from a
single signal; the test could proceed without the other consumers
actually reaching Setup.
- RebalanceTestHandler.assignments is now a size-1 channel, so readers
always observe the most recent rebalance snapshot instead of an
intermediate one from an earlier round.
|
||
 
|
f4ce2be875 |
doc: P14 S8 final bounded close — evidence matrix + P15 handoff (#9142)
* doc: P14 S8 final bounded close — evidence matrix + P15 handoff
Adds the six S8 closure deliverables consolidating S4-S7 evidence,
classifying V2 scenarios, and mapping residual product gaps onto
canonical P15 tracks (per v3-phase-15-product-plan.md §4).
New docs:
- v3-phase-14-s8-assignment.md — S8 execution contract.
- v3-phase-14-s8-final-bounded-close.md — bounded P14 target,
accepted topology, reject conditions.
- v3-phase-14-s8-evidence-matrix.md — 16 claims × {L0, L1, L2, L3,
Status, Residual}. 15 PROVEN, 1 PARTIAL (Claim 15 fence
quantitative bound, P14 internal follow-up). Rounds 2-3 architect
corrections: Claim 10 / 12 L2 narrowed; Claim 6 refresh gap closed
by the new L1 test (see companion commit in seaweed_block).
- v3-phase-14-s8-v2-scenario-classification.md — every V2 scenario
mapped to RUNNABLE-P14 / BLOCKED-FRONTEND / BLOCKED-OPS /
BLOCKED-HA / BLOCKED-PERF / PORT-MECHANISM; scenario YAMLs kept
as L3 shape, not executed evidence.
- v3-phase-14-s8-p15-handoff.md — 11 rows (10 canonical P15 tracks
+ 1 P14 internal follow-up anchored to Claim 15 PARTIAL); §4
integrity check split by row class.
- v3-phase-14-s8-closure.md — final P14 closure statement matching
the close doc §10 wording; explicit non-goals; all 9 P15 tracks
named with canonical numbering.
No claim of CSI / frontend / migration / security / performance /
production readiness. Every product gap is handed off with a
concrete first-proof gate.
Companion: seaweed_block commit adds the IntentRefreshEndpoint L1
route test that closes Claim 6.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* doc: P14 S8 — resolve port-now doc conflict (CodeRabbit #4)
final-bounded-close.md §7 previously said "Port now: testrunner,
scenarios, component harness, qa_block, learn/test" while
v2-scenario-classification.md §2 says S8 does NOT port testrunner
machinery and defers all actual porting to P15.
Align final-bounded-close.md §7 with classification: section
renamed "Classify now (S8 scope), port deferred to P15". Every
item now states which P15 track actually owns the port (Final Gate
or T1 Frontend + Data Path as applicable).
No scope expansion; no new handoff gap. Pure doc-consistency fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Chris LuandGitHub
|
f1d5f31a93 |
fix(mount): retry saveEntry on transient filer errors; stop mismapping Canceled to EIO (#9141)
* fix(mount): retry saveEntry on transient filer errors, stop mismapping Canceled to EIO When the mount's gRPC connection to the filer flaps (e.g. a transient restart or network blip), every in-flight setattr/utimes/chmod/xattr/ rename-driven saveEntry returns "code = Canceled desc = grpc: the client connection is closing" at the same instant. Two bugs in saveEntry then turned each of those into a hard EIO for the user: 1. The error was wrapped with fmt.Errorf(... %v ...) before being passed to grpcErrorToFuseStatus. %v stringifies the status, so status.FromError could no longer unwrap the gRPC code and the Canceled→ETIMEDOUT branch in the classifier never fired; every Canceled error fell through to the default EIO. 2. saveEntry issued a single streamUpdateEntry call with no retry, unlike doFlush which already wraps its CreateEntry in retryMetadataFlush. One stream flap therefore propagated straight to the FUSE caller instead of being ridden out across the 4-attempt / ~7s backoff window. Wrap the UpdateEntry call in retryMetadataFlush (matching doFlush and completeAsyncFlush) and switch the wrap verb to %w so the classifier can still see the gRPC code. This recovers transient closes silently and, if retries are exhausted, returns ETIMEDOUT instead of EIO. Reported by rclone users in #9139 where a large concurrent copy (hundreds of .partial uploads per filer flap) surfaced as walls of EIOs because each .partial rename's post-setattr hit saveEntry at the worst possible moment. * mount: skip saveEntry retries on permanent filer errors Address gemini-code-assist review on #9141: blindly retrying every UpdateEntry failure with exponential backoff means interactive FUSE ops like chmod/utimes/xattr can hang for ~7s before surfacing clearly permanent errors (NotFound, PermissionDenied, InvalidArgument, etc.). Introduce retryMetadataFlushIf, a variant of retryMetadataFlush that accepts a shouldRetry predicate, and an isRetryableFilerError classifier that short-circuits on a conservative whitelist of terminal gRPC codes. Transient errors (Canceled / Unavailable / DeadlineExceeded / ResourceExhausted / Internal) and non-gRPC errors still retry, so the original fix for #9139 (rclone EIO burst during filer connection flaps) is preserved. |
||
|
Chris Lu
|
8857dbfb74 |
fix(test): drop host port mapping from risingwave catalog test to kill TOCTOU flake
The random host port allocated by MustFreeMiniPorts was released before docker run bound it, occasionally losing the race to another process and failing with "address already in use". The sidecar already reaches RisingWave via shared netns (--network container:...), so the host -p mapping and the corresponding WaitForPort check were unused. |
||
|
Chris LuandGitHub
|
9a6b566fb1 |
fix(shell): volume.fsck keeps going past a single broken chunk manifest (#9140)
* fix(shell): volume.fsck no longer aborts on a single broken chunk manifest Previously a single entry whose chunk-manifest could not be read (e.g. the manifest needle was missing or its sub-chunks pointed at a now-gone volume) caused collectFilerFileIdAndPaths to return immediately with "failed to ResolveChunkManifest". The whole fsck run failed, so an operator with even one corrupted file could not use volume.fsck to find or clean up unrelated orphan needles on other volumes — they had to locate and delete the bad entries first, blind, with no help from fsck. Log the resolution failure with the entry path, fall back to recording the top-level chunk fids the entry references (data fids and manifest fids themselves; sub-chunks behind the unresolvable manifest stay unknown), and keep traversing. Track the count of unresolved entries on the command struct and refuse -reallyDeleteFromVolume for the run when the count is non-zero, since the in-use fid set is incomplete and a purge could otherwise delete live sub-chunks behind the broken manifest. Read-only fsck still produces a useful (if conservatively over-reported) orphan listing so the operator can see and fix the broken entries first, then re-run with apply. Discovered while diagnosing #9116. * address review: use callback ctx and atomic counter - Pass the BFS callback's ctx to ResolveChunkManifest so a Ctrl+C / first-error cancellation propagates into the manifest fetch instead of using context.Background(). - TraverseBfs runs the callback across K=5 worker goroutines (filer_pb/filer_client_bfs.go), so the unresolvedManifestEntries field on commandVolumeFsck is shared across workers and was racing. Switch it to atomic.Int64 with Add/Load. * address review: reset counter per Do(), pass through ctx errors - commandVolumeFsck is a singleton registered in init() and reused across shell invocations. Without resetting the unresolved-manifest counter at the top of Do(), a single failed run permanently suppressed -reallyDeleteFromVolume in the same shell session. Reset to 0 right after flag parsing. - Treating context cancellation as manifest corruption was wrong: a Ctrl+C or deadline mid-traversal would inflate the counter and emit misleading "manifest broken" warnings for entries that were never examined. Detect context.Canceled / context.DeadlineExceeded and return the error so the BFS unwinds cleanly. Not changing the findMissingChunksInFiler branch's purgeAbsent / applyPurging gating: that path checks recorded filer fids against volume idx files, and a broken-manifest entry's recorded manifest fid will fail the existence check and get purged — which is the cleanup the operator wants for those entries. Adding a gate would block the exact use case the warning points them at. |
||
|
Chris Lu
|
3ff92f797d | 4.21 4.21 | ||
|
Chris LuandGitHub
|
a8ba9d106e |
peer chunk sharing 7/8: tryPeerRead read-path hook (#9136)
* mount: batched announcer + pooled peer conns for mount-to-mount RPCs * peer_announcer.go: non-blocking EnqueueAnnounce + ticker flush that groups fids by HRW owner, fans out one ChunkAnnounce per owner in parallel. announcedAt is pruned at 2× TTL so it stays bounded. * peer_dialer.go: PeerConnPool caches one grpc.ClientConn per peer address; the announcer and (next PR) the fetcher share it so steady-state owner RPCs skip the handshake cost entirely. Bounded at 4096 cached entries; shutdown conns are transparently replaced. * WFS starts both alongside the gRPC server; stops them on unmount. * mount: wire tryPeerRead via FetchChunk streaming gRPC Replaces the HTTP GET byte-transfer path with a gRPC server-stream FetchChunk call. Same fall-through semantics: any failure drops through to entryChunkGroup.ReadDataAt, so reads never slow below status quo. * peer_fetcher.go: tryPeerRead resolves the offset to a leaf chunk (flattening manifests), asks the HRW owner for holders via ChunkLookup, then opens FetchChunk on each holder in LRU order (PR #5) until one succeeds. Assembled bytes are verified against FileChunk.ETag end-to-end — the peer is still treated as untrusted. Reuses the shared PeerConnPool from PR #6 for all outbound gRPC. * peer_grpc.go: expose SelfAddr() so the fetcher can avoid dialing itself on a self-owned fid. * filehandle_read.go: tryPeerRead slot between tryRDMARead and entryChunkGroup.ReadDataAt. Gated by option.PeerEnabled and the presence of peerGrpcServer (the single identity test). Read ordering with the feature enabled is now: local cache -> RDMA sidecar -> peer mount (gRPC stream) -> volume server One port, one identity, one connection pool — no more HTTP bytecast. * test(fuse_p2p): end-to-end CI test for peer chunk sharing Adds a FUSE-backed integration test that proves mount B can satisfy a read from mount A's chunk cache instead of the volume tier. Layout (modelled on test/fuse_dlm): test/fuse_p2p/framework_test.go — cluster harness (1 master, 1 volume, 1 filer, N mounts, all with -peer.enable) test/fuse_p2p/peer_chunk_sharing_test.go — writer-reader scenario The test (TestPeerChunkSharing_ReadersPullFromPeerCache): 1. Starts 3 mounts. Three is the sweet spot: with 2 mounts, HRW owner of a chunk is self ~50 % of the time (peer path short-circuits); with 3+ it drops to ≤ 1/3, so a multi-chunk file almost certainly exercises the remote-owner fan-out. 2. Mount 0 writes a ~8 MiB file, then reads it back through its own FUSE to warm its chunk cache. 3. Waits for seed convergence (one full MountList refresh) plus an announcer flush cycle, so chunk-holder entries have reached each HRW owner. 4. Mount 1 reads the same file. 5. Verifies byte-for-byte equality AND greps mount 1's log for "peer read successful" — content matching alone is not proof (the volume fallback would also succeed), so the log marker is what distinguishes p2p from fallback. Workflow .github/workflows/fuse-p2p-integration.yml triggers on any change to mount/filer peer code, the p2p protos, or the test itself. Failure artifacts (server + mount logs) are uploaded for 3 days. Mounts run with -v=4 so the tryPeerRead success/failure glog messages land in the log file the test greps. |
||
|
Chris LuandGitHub
|
73f10fa528 |
peer chunk sharing 6/8: announce queue + batched flush (#9135)
mount: batched announcer + pooled peer conns for mount-to-mount RPCs * peer_announcer.go: non-blocking EnqueueAnnounce + ticker flush that groups fids by HRW owner, fans out one ChunkAnnounce per owner in parallel. announcedAt is pruned at 2× TTL so it stays bounded. * peer_dialer.go: PeerConnPool caches one grpc.ClientConn per peer address; the announcer and (next PR) the fetcher share it so steady-state owner RPCs skip the handshake cost entirely. Bounded at 4096 cached entries; shutdown conns are transparently replaced. * WFS starts both alongside the gRPC server; stops them on unmount. |
||
|
Chris LuandGitHub
|
fe9ca35bbd |
peer chunk sharing 5/8: mount chunk-directory shard (#9134)
mount: tier-2 chunk directory + FetchChunk streaming on one gRPC port Collapses the old two-port design (HTTP peer-serve + separate gRPC directory) into a single gRPC service that handles every mount-to- mount exchange: ChunkAnnounce, ChunkLookup, and the new FetchChunk byte stream. * peer_directory.go: fid -> holders shard, HRW-gated; returns holders in LRU order; capacity-bounded; Sweep handles eviction under write-lock while Lookup runs under RLock (hot path is concurrent). * peer_grpc.go: single MountPeer gRPC server implementing all three RPCs. FetchChunk frames bytes at 1 MiB per Send so the default 4 MiB message cap does not constrain chunk size; cache miss returns gRPC NOT_FOUND so clients distinguish miss from transport error. Reuses pb.NewGrpcServer for consistent keepalive + msg-size tuning. * peer_bytepool.go: sync.Pool wrapper around *[]byte that the server uses to avoid a fresh 8 MiB allocation per FetchChunk call. * WFS wiring starts the gRPC server on option.PeerListen (the single peer port) using the advertise address resolved in PR #3 as the HRW identity. A background sweeper evicts expired directory entries every 60 s. |
||
|
Chris LuandGitHub
|
8a6348d3e9 |
peer chunk sharing 4/8: mount registrar + HRW owner selection (#9133)
* 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). * mount: add peer chunk sharing options + advertise address resolver First cut at the peer chunk sharing wiring on the mount side. No functional behavior yet — this PR just introduces the option fields, the -peer.* flags, and the helper that resolves a reachable host:port from them. The server implementation arrives in PR #5 (gRPC service) and the fetcher in PR #7. * ResolvePeerAdvertiseAddr: an explicit -peer.advertise wins; else we use -peer.listen's bind host if specific; else util.DetectedHostAddress combined with the port. This is what gets registered with the filer and announced to peers, so wildcard binds no longer result in unreachable identities like "[::]:18080". * Option fields: PeerEnabled, PeerListen, PeerAdvertise, PeerRack. One port handles both directory RPCs and streaming chunk fetches (see PR #1 FetchChunk proto), so there is no second -peer.grpc.* flag — the old HTTP byte-transfer path is gone. * New flags on weed mount: -peer.enable, -peer.listen (default :18080), -peer.advertise (default auto), -peer.rack. * mount: register with filer and maintain HRW seed view Adds the mount-side tier-1 client. On startup the mount calls MountRegister with its advertise address (PR #3) and keeps both the filer entry and the local seed view fresh via background tickers (30 s register / 30 s list, 90 s filer TTL). * peer_hrw.go: pure rendezvous-hashing helper picking a single owner per fid via top-1 HRW. Adding or removing one seed moves only ~1/N fids. * peer_registrar.go: heartbeat + list poller. Seeds() returns the slice directly (no per-call copy) since listOnce atomically swaps; background RPCs bind their context to Stop() so unmount doesn't hang on a slow filer. * WFS wiring uses ResolvePeerAdvertiseAddr from PR #3 for the identity registered with the filer. No HTTP server, no second port — one reachable address represents the mount. * mount: broadcast MountRegister/MountList to every filer Previously the registrar called through wfs.WithFilerClient, which only reaches whichever filer the WFS filer-client session happens to be on. That meant two mounts pointing at different filers would never see each other: the filer mount registries are in-memory and per-filer (no filer-to-filer sync), so each mount's MountList only returned peers that had also registered through the same filer. This commit makes the registrar multi-filer aware: * NewPeerRegistrar now takes the full FilerAddresses slice and a per-filer dial function. The old single-filer peerFilerClient interface is gone. * registerOnce fans a MountRegister RPC out to every filer in parallel. Succeeds if at least one filer accepted — an unreachable filer is tolerated, logged, and retried on the next heartbeat. * listOnce polls every filer's MountList in parallel and merges the responses by peer_addr, keeping the newest LastSeenNs on duplicates. Mounts talking to different filers therefore converge once every filer has been polled once. The merged-list property is what lets a fleet of mounts spread across multiple filers still form a single HRW seed view. Each filer only ever sees the subset of mounts that heartbeat through it, but the registrar reconstructs the union client-side. New unit tests guard both properties: - RegisterBroadcastsToAllFilers: one registerOnce hits all N filers. - ListMergesAcrossFilers: mount-a on filer-1 and mount-b on filer-2 both appear in the merged seed set. - ListMergeKeepsNewestLastSeen: the same mount reported by two filers collapses to one entry with the freshest timestamp. |
||
|
Chris LuandGitHub
|
af1e571297 |
peer chunk sharing 3/8: mount peer-serve HTTP endpoint (#9132)
* 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). * mount: add peer chunk sharing options + advertise address resolver First cut at the peer chunk sharing wiring on the mount side. No functional behavior yet — this PR just introduces the option fields, the -peer.* flags, and the helper that resolves a reachable host:port from them. The server implementation arrives in PR #5 (gRPC service) and the fetcher in PR #7. * ResolvePeerAdvertiseAddr: an explicit -peer.advertise wins; else we use -peer.listen's bind host if specific; else util.DetectedHostAddress combined with the port. This is what gets registered with the filer and announced to peers, so wildcard binds no longer result in unreachable identities like "[::]:18080". * Option fields: PeerEnabled, PeerListen, PeerAdvertise, PeerRack. One port handles both directory RPCs and streaming chunk fetches (see PR #1 FetchChunk proto), so there is no second -peer.grpc.* flag — the old HTTP byte-transfer path is gone. * New flags on weed mount: -peer.enable, -peer.listen (default :18080), -peer.advertise (default auto), -peer.rack. |
||
|
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
|
d7d834b8f9 |
peer chunk sharing 1/8: proto definitions (#9130)
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. |
||
|
Chris LuandGitHub
|
6787a4b4e8 |
fix kafka gateway and consumer group e2e flakes (#9129)
* fix(test): reduce kafka gateway and consumer group flakes * fix(kafka): make broker health-check backoff respect context Replace time.Sleep in the retry loop with a select on bc.ctx.Done() and time.After so the backoff is interruptible during shutdown, per review feedback on PR #9129. * fix(kafka): guard broker HealthCheck against nil client Return the same "broker client not connected" error used by the other exported BrokerClient methods instead of panicking on a partially initialized client, per CodeRabbit review feedback on PR #9129. |
||
|
Chris Lu
|
6832b9945b |
ci(s3tests): install libxml2/libxslt dev headers before pip install
ceph/s3-tests pins lxml without an upper bound. When pip picks a release whose prebuilt wheel isn't published for Python 3.9 on the runner, it falls back to sdist and fails without libxml2-dev / libxslt1-dev. |
||
|
Chris LuandGitHub
|
1c130c2d47 |
fix(mount): close inodeLocks cleanup race that let two flock holders coexist (#9128)
* fix(mount): close inodeLocks cleanup race that allowed two flock holders
PosixLockTable.getOrCreateInodeLocks released plt.mu before the caller
acquired il.mu. A concurrent maybeCleanupInode could delete the map
entry in that window; the first caller would then insert its lock into
the orphaned inodeLocks while a later caller created a fresh entry in
the map, so findConflict never observed the orphaned lock and two
owners could simultaneously believe they held the same exclusive flock.
This matches the flaky CI failure seen in
TestPosixFileLocking/ConcurrentLockContention:
Error: Should be empty, but was [worker N: flock overlap detected with 2 holders]
Mark removed inodeLocks as dead under plt.mu+il.mu, and have SetLk /
SetLkw recheck the flag after locking il.mu, refetching the live entry
from the map when orphaned. Also delete the map entry only if it still
points to this il, so a racing recreate is not clobbered.
Adds TestConcurrentFlockChurnPreservesMutualExclusion: 16 goroutines x
500 flock/unflock iterations on one inode. Reliably reports 500+
overlaps per run before the fix; clean across 100 race-enabled runs
after.
* fix(mount): extend dead-flag contract to GetLk and self-heal primitives
Address review feedback on the initial cleanup-race fix:
1. GetLk had the same stale-pointer bug as SetLk. A caller could grab
an inodeLocks pointer, have cleanup orphan it and a replacement il
receive a conflicting lock, then answer F_UNLCK off the empty dead
pointer. Add the same dead recheck + refetch loop.
2. getOrCreateInodeLocks and getInodeLocks now treat a dead map entry
as defective: the former replaces it with a fresh inodeLocks, the
latter drops it and returns nil. Production cannot reach that state
(maybeCleanupInode atomically deletes under plt.mu when it sets
dead), but the hardening guarantees the SetLk / SetLkw / GetLk
retry loops always make progress even if a future refactor reorders
those operations, and it lets the white-box tests set up a stale
dead entry without spinning.
3. Strengthen the regression suite:
- TestSetLkRetriesPastDeadInodeLocks: deterministic white-box test
that installs a dead il in the map and asserts SetLk routes the
new lock into a fresh il (not the orphan), that GetLk reports the
resulting conflict, and that a different-owner acquire is rejected
with EAGAIN.
- TestGetInodeLocksEvictsDeadEntry: verifies both map-read primitives
drop or replace dead entries.
- TestConcurrentFlockChurnPreservesMutualExclusion: replace the
timing-fragile Add(1)-and-check counter with a Swap+CAS detector.
Each worker claims a slot after SetLk OK and releases it before
UN, flagging both an observed predecessor and a lost CAS on
release. Against a reverted fix the detector fires 1000+ times per
run; with the fix clean across 100 race-enabled iterations.
* test(mount): fail fast on unexpected SetLk statuses in churn loop
The stress test blindly spun on any non-OK SetLk status and discarded
the unlock return. If SetLk ever returns something other than OK or
EAGAIN (e.g. after a future refactor introduces a new error), the
acquire loop would spin forever and an unlock failure would be
silently swallowed.
Capture the acquire status, retry only on the expected EAGAIN, and
assert unlock returns OK. Use t.Errorf + return (not t.Fatalf) because
the checks run on worker goroutines where FailNow is unsafe. The
Swap+CAS overlap detector is unchanged.
|
||
|
Chris LuandGitHub
|
c1ccbe97dd |
feat(filer.backup): -initialSnapshot seeds destination from live tree (#9126)
* feat(filer.backup): -initialSnapshot seeds destination from live tree Replaying the metadata event log on a fresh sync only leaves files that still exist on the source at replay time: any entry that was created and later deleted is replayed as a create/delete pair and never materializes on the destination. Users who wipe the destination and re-run filer.backup therefore see "only new files" instead of a full backup, even when -timeAgo=876000h is passed and the subscription genuinely starts from epoch (ref discussion #8672). Add a -initialSnapshot opt-in flag: when set on a fresh sync (no prior checkpoint, -timeAgo unset), walk the live filer tree under -filerPath via TraverseBfs and seed the destination through sink.CreateEntry, then persist the walk-start timestamp as the checkpoint and subscribe from there. Capturing the timestamp before the walk lets the subscription catch any create/update/delete racing with the walk — sink CreateEntry is idempotent across the builtin sinks so replay is safe. Honors existing -filerExcludePaths / -filerExcludeFileNames / -filerExcludePathPatterns filters and skips /topics/.system/log the same way the subscription path does. Also log "starting from <t> (no prior checkpoint)" instead of a misleading "resuming from 1970-01-01" when the KV has no stored offset. * fix(filer.backup): guard initialSnapshot counters under TraverseBfs workers TraverseBfs fans the callback out across 5 worker goroutines, so the entryCount / byteCount updates and the 5-second progress-log gate in runInitialSnapshot were racing. Switch the counters to atomic.Int64 and protect the lastLog check/update with a short-scoped mutex so the heavy sink.CreateEntry call stays outside the critical section. Flagged by gemini-code-assist on #9126; verified with go test -race. * fix(filer.backup): harden initialSnapshot against transient errors and path edge cases Three review items from CodeRabbit on #9126: 1. getOffset errors no longer leave isFreshSync=true. Before, a transient KV read failure would cause runFilerBackup's retry loop to redo the full -initialSnapshot walk on every retry. Treat any offset-read error as "not fresh" so the snapshot only runs when we've verified there really is no prior checkpoint. 2. initialSnapshotTargetKey now normalizes sourcePath to a trailing- slash base before stripping the prefix, so edge cases where sourceKey equals sourcePath (trailing-slash mismatch or root-entry emission) no longer index past the end. Unit tests cover both forms. 3. Documented the TraverseBfs-enumerates-excluded-subtrees performance characteristic on runInitialSnapshot, since pruning requires a separate change to TraverseBfs itself. * fix(filer.backup): retry setOffset after initialSnapshot to avoid full re-walks If the snapshot walk finishes but the subsequent setOffset fails, the retry loop in runFilerBackup will re-enter doFilerBackup with an empty checkpoint and run the full BFS again — on a multi-million-entry tree that's hours of wasted work over a 100-byte KV write. Retry the write a handful of times with exponential backoff before giving up, and log loudly at the final failure (with snapshotTsNs + sinkId) so operators recognize the symptom instead of guessing at mysterious repeated walks. Nitpick raised by CodeRabbit on #9126. * fix(filer.backup): initialSnapshot ignore404, skew margin, exclude dir-entry itself Three review items from CodeRabbit on #9126: 1. ignore404Error now threads into runInitialSnapshot. If a file is listed by TraverseBfs and then deleted before CreateEntry reads its chunks, the follow path already ignores 404s — the snapshot path was aborting and triggering a full re-walk. Treat an ignorable 404 as "skip this entry, continue." 2. snapshotTsNs now uses `time.Now() - 1min` instead of `time.Now()`. Metadata events are stamped server-side, so a fast backup-host clock could skip events that fire during or right after the walk. Matches the 1-minute margin meta_aggregator.go applies on initial peer traversal; duplicate replay is harmless because CreateEntry is idempotent. 3. Exclude checks now run against the entry's own full path, not just its parent. A walked directory whose full path matches SystemLogDir or -filerExcludePaths was being seeded to the destination; only its descendants were being skipped. Verified with a manual repro where -filerExcludePaths=/data/skipdir now keeps the skipdir entry itself off the destination. * refactor(filer): share destKey helper between buildKey and initialSnapshot Extract destKey(dataSink, targetPath, sourcePath, sourceKey, mTime) from buildKey in filer_sync.go. Both the event-log path (buildKey) and the initialSnapshot walk (initialSnapshotTargetKey) now go through the same helper, so a walk-seeded file and an event-replayed file always resolve to the same destination key. As a bonus, buildKey picks up the defensive trailing-slash normalization that initialSnapshotTargetKey introduced — no more index-past-end risk when sourceKey happens to equal sourcePath. Also tightens the mTime lookup to guard against nil Attributes (caught by an existing test against buildKey when I first moved the lookup out of the incremental branch). |
||
|
Chris LuandGitHub
|
d57fc67022 |
fix(shell): fs.mergeVolumes now rewrites manifest chunks for large files (#9127)
* fix(shell): fs.mergeVolumes now rewrites manifest chunks for large files Previously fs.mergeVolumes skipped any chunk whose IsChunkManifest flag was true, printing "Change volume id for large file is not implemented yet" and continuing. Because the BFS traversal only looks at top-level entry.Chunks, sub-chunks referenced inside a manifest were never considered either. For any file stored as a chunk manifest (large files go this path), chunks in the source volume stayed put, leaving behind a few MB of live data that vacuum and volume.deleteEmpty couldn't clean up. This change resolves each manifest chunk recursively, moves any sub-chunk whose volume id is in the merge plan via the existing moveChunk path, and re-serializes the manifest. If the manifest chunk itself lives in a source volume, or any sub-chunk moved, the new manifest blob is uploaded to a freshly assigned file id (the old needle becomes orphaned and is reclaimed by vacuum like any other moved chunk). Fixes #9116. * address review: batch UpdateEntry, fix dry-run, defer restore, avoid source volumes - Call UpdateEntry once per entry after the chunk loop instead of once per moved chunk (gemini nit). - In dry-run mode, mark anySubChanged when a sub-chunk in the plan is encountered and return changed=true after printing "rewrite manifest", so nested manifests also surface their would-rewrites (gemini nit). - Defer filer_pb.AfterEntryDeserialization so the manifest chunk list is restored even when proto.Marshal fails (coderabbit nit). - Reject AssignVolume results whose file id lands on a volume that is a source in the merge plan, and retry — otherwise the replacement manifest could be written to the volume being emptied (coderabbit). |
||
|
96af27a131 |
feat(shell): add fs.distributeChunks command for even chunk distribution (#9117)
* feat(shell): add fs.distributeChunks command for even chunk distribution
Add a new weed shell command that redistributes a file's chunks evenly
across volume server nodes.
Supports three distribution modes via -mode flag:
- primary: balance chunk ownership across nodes (default)
- replica: balance both ownership and replica copies
- round-robin: assign chunks by offset order for sequential read
optimization (chunk[0]->A, chunk[1]->B, chunk[2]->C, ...)
Additional options:
- -nodes=N to target specific number of nodes
- -apply to execute (dry-run by default)
Usage:
fs.distributeChunks -path=/buckets/file.dat
fs.distributeChunks -path=/buckets/file.dat -mode=round-robin -apply
fs.distributeChunks -path=/buckets/file.dat -mode=replica -apply
fs.distributeChunks -path=/buckets/file.dat -nodes=5 -apply
* fix(shell): improve fs.distributeChunks robustness and code quality
- Propagate flag parse errors instead of swallowing them (return err)
- Handle nil chunk.Fid by falling back to legacy FileId string parsing
- Simplify node membership check using slices.Contains
* fix(shell): fix dead round-robin print loop in fs.distributeChunks
The loop was computing targetNode with sc.index%totalNodes (original
chunk index) instead of the sequential position, and discarding it via
_ = targetNode without printing anything. Replace with a correct loop
using pos%totalNodes and actually print the first 12 node assignments.
* fix(shell): compute replication/collection per-chunk in fs.distributeChunks
Previously replication and collection were derived once from chunks[0]
and reused for all moves, causing wrong volume placement for chunks
belonging to different volumes or collections. Now each chunk looks up
its own volumeInfoMap entry immediately before calling operation.Assign.
* fix(shell): prefer assignResult.Auth JWT over local signing key in fs.distributeChunks
When the master returns an Auth token in the Assign response, use it
directly for the upload instead of generating a new JWT from the local
viper signing key. Fall back to local key generation only when Auth is
empty, matching the pattern used by other upload paths.
* fix(shell): add timeout and error handling to delete requests in fs.distributeChunks
The delete loop was ignoring http.NewRequest errors and had no timeout,
risking a nil-request panic or indefinite block. Replace with
http.NewRequestWithContext and a 30s timeout, handle request creation
errors by incrementing deleteFailCount, and cancel the context
immediately after Do returns.
* feat(shell): parallelize chunk moves in fs.distributeChunks using ErrorWaitGroup
Sequential chunk moves are a bottleneck for large LLM model files with
hundreds or thousands of chunks. Use ErrorWaitGroup with
DefaultMaxParallelization (10) to run download/assign/upload concurrently.
Guard movedRecords appends, chunk.Fid updates, and writer output with a
mutex. Individual chunk failures are non-fatal and logged inline; only
successfully moved chunks are included in the metadata update.
* fix(shell): try all replica URLs on download in fs.distributeChunks
Previously only the first volume server URL was attempted, causing chunk
moves to fail if that replica was unreachable. Now iterates through all
URLs returned by LookupVolumeServerUrl and stops at the first success.
* refactor(shell): apply extract method pattern to fs.distributeChunks
Do() was a single ~615-line function. Break it into focused helpers:
- lookupFileEntry: filer entry lookup
- validateChunks: chunk manifest guard
- collectVolumeTopology: master topology query + ownership mapping
- buildDistributionCounts: chunk→node mapping and owner/copy tallies
- selectActiveNodes: target node selection
- printCurrentDistribution: per-node distribution table
- planDistribution: mode-switch planning (primary/replica/round-robin)
- printRedistributionPlan: before/after plan table
- relevantNodes: active-or-occupied node filter
Do() is now ~100 lines of orchestration; each helper has a single
clear responsibility.
* test(shell): add unit tests for fs.distributeChunks algorithms
Cover all three distribution modes and supporting helpers:
- shortName, relevantNodes
- computeOwnerTarget (even/uneven split, inactive node drain)
- buildDistributionCounts (normal + nil Fid fallback)
- selectActiveNodes (all nodes / limited count)
- planOwnerMoves (imbalanced → balanced, already balanced)
- planDistribution primary (chunks balanced, no-op when even)
- planDistribution round-robin (offset ordering, correct assignment)
- planDistribution replica (owner + copy balancing)
- printRedistributionPlan (output format)
* fix(shell): add 5-minute timeout to chunk downloads in fs.distributeChunks
Download requests had no per-request timeout, unlike delete operations
which already use 30s. Replace readUrl() calls with inline
http.NewRequestWithContext + context.WithTimeout(5m) so a hung volume
server cannot block a goroutine indefinitely during redistribution.
* fix(shell): remove redundant deleteOldChunks in fs.distributeChunks
filer.UpdateEntry already calls deleteChunksIfNotNew internally, which
computes the diff between old and new entry chunks and deletes the ones
no longer referenced. Our explicit deleteOldChunks was racing with this
filer-side cleanup, causing spurious 404 warnings on ~75% of deletes.
Remove deleteOldChunks, movedChunkRecord type, and reduce
executeChunkMoves return type to (int, error) for the moved count.
* fix(shell): handle nil chunk.Fid via chunkVolumeId helper in fs.distributeChunks
chunk.Fid.GetVolumeId() silently returns 0 for legacy chunks stored with
a FileId string instead of a Fid struct, causing them to be skipped in
the replica balancing loop and looked up incorrectly in volumeInfoMap.
Introduce chunkVolumeId() that uses Fid when present and falls back to
parsing the legacy FileId string, matching the logic in
buildDistributionCounts. Apply it in the replica-mode copies loop and
in executeChunkMoves' replication/collection lookup.
* fix(shell): use already-parsed oldFid for volumeInfoMap lookup in fs.distributeChunks
chunkVolumeId(chunk) was being called to look up replication/collection
after oldFid had already been parsed and validated. Use oldFid.VolumeId
directly to avoid redundant parsing and guarantee the correct volume ID
regardless of whether chunk.Fid is nil.
* fix(shell): improve correctness and robustness in fs.distributeChunks
- Buffer download body before upload so dlCtx timeout only covers the
GET request; upload runs with context.Background() via bytes.NewReader
- Replace 'before, after := strings.Cut(...)' + '_ = before' with '_'
as the first return value directly
- Clone copiesCount before replica planner mutates it, keeping the
caller's map immutable
- Add nil-entry guard after filer LookupEntry to prevent panic on
unexpected nil response
* feat(shell): support chunk manifests in fs.distributeChunks
Large files stored as chunk manifests were previously rejected. Resolve
manifests up front via filer.ResolveChunkManifest, redistribute the
underlying data chunks, then re-pack through filer.MaybeManifestize
before UpdateEntry. The filer's MinusChunks resolves manifests on both
sides of the diff, so old manifest and inner data chunks are GC'd
automatically.
* fix(shell): match master's SaveDataAsChunkFunctionType 5-param signature
Master added expectedDataSize uint64; ignore it in shell-side saveAsChunk.
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|