Unprotect rdb channel when bgsave child fails in dual channel replication#1297
Merged
enjoy-binbin merged 2 commits intoNov 15, 2024
Merged
Conversation
…tion If bgsaveerr is error, there is no need to protect the rdb channel. The impact of this may be that when bgsave fails, we will protect the rdb channel for 60s. It may occupy the reference of the repl buf block, making it impossible to recycle it until we free the client due to COB or free the client after 60s. Signed-off-by: Binbin <binloveplay1314@qq.com>
Member
Author
|
The issue was found in #1288 (comment) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1297 +/- ##
============================================
+ Coverage 70.69% 70.71% +0.01%
============================================
Files 114 114
Lines 63161 63162 +1
============================================
+ Hits 44650 44663 +13
+ Misses 18511 18499 -12
|
naglera
approved these changes
Nov 13, 2024
naglera
left a comment
Contributor
There was a problem hiding this comment.
Thank you, @enjoy-binbin, for spotting that. We kept the RDB channel open as long as the replica hadn't established a main connection, even if the snapshot process failed.
There is no value in keeping the RDB client in this case.
Co-authored-by: Amit Nagler <58042354+naglera@users.noreply.github.com> Signed-off-by: Binbin <binloveplay1314@qq.com>
xbasel
reviewed
Nov 13, 2024
zuiderkwast
approved these changes
Nov 15, 2024
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
4 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.
If bgsaveerr is error, there is no need to protect the rdb channel.
The impact of this may be that when bgsave fails, we will protect
the rdb channel for 60s. It may occupy the reference of the repl
buf block, making it impossible to recycle it until we free the
client due to COB or free the client after 60s.
We kept the RDB channel open as long as the replica hadn't established
a main connection, even if the snapshot process failed. There is no
value in keeping the RDB client in this case.