wwbmmm commented on PR #3626: URL: https://github.com/apache/brpc/pull/3626#issuecomment-6342139123
[replied by brpc-oncall robot] Two clarifications. 1. My earlier claim that these tests cannot compile was wrong, please disregard it. I verified that `test/CMakeLists.txt` adds `-fno-access-control` to the `brpc_test_config` INTERFACE target that all `brpc_*_unittest` targets link against, and the same flag is set in `test/Makefile` and in `COPTS` of `test/BUILD.bazel`. Given that repo-wide test convention, accessing the internals directly here is acceptable. 2. On the `load` + `fetch_add` concern: for one endpoint `_sq_window_size` is only incremented in `HandleCompletion`, and `HandleCompletion` cannot run concurrently for the same endpoint. In event mode the CQ socket is driven through `Socket::OnInputEvent`, which only starts an event-processing bthread when `s->_nevent.fetch_add(1) == 0`, so a second drain cannot overlap; in polling mode each CQ socket id is assigned to exactly one poller via `butil::fmix32(_cq_sid) % pollers.size()`. The only other writer of this counter is `CutFromIOBufList`, which decrements (and restores on post failure). So the load + capacity check + `fetch_add` sequence preserves `_sq_window_size <= _local_window_capacity` and the CAS loop is not needed. Also, `wnd` is a hardcoded `1` (every WR is signalled), so `old >= _local_window_capacity` is the correct rejection test; if `wnd` ever becomes variable, both this check and the `increment` field in the log need to be updated together. No remaining blockers from my side. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
