Copilot commented on code in PR #3238:
URL: https://github.com/apache/brpc/pull/3238#discussion_r2898574599
##########
src/brpc/event_dispatcher.cpp:
##########
@@ -41,6 +51,15 @@ static bvar::LatencyRecorder* g_edisp_read_lantency = NULL;
static bvar::LatencyRecorder* g_edisp_write_lantency = NULL;
static pthread_once_t g_edisp_once = PTHREAD_ONCE_INIT;
+bool EventDispatcherUnsched() {
+#if BRPC_WITH_RDMA
+ return FLAGS_event_dispatcher_edisp_unsched ||
+ rdma::FLAGS_rdma_edisp_unsched;
+#else
+ return FLAGS_event_dispatcher_edisp_unsched;
+#endif
+}
Review Comment:
EventDispatcherUnsched() and the new unified flag introduce non-trivial
scheduling behavior changes, but there are no unit tests in the repository
exercising the new flag/legacy-flag interaction or the urgent/background
selection. Please add/adjust tests (e.g. in
test/brpc_event_dispatcher_unittest.cpp) to cover: unified flag toggling,
legacy rdma flag behavior when BRPC_WITH_RDMA, and that Tcp/Rdma ProcessEvent
choose the intended bthread_start_* path.
##########
src/brpc/rdma/rdma_endpoint.h:
##########
@@ -38,7 +38,6 @@ namespace rdma {
DECLARE_bool(rdma_use_polling);
DECLARE_int32(rdma_poller_num);
-DECLARE_bool(rdma_edisp_unsched);
DECLARE_bool(rdma_disable_bthread);
Review Comment:
This change removes the public DECLARE_bool(rdma_edisp_unsched). Since this
header is installed, any downstream code that referenced
brpc::rdma::FLAGS_rdma_edisp_unsched will now fail to compile, even though the
flag is still supported for command-line compatibility. Consider keeping the
DECLARE (marked deprecated) or re-homing it to a different installed header to
preserve source compatibility while deprecating usage.
```suggestion
DECLARE_bool(rdma_disable_bthread);
DECLARE_bool(rdma_edisp_unsched); // Deprecated: kept for source
compatibility
```
##########
src/brpc/event_dispatcher.h:
##########
@@ -188,6 +191,11 @@ template <typename T> friend class IOEvent;
EventDispatcher& GetGlobalEventDispatcher(int fd, bthread_tag_t tag);
+// Unified unsched switch for transport layer.
+// false -> background start (allowing schedule away),
+// true -> urgent start (foreground scheduling before caller continues).
Review Comment:
The mapping described in this comment is reversed relative to the actual
transport logic (TcpTransport/RdmaTransport use urgent when
EventDispatcherUnsched() is false, background when true). Please update the
comment to match the implemented semantics to avoid misleading future changes.
```suggestion
// false -> urgent start (foreground scheduling before caller continues),
// true -> background start (allowing schedule away).
```
##########
docs/en/rdma.md:
##########
@@ -47,6 +47,23 @@ The application can manage memory by itself and send data
with IOBuf::append_use
RDMA is hardware-related. It has some different concepts such as device, port,
GID, LID, MaxSge and so on. These parameters can be read from NICs at
initialization, and brpc will make the default choice (see
src/brpc/rdma/rdma_helper.cpp). Sometimes the default choice is not the
expectation, then it can be changed in the flag way.
+`event_dispatcher_edisp_unsched` is a global flag and affects EventDispatcher
scheduling in both normal mode (TCP) and RDMA mode. For backward compatibility,
`rdma_edisp_unsched` is still kept, but it is deprecated and will be removed in
a future release.
+
+The effective unsched condition is unified as:
+`event_dispatcher_edisp_unsched || rdma_edisp_unsched`
+
+No startup synchronization rewrites user flags. Runtime behavior is determined
directly from user-provided values.
+
+Recommended usage:
+1. New deployment: set only `event_dispatcher_edisp_unsched`.
+2. Existing deployment: keep `rdma_edisp_unsched` temporarily, but migrate to
`event_dispatcher_edisp_unsched`.
+3. Avoid conflicting values in scripts; with unified OR semantics, either flag
being `true` makes EventDispatcher unschedulable.
+
+Examples:
+1. Only `-rdma_edisp_unsched=true`: `rdma_edisp_unsched=true`,
`event_dispatcher_edisp_unsched=false`; both TCP and RDMA are unschedulable.
+2. Only `-event_dispatcher_edisp_unsched=true`: both flags are `true`; both
TCP and RDMA are unschedulable.
+3. Both `-rdma_edisp_unsched=true -event_dispatcher_edisp_unsched=false`:
`rdma_edisp_unsched=true`, `event_dispatcher_edisp_unsched=false`; both TCP and
RDMA are unschedulable.
Review Comment:
Example 2 is factually incorrect: setting only
`-event_dispatcher_edisp_unsched=true` does not make `rdma_edisp_unsched`
become `true` (it remains at its default unless explicitly set). Please adjust
the example text to distinguish the effective condition (OR) from the literal
flag values.
```suggestion
1. Only `-rdma_edisp_unsched=true`: `rdma_edisp_unsched=true`,
`event_dispatcher_edisp_unsched=false`; effective condition is true, so both
TCP and RDMA are unschedulable.
2. Only `-event_dispatcher_edisp_unsched=true`: `rdma_edisp_unsched=false`
(default), `event_dispatcher_edisp_unsched=true`; effective condition is true,
so both TCP and RDMA are unschedulable.
3. Both `-rdma_edisp_unsched=true -event_dispatcher_edisp_unsched=false`:
`rdma_edisp_unsched=true`, `event_dispatcher_edisp_unsched=false`; effective
condition is true, so both TCP and RDMA are unschedulable.
```
##########
docs/cn/rdma.md:
##########
@@ -47,7 +47,24 @@ RDMA要求数据收发所使用的内存空间必须被注册（memory register
RDMA是硬件相关的通信技术，有很多独特的概念，比如device、port、GID、LID、MaxSge等。这些参数在初始化时会从对应的网卡中读取出来，并且做出默认的选择（参见src/brpc/rdma/rdma_helper.cpp）。有时默认的选择并非用户的期望，则可以通过flag参数方式指定。
-RDMA支持事件驱动和轮询两种模式，默认是事件驱动模式，通过设置rdma_use_polling可以开启轮询模式。轮询模式下还可以设置轮询器数目（rdma_poller_num），以及是否主动放弃CPU（rdma_poller_yield）。轮询模式下还可以设置一个回调函数，在每次轮询时调用，可以配合io_uring/spdk等使用。在配合使用spdk等驱动的时候，因为spdk只支持轮询模式，并且只能在单线程使用（或者叫Run
To
Completion模式上使用）执行一个任务过程中不允许被调度到别的线程上，所以这时候需要设置（rdma_edisp_unsched）为true，使事件驱动程序一直占用一个worker线程，不能调度别的任务。
+RDMA支持事件驱动和轮询两种模式，默认是事件驱动模式，通过设置rdma_use_polling可以开启轮询模式。轮询模式下还可以设置轮询器数目（rdma_poller_num），以及是否主动放弃CPU（rdma_poller_yield）。轮询模式下还可以设置一个回调函数，在每次轮询时调用，可以配合io_uring/spdk等使用。
+
+`event_dispatcher_edisp_unsched`
是全局开关，同时影响普通模式（TCP）和RDMA模式的EventDispatcher调度行为。兼容历史配置，`rdma_edisp_unsched`
仍保留，但已标记为废弃，未来版本会移除。
+
+最终生效条件统一为：
+`event_dispatcher_edisp_unsched || rdma_edisp_unsched`
+
+启动时不会再改写用户传入的 flag，运行时严格按用户配置值生效。
+
+推荐使用方式：
+1. 新部署：只配置 `event_dispatcher_edisp_unsched`。
+2. 存量部署：`rdma_edisp_unsched` 仅作过渡兼容，逐步迁移到 `event_dispatcher_edisp_unsched`。
+3. 避免脚本中给出“冲突值”；在统一 OR 语义下，只要任一 flag 为 `true`，EventDispatcher 就不可调度。
+
+行为示例：
+1. 仅设置
`-rdma_edisp_unsched=true`：`rdma_edisp_unsched=true`、`event_dispatcher_edisp_unsched=false`；TCP和RDMA均不可调度。
+2. 仅设置 `-event_dispatcher_edisp_unsched=true`：两个flag同为`true`；TCP和RDMA均不可调度。
+3. 同时设置 `-rdma_edisp_unsched=true
-event_dispatcher_edisp_unsched=false`：`rdma_edisp_unsched=true`、`event_dispatcher_edisp_unsched=false`；TCP和RDMA均不可调度。
Review Comment:
行为示例第2条表述不准确：仅设置 `-event_dispatcher_edisp_unsched=true` 并不会使
`rdma_edisp_unsched` 也变为 `true`（除非用户显式设置）。建议修改示例文案，区分“最终生效条件（OR）为 true”和“各 flag
实际取值”。
```suggestion
行为示例（其中“最终不可调度条件”指 `rdma_edisp_unsched OR event_dispatcher_edisp_unsched`）：
1. 仅设置
`-rdma_edisp_unsched=true`：`rdma_edisp_unsched=true`、`event_dispatcher_edisp_unsched=false`，最终不可调度条件为
`true`；TCP 和 RDMA 均不可调度。
2. 仅设置
`-event_dispatcher_edisp_unsched=true`：`rdma_edisp_unsched=false`（保持默认值）、`event_dispatcher_edisp_unsched=true`，最终不可调度条件为
`true`；TCP 和 RDMA 均不可调度。
3. 同时设置 `-rdma_edisp_unsched=true
-event_dispatcher_edisp_unsched=false`：`rdma_edisp_unsched=true`、`event_dispatcher_edisp_unsched=false`，最终不可调度条件为
`true`；TCP 和 RDMA 均不可调度。
```
--
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]
