diff --git a/weed/mount/dirty_pages_chunked.go b/weed/mount/dirty_pages_chunked.go index 0440d2057..ce0d34b71 100644 --- a/weed/mount/dirty_pages_chunked.go +++ b/weed/mount/dirty_pages_chunked.go @@ -88,6 +88,10 @@ func (pages *ChunkedDirtyPages) Destroy() { pages.uploadPipeline.Shutdown() } +func (pages *ChunkedDirtyPages) EvictOneWritableChunk() bool { + return pages.uploadPipeline.EvictOneWritableChunk() +} + func (pages *ChunkedDirtyPages) LockForRead(startOffset, stopOffset int64) { pages.uploadPipeline.LockForRead(startOffset, stopOffset) } diff --git a/weed/mount/page_writer.go b/weed/mount/page_writer.go index a95e19160..fd262c557 100644 --- a/weed/mount/page_writer.go +++ b/weed/mount/page_writer.go @@ -81,6 +81,10 @@ func (pw *PageWriter) Destroy() { pw.randomWriter.Destroy() } +func (pw *PageWriter) EvictOneWritableChunk() bool { + return pw.randomWriter.EvictOneWritableChunk() +} + func max(x, y int64) int64 { if x > y { return x diff --git a/weed/mount/page_writer/dirty_pages.go b/weed/mount/page_writer/dirty_pages.go index 472815dd5..8c32a0be1 100644 --- a/weed/mount/page_writer/dirty_pages.go +++ b/weed/mount/page_writer/dirty_pages.go @@ -7,6 +7,7 @@ type DirtyPages interface { Destroy() LockForRead(startOffset, stopOffset int64) UnlockForRead(startOffset, stopOffset int64) + EvictOneWritableChunk() bool } func max(x, y int64) int64 { diff --git a/weed/mount/page_writer/upload_pipeline.go b/weed/mount/page_writer/upload_pipeline.go index a6dcb38f3..ea3a15f39 100644 --- a/weed/mount/page_writer/upload_pipeline.go +++ b/weed/mount/page_writer/upload_pipeline.go @@ -256,6 +256,34 @@ func (up *UploadPipeline) moveToSealed(memChunk PageChunk, logicChunkIndex Logic up.chunksLock.Lock() } +// EvictOneWritableChunk force-seals the fullest writable chunk in this +// pipeline, submitting it for async upload. Called by the accountant's +// evictor when Reserve would block. Returns true if a chunk was sealed. +// The fullest-chunk heuristic matches the over-limit path in SaveDataAt: +// sealing the chunk closest to full maximizes the upload's usefulness +// and avoids thrashing on repeatedly re-creating the same half-empty +// chunk. Callers must not hold up.chunksLock. +func (up *UploadPipeline) EvictOneWritableChunk() bool { + up.chunksLock.Lock() + defer up.chunksLock.Unlock() + if len(up.writableChunks) == 0 { + return false + } + var bestIndex LogicChunkIndex + var bestBytes int64 = -1 + for lci, wc := range up.writableChunks { + if b := wc.WrittenSize(); b > bestBytes { + bestIndex = lci + bestBytes = b + } + } + if bestBytes < 0 { + return false + } + up.moveToSealed(up.writableChunks[bestIndex], bestIndex) + return true +} + func (up *UploadPipeline) Shutdown() { up.swapFile.FreeResource() diff --git a/weed/mount/page_writer/write_buffer_accountant.go b/weed/mount/page_writer/write_buffer_accountant.go index 5f594c06d..6f4621d11 100644 --- a/weed/mount/page_writer/write_buffer_accountant.go +++ b/weed/mount/page_writer/write_buffer_accountant.go @@ -14,10 +14,12 @@ import ( // // A nil receiver is treated as "unlimited" for backward compatibility. type WriteBufferAccountant struct { - mu sync.Mutex - cond *sync.Cond - cap int64 // 0 means unlimited - used int64 + mu sync.Mutex + cond *sync.Cond + cap int64 // 0 means unlimited + used int64 + evictor func(needBytes int64) bool + evicting bool } func NewWriteBufferAccountant(capBytes int64) *WriteBufferAccountant { @@ -26,6 +28,26 @@ func NewWriteBufferAccountant(capBytes int64) *WriteBufferAccountant { return a } +// SetEvictor registers a callback that Reserve invokes when the cap would +// otherwise block. The evictor is expected to force-seal at least one +// writable chunk in some UploadPipeline, which turns a pinned-forever +// writable chunk into a sealed chunk that the async uploader drains and +// Releases. Without this hook, workloads that hold many files open for +// write with less-than-chunkSize data in each (e.g. fio 4k randwrite with +// nrfiles * chunkSize > cap) deadlock permanently, because writable chunks +// only seal on close or when they fill. +// +// The evictor must not call Reserve on the same accountant or re-enter +// Reserve transitively — it would deadlock on accountant.mu. +func (a *WriteBufferAccountant) SetEvictor(fn func(needBytes int64) bool) { + if a == nil { + return + } + a.mu.Lock() + a.evictor = fn + a.mu.Unlock() +} + // Reserve blocks until n bytes can be accounted for under the cap. // It must not be called while holding any UploadPipeline lock, or the // uploader goroutines that eventually call Release will deadlock. @@ -36,11 +58,41 @@ func (a *WriteBufferAccountant) Reserve(n int64) { a.mu.Lock() defer a.mu.Unlock() for a.used+n > a.cap && a.used > 0 { + // Before blocking, try to force-seal a writable chunk somewhere so + // its async upload path will eventually Release a slot. Single-flight + // on `evicting` so a stampede of blocked reservers doesn't iterate + // the fhMap concurrently. + if a.evictor != nil && !a.evicting { + a.runEvictorLocked(n) + // A concurrent Release may have brought used back under the + // cap during the evict window — re-check before waiting so we + // do not block on a broadcast that has already fired. + if a.used+n <= a.cap || a.used == 0 { + break + } + } a.cond.Wait() } a.used += n } +// runEvictorLocked is called with a.mu held and !a.evicting. It drops the +// lock around the evictor invocation so uploader goroutines (which call +// Release under the same lock) can make progress, and uses defer to +// guarantee the `evicting` flag and the lock are restored even if the +// evictor panics. +func (a *WriteBufferAccountant) runEvictorLocked(n int64) { + evictor := a.evictor + a.evicting = true + a.mu.Unlock() + defer func() { + a.mu.Lock() + a.evicting = false + a.cond.Broadcast() + }() + evictor(n) +} + func (a *WriteBufferAccountant) Release(n int64) { if a == nil || a.cap <= 0 { return diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index 87460d594..e6564c93e 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -251,6 +251,7 @@ func NewSeaweedFileSystem(option *Option) *WFS { } if option.WriteBufferSizeMB > 0 { wfs.writeBufferAccountant = page_writer.NewWriteBufferAccountant(option.WriteBufferSizeMB * 1024 * 1024) + wfs.writeBufferAccountant.SetEvictor(wfs.evictOneWritableChunk) } wfs.metaCache = meta_cache.NewMetaCache(path.Join(option.getUniqueCacheDirForRead(), "meta"), option.UidGidMapper, diff --git a/weed/mount/weedfs_write_buffer_evict.go b/weed/mount/weedfs_write_buffer_evict.go new file mode 100644 index 000000000..ba09e1272 --- /dev/null +++ b/weed/mount/weedfs_write_buffer_evict.go @@ -0,0 +1,47 @@ +package mount + +// evictOneWritableChunk is the callback the WriteBufferAccountant invokes +// when Reserve would otherwise block. It walks open file handles and asks +// the first one that has a writable chunk to force-seal it. Sealing moves +// the chunk into the async upload path, whose completion Releases a slot +// on the accountant and unblocks the waiter. +// +// Without this hook, workloads that open many files for write but never +// fill or close any single chunk (e.g. fio 4k randwrite across nrfiles +// where nrfiles * chunkSizeLimit > writeBufferSizeMB) deadlock +// permanently: every writable chunk reserves a full chunkSize slot on +// creation, and the pipeline only releases the slot when the chunk +// finishes uploading, which only happens after the chunk fills or the +// file closes. +// +// Performance note: this does a linear scan of fhMap.inode2fh looking +// for the first handle with a writable chunk. The scan is bounded by +// the number of open-for-write file handles at the moment Reserve +// blocks, not by nrfiles in the workload: handles that have no dirty +// pages fall through EvictOneWritableChunk in O(1) under their own +// chunksLock, so the hot-path cost is just the map walk. The scan is +// already serialized by the caller's single-flight `evicting` flag +// (one eviction across the entire accountant at a time), and happens +// only when Reserve is about to block — i.e., when the cap is actually +// exhausted and we would otherwise park a writer — so the O(N) walk is +// paid at most once per chunkSize drained, not per write. Replacing +// the map with an LRU / dirty-set would trade this for per-AddPage +// bookkeeping overhead on a hotter path; we keep the scan until a +// profile shows it mattering at the scale users actually hit. +// +// Called from WriteBufferAccountant.Reserve with accountant.mu dropped. +// Must not call anything that takes accountant.mu back; see the caller's +// single-flight `evicting` flag for safety against concurrent invocation. +func (wfs *WFS) evictOneWritableChunk(needBytes int64) bool { + wfs.fhMap.RLock() + defer wfs.fhMap.RUnlock() + for _, fh := range wfs.fhMap.inode2fh { + if fh == nil || fh.dirtyPages == nil { + continue + } + if fh.dirtyPages.EvictOneWritableChunk() { + return true + } + } + return false +}