Repository navigation
[ISSUE #9854] Fix DLedgerCommitLog deleteExpiredFile override bypassed by CleanCommitLogService - #11497
Open
dd-xiaozhi wants to merge 1 commit into
Open
[ISSUE #9854] Fix DLedgerCommitLog deleteExpiredFile override bypassed by CleanCommitLogService#11497dd-xiaozhi wants to merge 1 commit into
dd-xiaozhi wants to merge 1 commit into
Conversation
…ypassed by CleanCommitLogService CleanCommitLogService calls the deleteExpiredFile overload that takes deleteFileBatchMax, but DLedgerCommitLog only overrode the 4-argument overload, so its DLedger-specific cleanup logic was never executed. Override the 5-argument overload instead; the 4-argument overload in CommitLog delegates to it, so both entry points now reach the override.
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.
Which Issue(s) This PR Fixes
Brief Description
CleanCommitLogService#deleteExpiredFilescalls theCommitLog#deleteExpiredFileoverload that takesdeleteFileBatchMax:But
DLedgerCommitLogonly overrides the 4-argument overload. As a result, the 5-argument call goes straight toCommitLog→mappedFileQueue.deleteExpiredFileByTime(...), and the DLedger-specific logic in the override never runs in a broker:mappedFileQueueis empty, so the call returns0instead of the intendedInteger.MAX_VALUE. Whenever disk usage exceeds the threshold, the broker keeps loggingdisk space will be full soon, but delete file failed., which is the warning shown in [Bug] dledger模式下，历史消息文件无法删除 #9854.refreshConfig()is never invoked, sofileReservedTime/deleteWhen/cleanFileForciblyEnablechanged at runtime (e.g. viaupdateBrokerConfig) are not propagated to the DLedger store.disableDeleteDledger()is not applied and the last old commitlog file is never removed.This PR changes the override to the 5-argument overload. The 4-argument overload in
CommitLogalready delegates to the 5-argument one, so both entry points now go through the DLedger override.Note: the main symptom of #9854 on JDK 9+ (expired DLedger files not being deleted) was addressed by #9851. This PR fixes the remaining part: the bypassed override behind the misleading warning and the missing config refresh.
How Did You Test This Change?
MixCommitlogTest#testDeleteExpiredFilesWithBatchMax, which calls the same overload asCleanCommitLogService. Without the fix it fails withexpected:<2147483647> but was:<0>; with the fix it passes. It also verifies that a runtime change ofcleanFileForciblyEnableis refreshed into the DLedger config.MixCommitlogTest#testDeleteExpiredFiles(mixed deployment, 4-argument entry) still passes, now going through the override via delegation.mvn -pl store test -Dtest='MixCommitlogTest,DLedgerDeprecationTest,DLedgerMultiPathTest,DLedgerCommitlogTest'passes, and checkstyle reports no violations.