-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix:not correct used bgsave_info_ #2745
Conversation
WalkthroughIn these changes, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PikaServer
participant DB
User->>PikaServer: Trigger Bgslotsreload
PikaServer->>DB: Check IsBgSaving
alt IsBgSaving = false
PikaServer->>PikaServer: Acquire bgslots_protector_
PikaServer->>DB: Reload Slots
PikaServer->>PikaServer: Release bgslots_protector_
end
User->>PikaServer: Trigger Bgslotscleanup
PikaServer->>DB: Check IsBgSaving
alt IsBgSaving = false
PikaServer->>PikaServer: Acquire bgslots_protector_
PikaServer->>DB: Cleanup Slots
PikaServer->>PikaServer: Release bgslots_protector_
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- include/pika_server.h (3 hunks)
- src/pika_server.cc (2 hunks)
Additional comments not posted (10)
include/pika_server.h (8)
313-313
: The renaming ofbgsave_protector_
tobgslots_protector_
aligns better with its usage context which is primarily slot management.
339-339
: Correct usage ofbgslots_protector_
inbgslots_reload()
ensures thread safety.
343-343
: Proper synchronization inGetSlotsreloading()
using the newbgslots_protector_
.
347-347
: Use ofbgslots_protector_
inSetSlotsreloading()
is consistent and ensures thread safety.
351-351
: Consistent use ofbgslots_protector_
inSetSlotsreloadingCursor()
for thread safety.
355-355
: Correct synchronization inGetSlotsreloadingCursor()
usingbgslots_protector_
.
360-360
: Proper use ofbgslots_protector_
inSetSlotsreloadingEndTime()
to ensure thread safety.
401-427
: All methods related toBGSlotsCleanup
correctly usebgslots_protector_
for thread safety. This includesbgslots_cleanup()
,GetSlotscleaningup()
,SetSlotscleaningup()
,SetSlotscleaningupCursor()
,SetCleanupSlots()
,GetCleanupSlots()
, andStopBgslotscleanup()
.src/pika_server.cc (2)
1436-1438
: The locking mechanism has been updated to usebgslots_protector_
instead ofbgsave_protector_
. This change aligns with the PR's objective to correctly reflect the mutex's usage in slot management.
1501-1502
: The update in the locking mechanism within theBgslotscleanup
method to usebgslots_protector_
is consistent with the changes made in theBgslotsreload
method. This ensures that the mutex name accurately represents its purpose.
src/pika_server.cc
Outdated
@@ -1433,9 +1433,9 @@ bool PikaServer::SlotsMigrateAsyncCancel() { | |||
|
|||
void PikaServer::Bgslotsreload(const std::shared_ptr<DB>& db) { | |||
// Only one thread can go through | |||
std::lock_guard ml(bgslots_protector_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个放{}里面
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个放{}里面
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pika_server.cc (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/pika_server.cc
* fix:not correct used bgsave_info_ * fix:not correct used bgsave_info_ --------- Co-authored-by: chejinge <[email protected]>
* fix:not correct used bgsave_info_ * fix:not correct used bgsave_info_ --------- Co-authored-by: chejinge <[email protected]>
这里是为了bgsave和slots cleanup相互制衡,但是使用了 pika server中的bgsave_info_,但是pika_server中的bgsave_info_没有赋值,应该使用 DB中的,考虑到codis slot迁移只能支持一个DB这里暂时没有考虑多DB 场景
Summary by CodeRabbit
Refactor
Bug Fixes