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

check 失败: CHECK(NULL == _write_head.load(butil::memory_order_relaxed)); #643

Closed
TousakaRin opened this issue Feb 2, 2019 · 4 comments

Comments

@TousakaRin
Copy link
Contributor

TousakaRin commented Feb 2, 2019

Describe the bug (描述bug)
WaitAndReset里的 CHECK(NULL == _write_head.load(butil::memory_order_relaxed) check失败,具体位置在这里

线上出现问题时的情况:
连接方式 single, 协议 baidu_std, naming_service: file, 熔断开启( ChannelOptions.enable_circuit_breaker = true)
灾难演练时使用iptable断开下游某个节点的网络之后有时会出现,线上环境下游节点发生网络异常时也出现过。几乎所有上游节点同时FATAL,看fatal日志都是在 WaitAndReset 里check失败。check 失败之前可以看到naming service thread 删除节点的日志,以及熔断该节点的日志(见下图,线上环境日志)。

分析原因是因为我们使用了 consul 来做服务发现,当下游节点网络异常时,consul 更新了服务列表文件(naming service thread 对该节点进行了移除),同时 brpc 也发生了熔断。推测是在 WaitAndReset(2) 时,SocketMap里的引用已经被移除,但keepwrite thread还未结束。所以跳出循环时,引用计数虽然是2,但是额外的引用计数来自KeepWrite而不是SocketMap,所以 _write_head != nullptr 的check 会失败。

To Reproduce (复现方法)

  1. 在写Socket时直接调用KeepWrite,并且KeepWrite 进入之后先sleep 2 秒 (让KeepWrite thread 晚一点再结束)
  2. 客户端使用file 名字服务,或者直接使用single server,并且在发送请求之后删除下游节点或者析构channel (触发SocketMap::Remove)
  3. 开启熔断,并让CircuitBreaker::OnCallEnd 每次都返回 false(触发health check)

可以看到每次发送请求时都会在WaitAndReset check 失败。

Expected behavior (期望行为)
health check在调用WaitAndReset之前,先主动等待一些已知可能存在的线程结束(比如读写线程)或者在
SocketMapRemove的时候,如果发现Socket正在执行health check,就先不remove。而是让health check被唤醒时检查是否需要直接remove 并放弃health check。

Versions (各种版本)
OS:
Compiler:
brpc:
protobuf:

Additional context/screenshots (更多上下文/截图)

image

@cdjingit
Copy link
Contributor

cdjingit commented Feb 2, 2019

@jamesge 帮忙看下这个问题。
目前想到的解决方案有两个:

  1. socket::setfailed()里清空所有pending的WriteRequest
  2. SocketMap::Remove()前确保相关socket的healthcheck()已经停止。

@cdjingit
Copy link
Contributor

ping @jamesge

@jamesge
Copy link
Contributor

jamesge commented Feb 18, 2019

这个check可能是不必要的,除了这个fatal之外并没有其他问题吧

@TousakaRin
Copy link
Contributor Author

没有发现其他问题,那就去掉这个check吧

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

No branches or pull requests

3 participants