You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "chenBright (via GitHub)" <gi...@apache.org> on 2025/03/14 14:04:51 UTC
[PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
chenBright opened a new pull request, #2916:
URL: https://github.com/apache/brpc/pull/2916
### What problem does this PR solve?
Issue Number: #2907
Problem Summary:
#2907 合入后，CI经常超时，都是发生在`EventDispatcherTest.dispatch_tasks`和`KeyTest.creating_key_in_parallel`。
原因：`signal`早于`wait`，`signal`没改`_pending_signal`，导致后续的`wait`不会立即返回。
### What is changed and the side effects?
Changed:
无论有无waiter，`signal`都要改`_pending_signal`。
Side effects:
- Performance effects(性能影响):
- Breaking backward compatibility(向后兼容性):
---
### Check List:
- Please make sure your changes are compilable(请确保你的更改可以通过编译).
- When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
- Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).(请遵循贡献者准则).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2728587930
> 我参照2907的修改进行了线上小流量验证，发现tp99上涨了约2-3ms，原先tp99大概是450ms，是否也是这个原因
这个不修改会产生bug，这个应该是必须要修改的，我这边遇到问题了。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "JimChengLin (via GitHub)" <gi...@apache.org>.
JimChengLin commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2729206996
> yanglimingcn
有巨大的差别，syscall 的成本是 us 级别的，atomic check 是 ns 级别的。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "JimChengLin (via GitHub)" <gi...@apache.org>.
JimChengLin commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2731600002
> > 我参照2907的修改进行了线上小流量验证，发现tp99上涨了约2-3ms，原先tp99大概是450ms，是否也是这个原因
>
> 按照这个bug_fix的修改进行了验证，tp99确实恢复了。
👍，我也去测试一下
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "zhoukangsheng (via GitHub)" <gi...@apache.org>.
zhoukangsheng commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2728578798
我参照2907的修改进行了线上小流量验证，发现tp99上涨了约2-3ms，原先tp99大概是450ms，是否也是这个原因
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #2916:
URL: https://github.com/apache/brpc/pull/2916
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "JimChengLin (via GitHub)" <gi...@apache.org>.
JimChengLin commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2728608117
确实会有问题，我之前线上没注意到这个问题。
如果你的任务来自于 worker，parking lot 的 waiter 有可能唤醒没那么频繁，但不会信号丢失。因为 signal 的 worker 本身最终会把 task 消费掉。这个 bug/feature 会进一步减少 futex wake，降低 CPU 占用。
如果任务来自于 non-worker，除了唤醒不那么频繁，叠加这种极端情况，即所有的 worker 都在准备 futex wait，同时有一个 non-worker 投递了一个 task，这样就会产生信号丢失，task 就永远不会被消费了。这种场景发生概率应该很低，大家可以 check 一下是不是遇到这种了。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2728636873
> 确实会有问题，我之前线上没注意到这个问题。
>
> 如果你的任务来自于 worker，parking lot 的 waiter 有可能唤醒没那么频繁，但不会信号丢失。因为 signal 的 worker 本身最终会把 task 消费掉。这个 bug/feature 会进一步减少 futex wake，降低 CPU 占用。
>
> 如果任务来自于 non-worker，除了唤醒不那么频繁，叠加这种极端情况，即所有的 worker 都在准备 futex wait，同时有一个 non-worker 投递了一个 task，这样就会产生信号丢失，task 就永远不会被消费了。这种场景发生概率应该很低，大家可以 check 一下是不是遇到这种了。
现在signal里面的由最开始的futex_wake_private变为原子变量_waiter_num的检查，我不太清楚收益有多大，我这边没测试过。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix ParkingLot::signal does not modify _pending_signal when there is no waiter (brpc)
Posted by "zhoukangsheng (via GitHub)" <gi...@apache.org>.
zhoukangsheng commented on PR #2916:
URL: https://github.com/apache/brpc/pull/2916#issuecomment-2731404875
> 我参照2907的修改进行了线上小流量验证，发现tp99上涨了约2-3ms，原先tp99大概是450ms，是否也是这个原因
按照这个bug_fix的修改进行了验证，tp99确实恢复了。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org