fix(storage): skip docdb watcher reconfiguration on empty file read - #1769
Merged
Conversation
Signed-off-by: Shubham Bhardwaj <shubbhar@redhat.com>
Member
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkhelil The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Member
|
/lgtm |
Member
|
/cherry-pick release-v0.28.x |
|
✅ Cherry-pick to A new pull request has been created to cherry-pick this change to PR: #1770 Please review and merge the cherry-pick PR. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Fix flaky
TestWatchBackendthat fails intermittently in the release pipeline but passes locally.Problem
os.WriteFileis not atomic — it truncates the file (O_TRUNC) writing new content. When the fsnotify watcher goroutine reads the file during this truncation window, it gets an empty string. This causes:MONGO_SERVER_URLenv var set to""docstore.OpenCollectionfails: "MONGO_SERVER_URL environmentvariable is not set"
nilto the unbufferedbackendChanand blocksThe race is timing-dependent: on a fast local machine the watcher processes events before the next file write starts; on a busy CI node the truncation window is wide enough to hit.
Fix
Skip reconfiguration when the file read returns an empty value. An empty
MONGO_SERVER_URLis never a valid configuration — reconfiguring with it always fails. In production, Kubernetes secret mounts use atomic symlink swaps (..data), so transient empty reads don't occur; this guard makes the watcher robust to non-atomic writes without changing any valid production behavior.Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes
/kind bug