Skip to content
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

bugfix: fix panic when sync reader needed to exit #760

Merged

Conversation

zgg2001
Copy link
Contributor

@zgg2001 zgg2001 commented Jan 17, 2024

之前的sync读部分新增了优雅退出部分,但是存在一些缺陷,具体如下:

复现流程一:

  1. sync模式配置开启RDB同步、关闭AOF同步
  2. 在某个协程RDB同步完成时,会直接关闭reader client,导致协程里读到关闭的连接引发panic。

复现流程二:

  1. sync模式配置开启RDB同步
  2. 发送信号触发优雅退出,会直接关闭reader client,导致协程里读到关闭的连接引发panic。

由于集群模式下是多协程同步,一个协程引发panic会导致其余协程存在数据同步不完全的问题,需要修复一下。

@CLAassistant
Copy link

CLAassistant commented Jan 17, 2024

CLA assistant check
All committers have signed the CLA.

@zgg2001
Copy link
Contributor Author

zgg2001 commented Jan 17, 2024

由于避免一些Redis异常场景导致wait()长时间阻塞,我添加设置了10秒超时时间,麻烦一起评估下相关的更改

@suxb201
Copy link
Member

suxb201 commented Jan 19, 2024

我猜测问题原因是 reader 对象已经被析构了,但是对应的两个协程没有及时析构,访问不存在的对象。
或许这样修复就可以解决问题:

    go func() {
        r.sendReplconfAck()
    }()

使用闭包避免 r 被提前释放。receiveAOF 同理。

@zgg2001 zgg2001 force-pushed the 20240117/v4/bugfix-reader-shutdown branch from f3c0d7d to 4d7b027 Compare January 19, 2024 02:38
@zgg2001 zgg2001 changed the title bugfix: sync reader adaptive gracefully shutdown bugfix: fix panic when sync reader needed to exit Jan 19, 2024
@zgg2001
Copy link
Contributor Author

zgg2001 commented Jan 19, 2024

@suxb201 已调整,申请合入

@zgg2001 zgg2001 requested a review from suxb201 January 19, 2024 02:56
@suxb201 suxb201 merged commit c5ff14c into tair-opensource:v4 Jan 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants