mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-19 05:36:58 +00:00
f41595fb101feb428e3dc0bbaf8cfe5e49be67fc
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
f3dc530919 |
Re-look-up a chunk's locations as soon as they all fail (#10800)
* mount: re-resolve volume locations after a failed chunk read NewChunkGroup passed nil as the ReaderCache's CacheInvalidator, so retryFetchAfterCacheInvalidation was dead code on the FUSE read path. A mount that cached a volume's locations while one server was down kept retrying that server after it died, then returned EIO, even though the master and filer both resolved the live replica. The S3 gateway already passes its filerClient; do the same for the mount. * test: FUSE integration tests for volume server failover One mount appends while a second tails, and a volume server is killed, started or restarted mid-stream against a 001-replicated cluster of three volume servers. Automates the scenario matrix reported for Docker Swarm mounts, including the large-file variant and a no-chaos control. * test: report the filer's own view when append content mismatches A mismatch between what the writer wrote and what the reader sees can come from either side's cache. Read the file back through the filer's HTTP handler as well, and let the mount verbosity be raised from the environment, so a failing run says which layer lost the data. * test: wait for the reader mount to converge before comparing A mount caches metadata for about a second, so reading the file the instant the writer's last close returned can legitimately come back short. Poll the reader until it matches or the timeout expires; content that is wrong rather than merely late never converges and still fails, now with the writer's mount and the filer's own view alongside it. * test: detect a failover cluster child that exited at startup Signal(0) succeeds for a zombie and nothing reaped these children until shutdown, so a process that died on startup looked alive until the readiness timeout expired. Reap each child as it is started and consult the result. * test: read a file the killed volume server actually holds Placement decides which two of three servers back each volume, so killing volume N and reading readfile-N could pass without the victim ever holding a replica of it. Resolve each file's volumes through the filer and the master, and pick one the victim backs, preferring a file the reader has not cached. * ci: stop persisting checkout credentials in the failover workflow The job does not use the token after cloning. Also tag the README's command block as bash and match the timeout the workflow actually uses. * test: discard the ignored errors errcheck flags in the failover harness * test: resolve manifests when mapping a file to its volumes A manifest chunk's own fid names the volume holding the manifest, not the volumes holding the data, so a large enough file would point the failover victim at the wrong server. * test: pin the stale-location recovery path with a primed reader Reading a file for the first time after a server dies proves nothing: the lookup is fresh and returns the survivor. Kill one holder and wait for the master to drop it, read a file on that volume so the reader caches the lone survivor, restart the first server, then kill the survivor. The reader's only cached location is now dead while the data is live elsewhere, which is the case the invalidator exists for: EIO without it, recovery with it. * filer: re-look-up a chunk's locations as soon as they all fail A read that fails against every location it was given is far more likely to be holding a stale list than to be hitting a cluster that is briefly slow, but the retry loops spent the whole backoff ladder, about 13 s, before the caller got a chance to invalidate and look the chunk up again. Give the loops a refresh hook and let the reader cache invalidate on the first fully failed pass, so recovery starts in milliseconds. Clients without an invalidator keep the old behavior. The filer's streaming read path has its own fetch loop and is not covered. * filer: refresh locations on the random-read path too readChunkSliceAt bypasses the chunk cacher in random-access mode and fetches the range directly, which left it without the invalidation the cacher does: a random reader parked on a stale location had no way back at all. Hoist the refresh hook onto the reader cache so both paths share it. * filer: compare chunk locations as a set, not in order Lookups shuffle the locations they return, so comparing positionally reads a reshuffle of the very same replicas as a fresh set and spends an immediate retry on locations that just failed. weed/filer already had an order-independent comparison for this; move it next to the retry loops so both callers share one helper. |
||
 
|
424cd164e9 |
s3: invalidate stale reader cache locations on chunk read failure (#10156)
* s3: invalidate stale reader cache locations on chunk read failure * filer: share the chunk-read self-heal across reader cache and streaming paths The reader cache retry added a third copy of the invalidate-relookup-compare-retry dance already inlined in PrepareStreamContentWithThrottler and duplicated in retryWithCacheInvalidation. Extract retryFetchWithFreshLocations and route all three through it, parameterized by the refetch primitive. * filer: drop redundant completedTimeNew store in reader cache success path startCaching already stamps completedTimeNew unconditionally before the fetchErr branch; the second store inside the success branch is dead. * filer: make NewReaderCache cache invalidator an explicit parameter The variadic ...CacheInvalidator only ever read the first element, so a caller could pass two and silently get one. Take a single explicit argument and have the non-S3 callers pass nil. * filer: inject reader cache chunk fetch as a struct field Replace the process-global readerCacheFetchChunkData test seam with a per-instance fetchChunkDataFn field defaulted in NewReaderCache, matching how lookupFileIdFn is already wired. Tests set the field on the cache instead of swapping a shared global. * filer: log the location count, not full URLs, on self-heal retry --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
886d50a6a5 |
feat(mount): singleflight dedup for concurrent chunk reads (#9100)
* feat(mount): add singleflight deduplication for concurrent chunk reads When multiple FUSE readers request the same uncached chunk concurrently, only one network fetch is performed. Other readers wait and share the downloaded data, reducing redundant volume server traffic under parallel read workloads. * fix(util): make singleflight panic-safe with defer cleanup If the provided function panics, the WaitGroup and map entry are now cleaned up via defer, preventing other waiters from hanging forever. * fix(filer): remove singleflight from reader_cache to fix buffer ownership The singleflight wrapper around chunk fetches returned the same []byte buffer to concurrent callers. Since each SingleChunkCacher owns and frees its data buffer in destroy(), sharing the same slice would cause a use-after-free or double-free with the mem allocator. The downloaders map already deduplicates in-flight downloads for the same fileId, so the singleflight was redundant at this layer. The SingleFlightGroup utility is retained for use elsewhere. |
||
|
9012069bd7 |
chore: execute goimports to format the code (#7983)
* chore: execute goimports to format the code Signed-off-by: promalert <promalert@outlook.com> * goimports -w . --------- Signed-off-by: promalert <promalert@outlook.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
5c1de633cb |
mount: improve read throughput with parallel chunk fetching (#7627)
* filer: remove lock contention during chunk download This addresses issue #7504 where a single weed mount FUSE instance does not fully utilize node network bandwidth when reading large files. The SingleChunkCacher was holding a mutex during the entire HTTP download, causing readers to block until the download completed. This serialized chunk reads even when multiple goroutines were downloading in parallel. Changes: - Add sync.Cond to SingleChunkCacher for efficient waiting - Move HTTP download outside the critical section in startCaching() - Use condition variable in readChunkAt() to wait for download completion - Add isComplete flag to track download state Now multiple chunk downloads can proceed truly in parallel, and readers wait efficiently using the condition variable instead of blocking on a mutex held during I/O operations. Ref: #7504 * filer: parallel chunk fetching within doReadAt This addresses issue #7504 by enabling parallel chunk downloads within a single read operation. Previously, doReadAt() processed chunks sequentially in a loop, meaning each chunk had to be fully downloaded before the next one started. This left significant network bandwidth unused when chunks resided on different volume servers. Changes: - Collect all chunk read tasks upfront - Use errgroup to fetch multiple chunks in parallel - Each chunk reads directly into its correct buffer position - Limit concurrency to prefetchCount (min 4) to avoid overwhelming the system - Handle gaps and zero-filling before parallel fetch - Trigger prefetch after parallel reads complete For a read spanning N chunks on different volume servers, this can now utilize up to N times the bandwidth of a single connection. Ref: #7504 * http: direct buffer read to reduce memory copies This addresses issue #7504 by reducing memory copy overhead during chunk downloads. Previously, RetriedFetchChunkData used ReadUrlAsStream which: 1. Allocated a 64KB intermediate buffer 2. Read data in 64KB chunks 3. Called a callback to copy each chunk to the destination For a 16MB chunk, this meant 256 copy operations plus the callback overhead. Profiling showed significant time spent in memmove. Changes: - Add readUrlDirectToBuffer() that reads directly into the destination - Add retriedFetchChunkDataDirect() for unencrypted, non-gzipped chunks - Automatically use direct read path when possible (cipher=nil, gzip=false) - Use http.NewRequestWithContext for proper cancellation For unencrypted chunks (the common case), this eliminates the intermediate buffer entirely, reading HTTP response bytes directly into the final destination buffer. Ref: #7504 * address review comments - Use channel (done) instead of sync.Cond for download completion signaling This integrates better with context cancellation patterns - Remove redundant groupErr check in reader_at.go (errors are already captured in task.err) - Remove buggy URL encoding logic from retriedFetchChunkDataDirect (The existing url.PathEscape on full URL is a pre-existing bug that should be fixed separately) * address review comments (round 2) - Return io.ErrUnexpectedEOF when HTTP response is truncated This prevents silent data corruption from incomplete reads - Simplify errgroup error handling by using g.Wait() error directly Remove redundant task.err field and manual error aggregation loop - Define minReadConcurrency constant instead of magic number 4 Improves code readability and maintainability Note: Context propagation to startCaching() is intentionally NOT changed. The downloaded chunk is a shared resource that may be used by multiple readers. Using context.Background() ensures the download completes even if one reader cancels, preventing data loss for other waiting readers. * http: inject request ID for observability in direct read path Add request_id.InjectToRequest() call to readUrlDirectToBuffer() for consistency with ReadUrlAsStream path. This ensures full-chunk reads carry the same tracing/correlation headers for server logs and metrics. * filer: consistent timestamp handling in sequential read path Use max(ts, task.chunk.ModifiedTsNs) in sequential path to match parallel path behavior. Also update ts before error check so that on failure, the returned timestamp reflects the max of all chunks processed so far. * filer: document why context.Background() is used in startCaching Add comment explaining the intentional design decision: the downloaded chunk is a shared resource that may be used by multiple concurrent readers. Using context.Background() ensures the download completes even if one reader cancels, preventing errors for other waiting readers. * filer: propagate context for reader cancellation Address review comment: pass context through ReadChunkAt call chain so that a reader can cancel its wait for a download. The key distinction is: - Download uses context.Background() - shared resource, always completes - Reader wait uses request context - can be cancelled individually If a reader cancels, it stops waiting and returns ctx.Err(), but the download continues to completion for other readers waiting on the same chunk. This properly handles the shared resource semantics while still allowing individual reader cancellation. * filer: use defer for close(done) to guarantee signal on panic Move close(s.done) to a defer statement at the start of startCaching() to ensure the completion signal is always sent, even if an unexpected panic occurs. This prevents readers from blocking indefinitely. * filer: remove unnecessary code - Remove close(s.cacheStartedCh) in destroy() - the channel is only used for one-time synchronization, closing it provides no benefit - Remove task := task loop variable capture - Go 1.22+ fixed loop variable semantics, this capture is no longer necessary (go.mod specifies Go 1.24.0) * filer: restore fallback to chunkCache when cacher returns no data Fix critical issue where ReadChunkAt would return 0,nil immediately if SingleChunkCacher couldn't provide data for the requested offset, without trying the chunkCache fallback. Now if cacher.readChunkAt returns n=0 and err=nil, we fall through to try chunkCache. * filer: add comprehensive tests for ReaderCache Tests cover: - Context cancellation while waiting for download - Fallback to chunkCache when cacher returns n=0, err=nil - Multiple concurrent readers waiting for same chunk - Partial reads at different offsets - Downloader cleanup when exceeding cache limit - Done channel signaling (no hangs on completion) * filer: prioritize done channel over context cancellation If data is already available (done channel closed), return it even if the reader's context is also cancelled. This avoids unnecessary errors when the download has already completed. * filer: add lookup error test and document test limitations Add TestSingleChunkCacherLookupError to test error handling when lookup fails. Document that full HTTP integration tests for SingleChunkCacher require global HTTP client initialization which is complex in unit tests. The download path is tested via FUSE integration tests. * filer: add tests that exercise SingleChunkCacher concurrency logic Add tests that use blocking lookupFileIdFn to exercise the actual SingleChunkCacher wait/cancellation logic: - TestSingleChunkCacherContextCancellationDuringLookup: tests reader cancellation while lookup is blocked - TestSingleChunkCacherMultipleReadersWaitForDownload: tests multiple readers waiting on the same download - TestSingleChunkCacherOneReaderCancelsOthersContinue: tests that when one reader cancels, other readers continue waiting These tests properly exercise the done channel wait/cancel logic without requiring HTTP calls - the blocking lookup simulates a slow download. |