fix(backup): retry opening NVMe device on ENXIO race - #568
Conversation
After the NVMe-oF controller connects, the kernel may not have instantiated the namespace block device yet, causing open() on the device endpoint to return ENXIO. This intermittently fails backup creation right after StartNvmeTCPInitiator returns. Retry the open up to 10 times with a 500ms fixed delay (5s total) when the error is ENXIO, using retry-go for consistency with the existing retry usage in pkg/spdk/engine.go. Non-ENXIO errors are wrapped in retrygo.Unrecoverable so they fail fast as before. Longhorn 13188 Signed-off-by: Derek Su <derek.su@suse.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens backup snapshot opening against a known NVMe-oF timing race where the kernel has connected the controller but hasn’t yet created the namespace block device node, causing open() to intermittently fail with ENXIO.
Changes:
- Add
retry-go-based retry logic around opening the NVMe device endpoint inBackup.OpenSnapshot. - Retry only on
ENXIOand fail fast on any other open error viaretrygo.Unrecoverable. - Introduce constants for the retry attempt count and delay interval.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Derek Su <derek.su@suse.com>
|
@mergify backport v1.12.x |
🟠 Waiting for conditions to matchDetails
|
c3y1huang
left a comment
c3y1huang
left a comment
There was a problem hiding this comment.
I am a bit confused - is this intended to fix longhorn/longhorn#13188?
I looked through the bundle in the issue description, but couldn't find failed to open NVMe device or ENXIO errors. Could you elaborate on how you arrived at this root cause? Is there evidence that links to this code path, or was it reproduced separately?
Also, if this is the correct fix, should we apply the same handling to the other call site as well?
Yes. While working on the fix, I encountered an ENXIO error that caused backup failures. |
Got it, so this PR is about ENXIO hardening. Should we do the same for the other call site as well? |
Longhorn 13188 Signed-off-by: Derek Su <derek.su@suse.com>
| retry.OnRetry(func(n uint, err error) { | ||
| log.Warnf("NVMe device %v not ready (ENXIO), retrying (%d/%d)", endpoint, n+1, openDeviceMaxRetries) |
There was a problem hiding this comment.
How about dropping the (ENXIO) and letting the errno come from the error itself (.WithError(err))?
|
This pull request is now in conflict. Could you fix it @derekbit? 🙏 |
|
This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Which issue(s) this PR fixes:
Issue longhorn/longhorn#13188
What this PR does / why we need it:
After the NVMe-oF controller connects, the kernel may not have instantiated the namespace block device yet, causing open() on the device endpoint to return ENXIO. This intermittently fails backup creation right after StartNvmeTCPInitiator returns.
Retry the open up to 10 times with a 500ms fixed delay (5s total) when the error is ENXIO, using retry-go for consistency with the existing retry usage in pkg/spdk/engine.go. Non-ENXIO errors are wrapped in retrygo.Unrecoverable so they fail fast as before.
Special notes for your reviewer:
Additional documentation or context