[PR #6463] [MERGED] [signal] Serialize concurrent sends to a peer stream #28278

Closed
opened 2026-08-05 08:06:05 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6463
Author: @lixmal
Created: 6/18/2026
Status: Merged
Merged: 6/18/2026
Merged by: @lixmal

Base: mainHead: signal-serialize-peer-send


📝 Commits (1)

  • 24fa5df Serialize concurrent sends to a peer signal stream

📊 Changes

3 files changed (+79 additions, -1 deletions)

View changed files

📝 signal/peer/peer.go (+11 -0)
signal/server/concurrent_send_test.go (+67 -0)
📝 signal/server/signal.go (+1 -1)

📄 Description

Describe your changes

The signal server forwarded messages to a peer by calling Stream.Send on the same gRPC stream from multiple goroutines at once: Send is a unary RPC (one goroutine per call) and every sender targeting a peer ends up calling forwardMessageToPeer for that peer concurrently. gRPC does not allow concurrent SendMsg on a single ServerStream, so a peer that is the destination of many senders at the same time (for example a routing peer with a large fan-in) can have its stream's write path raced, which can stall delivery while the connection stays open. This serializes writes per peer.

  • Add a per-peer send mutex and route all stream writes through Peer.Send

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Internal concurrency fix in the signal server; no user-facing behavior or configuration changes.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Resolved a potential race condition in message delivery by ensuring messages to the same peer are sent serially, preventing concurrent write conflicts.
  • Tests

    • Added test to verify message delivery serialization under concurrent conditions.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6463 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 6/18/2026 **Status:** ✅ Merged **Merged:** 6/18/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `main` ← **Head:** `signal-serialize-peer-send` --- ### 📝 Commits (1) - [`24fa5df`](https://github.com/netbirdio/netbird/commit/24fa5df9e4821e4921381d1b71bd3afe17954557) Serialize concurrent sends to a peer signal stream ### 📊 Changes **3 files changed** (+79 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `signal/peer/peer.go` (+11 -0) ➕ `signal/server/concurrent_send_test.go` (+67 -0) 📝 `signal/server/signal.go` (+1 -1) </details> ### 📄 Description ## Describe your changes The signal server forwarded messages to a peer by calling `Stream.Send` on the same gRPC stream from multiple goroutines at once: `Send` is a unary RPC (one goroutine per call) and every sender targeting a peer ends up calling `forwardMessageToPeer` for that peer concurrently. gRPC does not allow concurrent `SendMsg` on a single `ServerStream`, so a peer that is the destination of many senders at the same time (for example a routing peer with a large fan-in) can have its stream's write path raced, which can stall delivery while the connection stays open. This serializes writes per peer. - Add a per-peer send mutex and route all stream writes through `Peer.Send` ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [x] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Internal concurrency fix in the signal server; no user-facing behavior or configuration changes. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Resolved a potential race condition in message delivery by ensuring messages to the same peer are sent serially, preventing concurrent write conflicts. * **Tests** * Added test to verify message delivery serialization under concurrent conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:06:05 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28278