nfsproxy: middleware chain hides CachingHandler and disables directory verifier caching
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 438
- PR merge metrics
- No merged PRs in 30d
Description
Symptom
The NFS proxy's directory verifier cache is silently bypassed.
Paginated READDIR and READDIRPLUS requests cannot reuse the directory listing associated with the cookie verifier returned by the first page. Each subsequent request reads and sorts the directory again.
If the directory changes between pages, the newly calculated verifier differs from the verifier supplied by the client, causing go-nfs to return NFS3ERR_BAD_COOKIE:
if obj.Cookie > 0 && obj.CookieVerif > 0 && verifier != obj.CookieVerif {
return &NFSStatusError{NFSStatusBadCookie, nil}
}
Even when the directory does not change, the repeated ReadDir calls add unnecessary filesystem work for large paginated listings.
Root cause
The proxy creates a helpers.CachingHandler before applying its middleware:
handler = helpers.NewCachingHandler(handler, cacheLimit)
if config.Tracing {
handler = tracing.WrapWithTracing(handler, config)
}
if config.Metrics {
handler = metrics.WrapWithMetrics(handler, config)
}
if config.Logging {
handler = logged.WrapWithLogging(ctx, handler, config)
}
handler = recovery.WrapWithRecovery(ctx, handler)
go-nfs checks the outermost handler for the optional nfs.CachingHandler interface:
if vh, ok := userHandle.(CachingHandler); verifier != 0 && ok {
entries := vh.DataForVerifier(path, verifier)
if entries != nil {
return entries, verifier, nil
}
}
The tracing, metrics, logging, and recovery handlers only implement nfs.Handler. They do not expose or forward:
VerifierFor(path string, contents []fs.FileInfo) uint64
DataForVerifier(path string, verifier uint64) []fs.FileInfo
Because recovery middleware is always installed, the outermost handler never satisfies nfs.CachingHandler, regardless of whether tracing, metrics, or logging are enabled.
As a result, go-nfs never reaches the verifier cache maintained by helpers.CachingHandler:
func (c *CachingHandler) VerifierFor(path string, contents []fs.FileInfo) uint64 {
id := hashPathAndContents(path, contents)
c.activeVerifiers.Add(id, verifier{path, contents})
return id
}
func (c *CachingHandler) DataForVerifier(path string, id uint64) []fs.FileInfo {
if cache, ok := c.activeVerifiers.Get(id); ok {
return cache.contents
}
return nil
}
Why it matters
NFS directory cookies are indexes into the sorted directory listing:
cookie := uint64(i + 2)
For pagination to remain consistent, subsequent requests should use the same listing that produced the original cookie verifier.
Without verifier caching:
- Every page performs another
ReadDirand sort. - Insertions or removals can shift cookie indexes between pages.
- Directory changes can cause
NFS3ERR_BAD_COOKIE. - Large directory listings perform unnecessary repeated filesystem operations.
Proposed fix
Preserve nfs.CachingHandler through the complete middleware chain:
- Implement
VerifierForandDataForVerifieron the tracing handler. - Implement
VerifierForandDataForVerifieron the metrics handler. - Implement
VerifierForandDataForVerifieron the logging handler. - Implement
VerifierForandDataForVerifieron the recovery handler. - Forward both calls to the wrapped handler when it implements
nfs.CachingHandler. - Add compile-time assertions that each middleware implements
nfs.CachingHandler. - Preserve panic recovery around verifier operations in the recovery handler.
- Add a test covering the complete middleware chain and verifying that arguments and return values reach the underlying caching handler.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the tracing, metrics, logging, and recovery middleware implementations and helpers.CachingHandler. Trace how the middleware chain is assembled, then inspect the existing middleware tests before adding coverage for the complete chain. Done means verifier calls and their arguments and return values reach the underlying caching handler, with recovery behavior preserved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100