{184305174} Fix delfiles to clear ufid-hash before deleting files#5956
Open
chands10 wants to merge 1 commit into
Open
{184305174} Fix delfiles to clear ufid-hash before deleting files#5956chands10 wants to merge 1 commit into
chands10 wants to merge 1 commit into
Conversation
Contributor
Author
|
test AI |
This fixes a regression introduced in commit 5802570 where ufid clearing was moved from bdb_del_file() to schema changes only (bdb_close_only_sc). This caused the standalone 'delfiles' command to leave open file descriptors in ufid-hash, especially on replicants that opened handles during recovery. Root Cause: - When replicants apply log records during recovery, they open DB handles via ufid-hash and mark them with DB_AM_RECOVER flag - These handles are kept open for performance - When 'delfiles' command runs WITHOUT a schema change, bdb_del_file() directly calls dbremove() without clearing ufid-hash entries - The OS cannot release disk space because handles remain open - Files show as "deleted" in lsof output but space is not reclaimed Impact: - Production databases accumulate "deleted" files over time - Requires weekly database bounces to reclaim space The Fix: - Restore ufid clearing to bdb_del_file() before calling dbremove() - Respects gbl_clear_ufid_on_db_close tunable (default=1) - Opens the file briefly, calls clear_ufid_hash(), then closes it - This clears any recovery handles (DB_AM_RECOVER) from ufid-hash - Non-fatal if clearing fails - logs warning and continues Test: - New test delfiles_ufid_leak.test validates delfiles WITHOUT schema change - Unlike berkdb_file_leaks_rep.test which does TRUNCATE before delfiles, this test reproduces the production scenario - Verifies no "deleted" files remain open after delfiles Signed-off-by: Salil Chandra <schandra107@bloomberg.net>
roborivers
suggested changes
May 21, 2026
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**
Contributor
|
I don't think this is correct. delfiles runs on master only and has no effects on replicants. Can you please turn on |
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
3 participants
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.
This fixes a regression introduced in commit 5802570 where ufid clearing was moved from bdb_del_file() to schema changes only (bdb_close_only_sc). This caused the standalone 'delfiles' command to leave open file descriptors in ufid-hash, especially on replicants that opened handles during recovery.
Root Cause:
Impact:
The Fix:
Test:
To help us review your pull request, please consider providing an overview of the following: