[feature][bug] Add tcp transport event dispatcher unsched flag & fix RDMA event dispatcher unsched flag by MalikHou · Pull Request #3238 · apache/brpc

MalikHou · 2026-03-06T12:37:06Z

Event Dispatcher Unsched Flag

What problem does this PR solve?

Problem Summary:

Event-dispatch scheduling behavior was not unified across transports.
RDMA used a transport-specific flag (rdma_edisp_unsched), while TCP had no unified switch.
In RDMA path, bthread_start_urgent and bthread_start_background logic was reversed, causing behavior opposite to expected unsched semantics.
The scheduling decision logic was duplicated in transport code, making behavior harder to reason about and test.

What is changed and the side effects?

Changed:

Added a unified flag in dispatcher layer: event_dispatcher_edisp_unsched.
Added EventDispatcherUnsched() helper in event_dispatcher.{h,cpp} as single source of truth.
Updated TcpTransport::ProcessEvent and RdmaTransport::ProcessEvent to use EventDispatcherUnsched():
- false -> bthread_start_urgent
- true -> bthread_start_background
Fixed RDMA bug where bthread_start_urgent and bthread_start_background branches were previously swapped.
Deprecated rdma_edisp_unsched and kept compatibility through dispatcher-layer unified check.
Added/updated tests in test/brpc_event_dispatcher_unittest.cpp:
- event_dispatcher_unsched_by_unified_flag
- event_dispatcher_unsched_by_legacy_rdma_flag (when BRPC_WITH_RDMA)
- tcp_unsched_true_returns_before_onedge_finish
- tcp_unsched_false_blocks_caller_when_single_worker

Effects:

event_dispatcher_edisp_unsched=true may reduce immediate foreground switching of dispatcher path.
rdma_edisp_unsched is deprecated.
RDMA runtime behavior now matches intended unsched semantics after fixing the swapped urgent/background branches. Users depending on previous incorrect behavior should migrate and re-check tuning.

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.

…atcher unsched flag

[feature][bugfix] Add tcp transport event dispatcher unsched flag & fix RDMA event dispatcher

Copilot

Pull request overview

This PR unifies “unsched” scheduling behavior across TCP and RDMA transports by introducing a single dispatcher-layer flag/helper, and updates transport event processing to use that unified decision (also correcting RDMA’s urgent/background selection).

Changes:

Added event_dispatcher_edisp_unsched and EventDispatcherUnsched() as the single source of truth for “unsched” behavior (with legacy RDMA flag OR’ed in when enabled).
Updated TcpTransport::ProcessEvent and RdmaTransport::ProcessEvent to pick bthread_start_urgent vs bthread_start_background via EventDispatcherUnsched().
Updated RDMA docs to describe the unified flag and deprecation of rdma_edisp_unsched.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/brpc/tcp_transport.cpp	Switches ProcessEvent scheduling decision to `EventDispatcherUnsched()`.
src/brpc/rdma_transport.cpp	Uses unified unsched helper and fixes urgent/background selection for RDMA.
src/brpc/rdma/rdma_endpoint.h	Removes the legacy `rdma_edisp_unsched` declaration from an installed header.
src/brpc/rdma/rdma_endpoint.cpp	Removes the legacy `rdma_edisp_unsched` definition (re-homed elsewhere).
src/brpc/event_dispatcher.h	Declares new unified flag and helper (but has an incorrect semantics comment).
src/brpc/event_dispatcher.cpp	Defines new unified flag + legacy flag (deprecated) and implements `EventDispatcherUnsched()`.
docs/en/rdma.md	Documents unified flag semantics and deprecation (contains an incorrect example).
docs/cn/rdma.md	Same as English RDMA doc update (contains an incorrect example).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T01:00:04Z

src/brpc/event_dispatcher.h

+// false -> background start (allowing schedule away),
+// true  -> urgent start (foreground scheduling before caller continues).


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.

Suggested change

// false -> background start (allowing schedule away),

// true -> urgent start (foreground scheduling before caller continues).

// false -> urgent start (foreground scheduling before caller continues),

// true -> background start (allowing schedule away).

Copilot · 2026-03-07T01:00:05Z

src/brpc/rdma/rdma_endpoint.h

 DECLARE_bool(rdma_use_polling);
 DECLARE_int32(rdma_poller_num);
-DECLARE_bool(rdma_edisp_unsched);
 DECLARE_bool(rdma_disable_bthread);


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.

Suggested change

DECLARE_bool(rdma_disable_bthread);

DECLARE_bool(rdma_disable_bthread);

DECLARE_bool(rdma_edisp_unsched); // Deprecated: kept for source compatibility

Copilot · 2026-03-07T01:00:05Z

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
+}


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.

Copilot · 2026-03-07T01:00:05Z

docs/en/rdma.md

+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.


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.

Suggested change

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.

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.

Copilot · 2026-03-07T01:00:06Z

docs/cn/rdma.md

+行为示例：
+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均不可调度。


行为示例第2条表述不准确：仅设置 -event_dispatcher_edisp_unsched=true 并不会使 rdma_edisp_unsched 也变为 true（除非用户显式设置）。建议修改示例文案，区分“最终生效条件（OR）为 true”和“各 flag 实际取值”。

Suggested change

行为示例：

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均不可调度。

行为示例（其中“最终不可调度条件”指 `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 均不可调度。

MalikHou and others added 2 commits March 6, 2026 20:26

add tcp transport event dispatcher unsched flag & fix RDMA event disp…

6b73ee9

…atcher unsched flag

Merge pull request #1 from MalikHou/malikhou/add_edisp_unsched

77db6c5

[feature][bugfix] Add tcp transport event dispatcher unsched flag & fix RDMA event dispatcher

chenBright requested a review from Copilot March 7, 2026 00:53

Copilot started reviewing on behalf of chenBright March 7, 2026 00:54 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

wwbmmm requested a review from yanglimingcn March 7, 2026 02:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature][bug] Add tcp transport event dispatcher unsched flag & fix RDMA event dispatcher unsched flag#3238

[feature][bug] Add tcp transport event dispatcher unsched flag & fix RDMA event dispatcher unsched flag#3238
MalikHou wants to merge 2 commits intoapache:masterfrom
MalikHou:master

MalikHou commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// false -> background start (allowing schedule away),
		// true -> urgent start (foreground scheduling before caller continues).

	DECLARE_bool(rdma_disable_bthread);
	DECLARE_bool(rdma_disable_bthread);
	DECLARE_bool(rdma_edisp_unsched); // Deprecated: kept for source compatibility

Conversation

MalikHou commented Mar 6, 2026

What problem does this PR solve?

What is changed and the side effects?

Check List:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants