fix: abnormal log output when exec slaveof no one #2801
Conversation
WalkthroughA new method Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PikaServer
participant PikaReplicaManager
Client->>PikaServer: RemoveMaster(ip, port)
PikaServer->>PikaReplicaManager: DeactivateSyncSlaveDB(ip, port)
PikaReplicaManager->>PikaServer: Deactivation Status
PikaServer->>Client: Acknowledgement
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- include/pika_rm.h (1 hunks)
- src/pika_rm.cc (1 hunks)
- src/pika_server.cc (1 hunks)
Additional comments not posted (3)
include/pika_rm.h (1)
169-169: New Method Added:DeactivateSyncSlaveDB.The method
DeactivateSyncSlaveDBhas been added to thePikaReplicaManagerclass. Ensure it is used correctly in the codebase and that it handles all necessary scenarios for deactivating sync slave databases.src/pika_rm.cc (1)
729-738: LGTM! Verify the function usage in the codebase.The code changes are approved. The method
DeactivateSyncSlaveDBcorrectly iterates throughsync_slave_dbsand deactivates the matching slave database based on IP and port.However, ensure that all calls to
DeactivateSyncSlaveDBare correctly implemented in the codebase.src/pika_server.cc (1)
662-662: Verify the correctness of the method replacement.The method
DeactivateSyncSlaveDBis used instead ofLostConnection. Ensure thatDeactivateSyncSlaveDBcorrectly handles all necessary deactivation processes thatLostConnectionwas responsible for.Verification successful
Verified the correctness of the method replacement.
The method
DeactivateSyncSlaveDBcorrectly handles the deactivation process by acquiring a shared lock and deactivating the relevant sync slave database. This replacement is appropriate and should function as intended.
include/pika_rm.h: Definition ofDeactivateSyncSlaveDB.src/pika_rm.cc: Implementation ofDeactivateSyncSlaveDB.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the method replacement. # Test: Search for the implementation of `DeactivateSyncSlaveDB`. Expect: Correct handling of deactivation processes. ast-grep --lang cpp --pattern $'class PikaReplicaManager { $$$ Status DeactivateSyncSlaveDB(const std::string& ip, int port) { $$$ } $$$ }'Length of output: 499
Script:
#!/bin/bash # Refined search pattern for the implementation of the DeactivateSyncSlaveDB method within the PikaReplicaManager class. ast-grep --lang cpp --pattern $'class PikaReplicaManager { Status DeactivateSyncSlaveDB(const std::string& ip, int port) { $$$ } }'Length of output: 140
Script:
#!/bin/bash # Broaden the search scope to locate the DeactivateSyncSlaveDB method in the codebase. rg 'DeactivateSyncSlaveDB' -A 5Length of output: 1147
Co-authored-by: cheniujh <[email protected]>
Co-authored-by: cheniujh <[email protected]>
Co-authored-by: cheniujh <[email protected]>
该PR修复了 Issue #2800
问题原因：执行slaveof no one时会执行RemoveMaster流程，会调一个LostConnect函数，其流程是：
1 这个函数把SyncMasterDB遍历一遍，移除MasterIp:port所对应的slaveNode（实际上slave端根本没有这样一个slaveNode，只是移除行为是一个map的erase，即使没有命中也不会报错，当然也没有实际的危害）。于是每次做完一个移除行为就会打印一条日志告知Remove了slaveNode
2 遍历一遍SyncSlaveDB，都执行Deactivate行为。
实际上在执行slaveof no one时，只做第二步就行了，所以把第二步的行为抽出了一个函数用来替代这个位置对于LostConnect的调用
This PR fixes Issue #2800.
Cause: When executing
slaveof no one, theRemoveMasterprocess is triggered, which calls aLostConnectfunction. The process of this function is as follows:The function iterates through
SyncMasterDBand removes theslaveNodecorresponding toMasterIp:port. In fact, there is no suchslaveNodeon the slave side. The removal is essentially a map erase operation. Even if there is no match, it does not report an error, and it does not cause any real harm.It iterates through
SyncSlaveDBand performs aDeactivateoperation on each entry.In reality, when executing
slaveof no one, only the second step is necessary. Therefore, a new function was created to extract the second step's behavior to replace the call toLostConnectin this context.