Skip to content

meta: commit new chunks in write order#7016

Open
davies wants to merge 1 commit into
mainfrom
commit_chunks
Open

meta: commit new chunks in write order#7016
davies wants to merge 1 commit into
mainfrom
commit_chunks

Conversation

@davies
Copy link
Copy Markdown
Contributor

@davies davies commented May 12, 2026

close #5038

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to prevent observed “zero-filled regions” during concurrent reads of a file that is actively being appended to, by enforcing a commit ordering constraint across newly growing chunks in pkg/vfs (close #5038).

Changes:

  • Track whether a slice contributes to file growth (growing) and whether it has been committed (committed).
  • Introduce cross-chunk commit dependencies so a new chunk’s first slice waits for the prior growing slice to commit before writing metadata.
  • Add a per-file condition variable (commitcond) to coordinate commit ordering across chunk commit goroutines.
Comments suppressed due to low confidence (1)

pkg/vfs/writer.go:307

  • Finding the dependency for a new chunk scans the entire f.chunks map (and then scans lastChunk.slices) while holding the file lock. With many buffered chunks/slices this can become O(n²) during sequential appends. Consider tracking the last in-progress/growing slice (or last uncommitted chunk index) on fileWriter and updating it when s.growing is set/committed, so setting s.dep is O(1). Also, the inner loop variable c shadows the outer c (chunkWriter), which makes this block harder to follow; renaming the inner variable would improve readability.
			if uint64(indx)*meta.ChunkSize >= f.length {
				// first slice of a new chunk, try to find the last slice of the last chunk as dependency
				var lastChunk *chunkWriter
				for i, c := range f.chunks {
					if i < indx && (lastChunk == nil || i > lastChunk.indx) {
						lastChunk = c
					}
				}
				if lastChunk != nil {
					var lastSlice *sliceWriter
					for _, s := range lastChunk.slices {
						if s.growing {
							lastSlice = s
						}
					}
					s.dep = lastSlice
				}
			}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/vfs/writer.go
Comment on lines +200 to +202
for s.dep != nil && !s.dep.committed {
f.commitcond.WaitWithTimeout(time.Millisecond * 100)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data corruption: zeroes are sometimes written instead of the actual data

2 participants