DYNR/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-08-05 00:15:26 -04:00
Code Issues 23.5k Packages Projects Releases 10 Wiki Activity

[PR #5943] [CLOSED] [management] Cached serial check on sync #24395

New Issue
Open
opened 2026-08-05 06:08:38 -04:00 by saavagebueno · 0 comments
saavagebueno commented 2026-08-05 06:08:38 -04:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5943
Author: @mlsmaycon
Created: 4/21/2026
Status: ❌ Closed

Base: main ← Head: cached-serial-check-on-sync

📝 Commits (10+)

  • 3f4ef00 [management] Skip full network map on Sync when peer state is unchanged
  • 8430b06 [management] Add Redis-backed kill switch for Sync fast path
  • 5d58000 Merge branch 'main' into cached-serial-check-on-sync
  • 3716838 Remove unused cacheKey helper and testcontainers imports, simplify Redis container setup
  • 48c080b Replace Redis dependency with a generic cache store for fast path flag handling
  • 93391fc generate only current.bin and android_current.bin on ci/cd
  • 3eb1298 Refactor sync fast path tests and fix CI flakiness
  • 46446ac Add detailed timing logs to sync fast path operations
  • 66494d6 Replace Tracef with Debugf for sync fast path logging
  • dc86c96 Improve timing precision in sync fast path logging

📊 Changes

37 files changed (+2448 additions, -52 deletions)

View changed files

📝 .github/workflows/golang-test-linux.yml (+4 -1)
📝 client/cmd/testutil_test.go (+1 -1)
📝 client/internal/engine_test.go (+1 -1)
📝 client/server/server_test.go (+1 -1)
📝 management/internals/controllers/network_map/controller/controller.go (+11 -2)
📝 management/internals/controllers/network_map/interface.go (+5 -0)
📝 management/internals/controllers/network_map/interface_mock.go (+17 -2)
📝 management/internals/server/boot.go (+3 -1)
➕ management/internals/shared/fastpathcache/invalidator.go (+48 -0)
➕ management/internals/shared/grpc/fast_path_caches.go (+122 -0)
➕ management/internals/shared/grpc/fast_path_flag.go (+131 -0)
➕ management/internals/shared/grpc/fast_path_flag_test.go (+176 -0)
➕ management/internals/shared/grpc/peer_serial_cache.go (+82 -0)
➕ management/internals/shared/grpc/peer_serial_cache_decision_test.go (+116 -0)
➕ management/internals/shared/grpc/peer_serial_cache_test.go (+134 -0)
📝 management/internals/shared/grpc/server.go (+91 -28)
➕ management/internals/shared/grpc/sync_fast_path.go (+562 -0)
➕ management/internals/shared/grpc/sync_fast_path_response_test.go (+163 -0)
📝 management/server/account.go (+16 -0)
📝 management/server/account/manager.go (+3 -0)

...and 17 more files

📄 Description

Describe your changes

[management] Cached serial check on Sync

Summary

Adds a Redis-gated fast path to the management gRPC Sync handler that lets
returning peers skip full NetworkMap computation when their client-side
state already matches the account's current serial. When the fast path
applies, the peer receives a lightweight SyncResponse containing only a
fresh NetbirdConfig (TURN + Relay tokens) — mirroring what
TimeBasedAuthSecretsManager already pushes on token refresh — and relies on
its existing in-memory state.

No proto changes. No client changes. The optimisation is gated behind a
runtime kill switch so it can be rolled out gradually and disabled
cluster-wide without a redeploy.

Why

Every Sync today runs the full network-map computation even when nothing
has changed since the peer last connected. For a large account with many
peers reconnecting (e.g. after a management restart, a flaky network event,
or a routine client reconnect), that is a lot of wasted computation — all to
deliver the same bytes the peer already has.

Observations that made this safe to short-circuit:

  • Account state mutations bump Network.Serial via IncrementNetworkSerial.
  • NetworkMap.Serial on the last delivered map is already a monotonic
    witness of what the peer has.
  • Peer metadata changes (hostname, kernel, IP) already hash into
    metaHash(peerMeta, realIP) — so a meta change is independently observable
    without peeking into the peer object.
  • TimeBasedAuthSecretsManager already demonstrates that sending
    NetbirdConfig-only responses mid-stream is supported by every historical
    client (it's how token refreshes work).

Design

Fast-path gate

The fast path runs only when all of the following hold:

  1. Shared peerSerialCache is wired (nil cache disables it).
  2. FastPathFlag.Enabled() reports true (Redis-gated kill switch).
  3. The peer is not Android. Android's GrpcClient.GetNetworkMap errors on
    a nil NetworkMap — we bail out to stay safe.
  4. The cache holds an entry for this peer pubkey.
  5. The cached Serial matches network.CurrentSerial().
  6. The cached MetaHash matches the incoming metaHash(peerMeta, realIP).
  7. The cached Serial is non-zero (guard against uninitialised entries).

Any false branch falls through to the existing slow path unchanged.

Response shape

buildFastPathResponse constructs a SyncResponse with only
NetbirdConfig populated — same shape as
TimeBasedAuthSecretsManager.pushNewTURNAndRelayTokens. It includes:

  • Fresh TURN credentials (when time-based TURN is configured).
  • Fresh Relay token (when Relay is configured).
  • ExtraSettings from the settings manager (best-effort; errors log at debug).
  • Signal config, STUN config, and any integration-extended config via
    integrationsConfig.ExtendNetBirdConfig.

It omits NetworkMap, PeerConfig, Checks, and RemotePeers. The client
keeps whatever state it already has and just refreshes its control-plane
credentials.

Race handling (re-check after subscribe)

Between the eligibility check (GetAccountNetwork → serial N) and the
peer's subscription (OnPeerConnected) there is a window where another
writer could commit serial N+1 and broadcast. A peer that subscribes after
the broadcast lands never receives the update for N+1 and ends up at stale
serial N with no NetworkMap.

commitFastPath closes this race:

  1. MarkPeerConnected.
  2. GetPeerByPeerPubKey.
  3. OnPeerConnected → subscribe to the update channel.
  4. Re-fetch GetAccountNetwork and compare to the initial serial.
  5. If the serial advanced (or the re-fetch fails), call
    cancelPeerRoutinesWithoutLock to tear down the subscription and return
    committed=false so the caller falls through to the slow path — which
    delivers the new full map.

Only if the re-check succeeds does runFastPathSync actually send the lean
response.

Cache lifecycle

  • Written by the server after a successful wire send — either at the
    end of the slow-path sendInitialSync (recordPeerSyncEntry) or from the
    per-update handler for NetworkMap-typed messages
    (recordPeerSyncEntryFromUpdate).
  • Invalidated on every successful Login (invalidatePeerSyncEntry) so
    state changes that happen through login (SSH key rotation, approval, user
    binding) always yield a full map on the next Sync.
  • Expired by TTL (DefaultPeerSerialCacheTTL = 24h) — self-healing if a
    Set is dropped, the entry simply times out and the next Sync is a slow path
    that re-primes the cache.

Cache writes are best-effort: errors log at debug and do not fail the Sync.
Reads treat any error (including the miss sentinel) as a miss so the slow
path runs.

Runtime kill switch

FastPathFlag wraps an atomic.Bool populated by RunFastPathFlagRoutine,
which polls the shared cache store (same Redis used by the rest of
management, so no extra env var) every minute for key peerSyncFastPath.

Key value (trimmed) Effect
1, true, TRUE, True, " true " fast path enabled
0, false, "", missing key, yes, anything else fast path disabled

Properties:

  • Fail-closed: any store read error (other than the NotFound miss) flips
    the flag to disabled and logs at error level. A transient Redis outage
    while enabled rolls back to the slow path for the affected replica until
    Redis recovers.
  • No extra config: reuses BaseServer.CacheStore() — so the same
    NB_CACHE_REDIS_ADDRESS that already wires the shared cache wires the
    flag. Without Redis, the store falls back to in-process gocache which is
    enough for single-replica dev/test.
  • Default off: a brand-new deployment with no key set stays on the slow
    path until an operator sets the key.
  • Nil-safe: a nil *FastPathFlag (or one constructed by NewFastPathFlag(false))
    reports disabled so tests and consumers can opt out entirely.

Ops flow

# Enable cluster-wide
redis-cli -u $NB_CACHE_REDIS_ADDRESS SET peerSyncFastPath 1

# Disable cluster-wide
redis-cli -u $NB_CACHE_REDIS_ADDRESS DEL peerSyncFastPath
# or
redis-cli -u $NB_CACHE_REDIS_ADDRESS SET peerSyncFastPath 0

Propagation latency is ≤ 1 minute per replica (the poll interval).

Scenarios

Scenario Path Rationale
Fresh peer, first Sync slow No cache entry → miss → slow path primes cache.
Returning peer, same serial, same meta, flag on fast All conditions met; lightweight response with fresh tokens.
Returning peer, same serial, same meta, flag off slow Kill switch bypasses the whole optimisation.
Returning peer, account serial advanced (another peer registered / policy change / etc.) slow shouldSkipNetworkMap fails on serial mismatch; full map delivered.
Returning peer, hostname / kernel / realIP changed slow metaHash mismatch; full map delivered so integrations see the new meta.
Returning peer, serial advances during the fast-path attempt slow (fallback) commitFastPath re-check detects the advance after subscribing; tears down, returns false, slow path runs.
Android peer (any state) slow Android client errors on nil NetworkMap; gate unconditionally skips.
Returning peer after Login call slow invalidatePeerSyncEntry removed the entry on Login; cache miss → slow path.
Cache write dropped on the previous Sync slow Next Sync misses the cache → slow path → re-primes.
Redis transient read failure while flag was enabled slow Flag fails closed to disabled for that replica; recovers on next successful read.
Redis completely unavailable (NB_CACHE_REDIS_ADDRESS unset) slow Shared store is in-process gocache; single-replica deployments can still toggle by setting the key programmatically in tests.
v0.20 / v0.40 / v0.60 legacy clients slow (first Sync) First Sync primes cache. v0.40 reconnects do hit the fast path today (documented tradeoff — v0.40's GetNetworkMap call is indistinguishable from a main Sync on the server); will tighten when a proto opt-in lands.

Compatibility

  • Wire format: no change. SyncResponse with NetbirdConfig set and
    NetworkMap nil is already supported by every historical client because
    that's what TimeBasedAuthSecretsManager pushes on TURN/Relay token
    refresh.
  • Proto schema: no change. Future PR may add an opt-in flag so legacy
    clients can be identified and forced to the slow path on reconnect; the
    documented legacy-reconnect tradeoff above has a failing test ready to
    tighten when that lands.
  • Android: explicitly skipped — GrpcClient.GetNetworkMap on Android
    errors on a nil NetworkMap and was only fixed upstream in v0.50.0.

Testing

Unit

  • TestParseFastPathFlag — flag value parsing table.
  • TestFastPathFlag_{EnabledDefaultsFalse,NilSafeEnabled,SetEnabled} —
    flag container behaviour.
  • TestRunFastPathFlagRoutine_{NilStoreStaysDisabled,ReadsFlagFromStore,MissingKeyKeepsDisabled,DefaultKeyUsedWhenEmpty,FailsClosedOnReadError}
    — runtime routine behaviour, including the fail-closed flip via a
    flakyStore wrapper that injects transient errors.
  • TestShouldSkipNetworkMap — 10-row table over the eligibility predicate.
  • TestPeerSerialCache_{GetSetDelete,GetMissReturnsZero,TTLExpiry,OverwriteUpdatesValue,IsolatedPerKey,Concurrent,Redis}
    — cache wrapper behaviour over both gocache and a Redis testcontainer.
  • TestBuildFastPathResponse_* — response composition across
    time-based-TURN, static-TURN, no-relay, and extra-settings-error branches.

Behavioural (in-process gRPC)

  • TestSyncFastPath_FirstSync_SendsFullMap — first Sync is always slow.
  • TestSyncFastPath_SecondSync_MatchingSerial_SkipsMap — happy path: lean
    response, fresh tokens, no NetworkMap.
  • TestSyncFastPath_AndroidNeverSkips — Android gate holds even with primed
    cache.
  • TestSyncFastPath_MetaChanged_SendsFullMap — meta hash change forces
    slow path.
  • TestSyncFastPath_LoginInvalidatesCache — post-Login Sync hits the slow
    path.
  • TestSyncFastPath_OtherPeerRegistered_ForcesFullMap — serial advance
    caused by another peer's registration forces the slow path.

Legacy wire-format regression

testdata/sync_request_wire/ contains frozen SyncRequest bytes for
v0.20.0, v0.40.0, and v0.60.0. Tests replay each fixture through the
in-process server to assert:

  • TestSync_WireFixture_LegacyClients_AlwaysReceiveFullMap — first Sync
    always delivers a full map.
  • TestSync_WireFixture_LegacyClient_ReconnectStillGetsFullMap — pins the
    current accepted tradeoff where a v0.40 reconnect with a primed cache
    does hit the fast path (test must be tightened when proto opt-in lands).
  • TestSync_WireFixture_AndroidReconnect_NeverSkips — android gate holds
    at the wire level.
  • TestSync_WireFixture_ModernClientReconnect_TakesFastPath — baseline
    fast-path hit over the real wire.

current.bin and android_current.bin are regenerated by CI
(go run ./management/server/testdata/sync_request_wire/generate.go) and
.gitignore'd; the three legacy fixtures are checked in and frozen.

Test hygiene

  • Fixed-sleep waitForPeerDisconnect replaced with a bounded
    require.Eventually poll on peer.Status.Connected — no more timing
    flakes under race detector.
  • Windows skip consolidated into a single skipOnWindows(t) helper and
    applied uniformly to every fast-path + legacy-wire test.

Rollback

  1. redis-cli -u $NB_CACHE_REDIS_ADDRESS DEL peerSyncFastPath — propagates
    within 1 minute, every replica falls back to the slow path.
  2. If a deeper rollback is needed, revert the PR; the code is additive and
    its only integration point is the optional peerSerialCache + fastPathFlag
    params in NewServer.

Issue ticket number and link

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)

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)

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

Release Notes

  • New Features
    • Added Sync fast path optimization that improves reconnection performance by delivering lightweight responses when network state remains unchanged.
    • Implemented network state caching and feature-flag gating for optimized peer synchronization.
    • Maintained full backward compatibility with legacy clients.

🔄 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/5943 **Author:** [@mlsmaycon](https://github.com/mlsmaycon) **Created:** 4/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `cached-serial-check-on-sync` --- ### 📝 Commits (10+) - [`3f4ef00`](https://github.com/netbirdio/netbird/commit/3f4ef0031b22aa0c742dd2b42209c1d0881750e2) [management] Skip full network map on Sync when peer state is unchanged - [`8430b06`](https://github.com/netbirdio/netbird/commit/8430b06f2a5eee339d4cf5a878ab7335a5f0e0a9) [management] Add Redis-backed kill switch for Sync fast path - [`5d58000`](https://github.com/netbirdio/netbird/commit/5d58000dbd5ee25c2183a6478e18e3d6a597c4c9) Merge branch 'main' into cached-serial-check-on-sync - [`3716838`](https://github.com/netbirdio/netbird/commit/3716838c25b2516bd235b0c80d3789cf5f7a75f7) Remove unused cacheKey helper and testcontainers imports, simplify Redis container setup - [`48c080b`](https://github.com/netbirdio/netbird/commit/48c080b8612fcf9b5aa1418ae73dff574d182f36) Replace Redis dependency with a generic cache store for fast path flag handling - [`93391fc`](https://github.com/netbirdio/netbird/commit/93391fc68fc78c8791e002f0ddb2d7da51a91594) generate only current.bin and android_current.bin on ci/cd - [`3eb1298`](https://github.com/netbirdio/netbird/commit/3eb1298cb4ff3d26233315ebd867ed154c17a4e3) Refactor sync fast path tests and fix CI flakiness - [`46446ac`](https://github.com/netbirdio/netbird/commit/46446acd301aef2a00ec507ee9c5940741afb353) Add detailed timing logs to sync fast path operations - [`66494d6`](https://github.com/netbirdio/netbird/commit/66494d61afbc2b105f18a74750532ec7e59bcf67) Replace Tracef with Debugf for sync fast path logging - [`dc86c96`](https://github.com/netbirdio/netbird/commit/dc86c9655d1d4ab11d9bc2243cc21e77a20eb110) Improve timing precision in sync fast path logging ### 📊 Changes **37 files changed** (+2448 additions, -52 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/golang-test-linux.yml` (+4 -1) 📝 `client/cmd/testutil_test.go` (+1 -1) 📝 `client/internal/engine_test.go` (+1 -1) 📝 `client/server/server_test.go` (+1 -1) 📝 `management/internals/controllers/network_map/controller/controller.go` (+11 -2) 📝 `management/internals/controllers/network_map/interface.go` (+5 -0) 📝 `management/internals/controllers/network_map/interface_mock.go` (+17 -2) 📝 `management/internals/server/boot.go` (+3 -1) ➕ `management/internals/shared/fastpathcache/invalidator.go` (+48 -0) ➕ `management/internals/shared/grpc/fast_path_caches.go` (+122 -0) ➕ `management/internals/shared/grpc/fast_path_flag.go` (+131 -0) ➕ `management/internals/shared/grpc/fast_path_flag_test.go` (+176 -0) ➕ `management/internals/shared/grpc/peer_serial_cache.go` (+82 -0) ➕ `management/internals/shared/grpc/peer_serial_cache_decision_test.go` (+116 -0) ➕ `management/internals/shared/grpc/peer_serial_cache_test.go` (+134 -0) 📝 `management/internals/shared/grpc/server.go` (+91 -28) ➕ `management/internals/shared/grpc/sync_fast_path.go` (+562 -0) ➕ `management/internals/shared/grpc/sync_fast_path_response_test.go` (+163 -0) 📝 `management/server/account.go` (+16 -0) 📝 `management/server/account/manager.go` (+3 -0) _...and 17 more files_ </details> ### 📄 Description ## Describe your changes # [management] Cached serial check on Sync ## Summary Adds a Redis-gated fast path to the management gRPC `Sync` handler that lets returning peers skip full `NetworkMap` computation when their client-side state already matches the account's current serial. When the fast path applies, the peer receives a lightweight `SyncResponse` containing only a fresh `NetbirdConfig` (TURN + Relay tokens) — mirroring what `TimeBasedAuthSecretsManager` already pushes on token refresh — and relies on its existing in-memory state. No proto changes. No client changes. The optimisation is gated behind a runtime kill switch so it can be rolled out gradually and disabled cluster-wide without a redeploy. ## Why Every `Sync` today runs the full network-map computation even when nothing has changed since the peer last connected. For a large account with many peers reconnecting (e.g. after a management restart, a flaky network event, or a routine client reconnect), that is a lot of wasted computation — all to deliver the same bytes the peer already has. Observations that made this safe to short-circuit: - Account state mutations bump `Network.Serial` via `IncrementNetworkSerial`. - `NetworkMap.Serial` on the last delivered map is already a monotonic witness of what the peer has. - Peer metadata changes (hostname, kernel, IP) already hash into `metaHash(peerMeta, realIP)` — so a meta change is independently observable without peeking into the peer object. - `TimeBasedAuthSecretsManager` already demonstrates that sending `NetbirdConfig`-only responses mid-stream is supported by every historical client (it's how token refreshes work). ## Design ### Fast-path gate The fast path runs only when **all** of the following hold: 1. Shared `peerSerialCache` is wired (nil cache disables it). 2. `FastPathFlag.Enabled()` reports true (Redis-gated kill switch). 3. The peer is **not** Android. Android's `GrpcClient.GetNetworkMap` errors on a nil `NetworkMap` — we bail out to stay safe. 4. The cache holds an entry for this peer pubkey. 5. The cached `Serial` matches `network.CurrentSerial()`. 6. The cached `MetaHash` matches the incoming `metaHash(peerMeta, realIP)`. 7. The cached `Serial` is non-zero (guard against uninitialised entries). Any `false` branch falls through to the existing slow path unchanged. ### Response shape `buildFastPathResponse` constructs a `SyncResponse` with only `NetbirdConfig` populated — same shape as `TimeBasedAuthSecretsManager.pushNewTURNAndRelayTokens`. It includes: - Fresh TURN credentials (when time-based TURN is configured). - Fresh Relay token (when Relay is configured). - `ExtraSettings` from the settings manager (best-effort; errors log at debug). - Signal config, STUN config, and any integration-extended config via `integrationsConfig.ExtendNetBirdConfig`. It omits `NetworkMap`, `PeerConfig`, `Checks`, and `RemotePeers`. The client keeps whatever state it already has and just refreshes its control-plane credentials. ### Race handling (re-check after subscribe) Between the eligibility check (`GetAccountNetwork` → serial N) and the peer's subscription (`OnPeerConnected`) there is a window where another writer could commit serial N+1 and broadcast. A peer that subscribes *after* the broadcast lands never receives the update for N+1 and ends up at stale serial N with no `NetworkMap`. `commitFastPath` closes this race: 1. `MarkPeerConnected`. 2. `GetPeerByPeerPubKey`. 3. `OnPeerConnected` → subscribe to the update channel. 4. Re-fetch `GetAccountNetwork` and compare to the initial serial. 5. If the serial advanced (or the re-fetch fails), call `cancelPeerRoutinesWithoutLock` to tear down the subscription and return `committed=false` so the caller falls through to the slow path — which delivers the new full map. Only if the re-check succeeds does `runFastPathSync` actually send the lean response. ### Cache lifecycle - **Written** by the server *after* a successful wire send — either at the end of the slow-path `sendInitialSync` (`recordPeerSyncEntry`) or from the per-update handler for `NetworkMap`-typed messages (`recordPeerSyncEntryFromUpdate`). - **Invalidated** on every successful `Login` (`invalidatePeerSyncEntry`) so state changes that happen through login (SSH key rotation, approval, user binding) always yield a full map on the next Sync. - **Expired** by TTL (`DefaultPeerSerialCacheTTL = 24h`) — self-healing if a Set is dropped, the entry simply times out and the next Sync is a slow path that re-primes the cache. Cache writes are best-effort: errors log at debug and do not fail the Sync. Reads treat any error (including the miss sentinel) as a miss so the slow path runs. ### Runtime kill switch `FastPathFlag` wraps an `atomic.Bool` populated by `RunFastPathFlagRoutine`, which polls the shared cache store (same Redis used by the rest of management, so no extra env var) every minute for key `peerSyncFastPath`. | Key value (trimmed) | Effect | | --- | --- | | `1`, `true`, `TRUE`, `True`, `" true "` | fast path **enabled** | | `0`, `false`, `""`, missing key, `yes`, anything else | fast path **disabled** | Properties: - **Fail-closed**: any store read error (other than the NotFound miss) flips the flag to disabled and logs at error level. A transient Redis outage while enabled rolls back to the slow path for the affected replica until Redis recovers. - **No extra config**: reuses `BaseServer.CacheStore()` — so the same `NB_CACHE_REDIS_ADDRESS` that already wires the shared cache wires the flag. Without Redis, the store falls back to in-process gocache which is enough for single-replica dev/test. - **Default off**: a brand-new deployment with no key set stays on the slow path until an operator sets the key. - **Nil-safe**: a nil `*FastPathFlag` (or one constructed by `NewFastPathFlag(false)`) reports disabled so tests and consumers can opt out entirely. #### Ops flow ```sh # Enable cluster-wide redis-cli -u $NB_CACHE_REDIS_ADDRESS SET peerSyncFastPath 1 # Disable cluster-wide redis-cli -u $NB_CACHE_REDIS_ADDRESS DEL peerSyncFastPath # or redis-cli -u $NB_CACHE_REDIS_ADDRESS SET peerSyncFastPath 0 ``` Propagation latency is ≤ 1 minute per replica (the poll interval). ## Scenarios | Scenario | Path | Rationale | | --- | --- | --- | | Fresh peer, first Sync | slow | No cache entry → miss → slow path primes cache. | | Returning peer, same serial, same meta, flag on | fast | All conditions met; lightweight response with fresh tokens. | | Returning peer, same serial, same meta, flag off | slow | Kill switch bypasses the whole optimisation. | | Returning peer, account serial advanced (another peer registered / policy change / etc.) | slow | `shouldSkipNetworkMap` fails on serial mismatch; full map delivered. | | Returning peer, hostname / kernel / realIP changed | slow | `metaHash` mismatch; full map delivered so integrations see the new meta. | | Returning peer, serial advances *during* the fast-path attempt | slow (fallback) | `commitFastPath` re-check detects the advance after subscribing; tears down, returns false, slow path runs. | | Android peer (any state) | slow | Android client errors on nil `NetworkMap`; gate unconditionally skips. | | Returning peer after `Login` call | slow | `invalidatePeerSyncEntry` removed the entry on Login; cache miss → slow path. | | Cache write dropped on the previous Sync | slow | Next Sync misses the cache → slow path → re-primes. | | Redis transient read failure while flag was enabled | slow | Flag fails closed to disabled for that replica; recovers on next successful read. | | Redis completely unavailable (`NB_CACHE_REDIS_ADDRESS` unset) | slow | Shared store is in-process gocache; single-replica deployments can still toggle by setting the key programmatically in tests. | | v0.20 / v0.40 / v0.60 legacy clients | slow (first Sync) | First Sync primes cache. v0.40 reconnects *do* hit the fast path today (documented tradeoff — v0.40's `GetNetworkMap` call is indistinguishable from a main Sync on the server); will tighten when a proto opt-in lands. | ## Compatibility - **Wire format**: no change. `SyncResponse` with `NetbirdConfig` set and `NetworkMap` nil is already supported by every historical client because that's what `TimeBasedAuthSecretsManager` pushes on TURN/Relay token refresh. - **Proto schema**: no change. Future PR may add an opt-in flag so legacy clients can be identified and forced to the slow path on reconnect; the documented legacy-reconnect tradeoff above has a failing test ready to tighten when that lands. - **Android**: explicitly skipped — `GrpcClient.GetNetworkMap` on Android errors on a nil `NetworkMap` and was only fixed upstream in v0.50.0. ## Testing ### Unit - `TestParseFastPathFlag` — flag value parsing table. - `TestFastPathFlag_{EnabledDefaultsFalse,NilSafeEnabled,SetEnabled}` — flag container behaviour. - `TestRunFastPathFlagRoutine_{NilStoreStaysDisabled,ReadsFlagFromStore,MissingKeyKeepsDisabled,DefaultKeyUsedWhenEmpty,FailsClosedOnReadError}` — runtime routine behaviour, including the fail-closed flip via a `flakyStore` wrapper that injects transient errors. - `TestShouldSkipNetworkMap` — 10-row table over the eligibility predicate. - `TestPeerSerialCache_{GetSetDelete,GetMissReturnsZero,TTLExpiry,OverwriteUpdatesValue,IsolatedPerKey,Concurrent,Redis}` — cache wrapper behaviour over both gocache and a Redis testcontainer. - `TestBuildFastPathResponse_*` — response composition across time-based-TURN, static-TURN, no-relay, and extra-settings-error branches. ### Behavioural (in-process gRPC) - `TestSyncFastPath_FirstSync_SendsFullMap` — first Sync is always slow. - `TestSyncFastPath_SecondSync_MatchingSerial_SkipsMap` — happy path: lean response, fresh tokens, no `NetworkMap`. - `TestSyncFastPath_AndroidNeverSkips` — Android gate holds even with primed cache. - `TestSyncFastPath_MetaChanged_SendsFullMap` — meta hash change forces slow path. - `TestSyncFastPath_LoginInvalidatesCache` — post-Login Sync hits the slow path. - `TestSyncFastPath_OtherPeerRegistered_ForcesFullMap` — serial advance caused by another peer's registration forces the slow path. ### Legacy wire-format regression `testdata/sync_request_wire/` contains frozen `SyncRequest` bytes for v0.20.0, v0.40.0, and v0.60.0. Tests replay each fixture through the in-process server to assert: - `TestSync_WireFixture_LegacyClients_AlwaysReceiveFullMap` — first Sync always delivers a full map. - `TestSync_WireFixture_LegacyClient_ReconnectStillGetsFullMap` — pins the current accepted tradeoff where a v0.40 reconnect with a primed cache *does* hit the fast path (test must be tightened when proto opt-in lands). - `TestSync_WireFixture_AndroidReconnect_NeverSkips` — android gate holds at the wire level. - `TestSync_WireFixture_ModernClientReconnect_TakesFastPath` — baseline fast-path hit over the real wire. `current.bin` and `android_current.bin` are regenerated by CI (`go run ./management/server/testdata/sync_request_wire/generate.go`) and `.gitignore`'d; the three legacy fixtures are checked in and frozen. ### Test hygiene - Fixed-sleep `waitForPeerDisconnect` replaced with a bounded `require.Eventually` poll on `peer.Status.Connected` — no more timing flakes under race detector. - Windows skip consolidated into a single `skipOnWindows(t)` helper and applied uniformly to every fast-path + legacy-wire test. ## Rollback 1. `redis-cli -u $NB_CACHE_REDIS_ADDRESS DEL peerSyncFastPath` — propagates within 1 minute, every replica falls back to the slow path. 2. If a deeper rollback is needed, revert the PR; the code is additive and its only integration point is the optional `peerSerialCache` + `fastPathFlag` params in `NewServer`. ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > 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) ### 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 ## Release Notes * **New Features** * Added Sync fast path optimization that improves reconnection performance by delivering lightweight responses when network state remains unchanged. * Implemented network state caching and feature-flag gating for optimized peer synchronization. * Maintained full backward compatibility with legacy clients. <!-- 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 06:08:38 -04:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
2021 Q4
2021 Q4
2021 Q4
2022 Q1
2022 Q1
2022 Q1
2022 Q1
2022 Q1
2022 Q1
accessibility
accessibility
accessibility
acl
acl
acl
agent
agent
agent
agent
agent
agent
Android
Android
Android
Android
Android
Android
api
api
api
authentik
authentik
authentik
automation
automation
automation
azure
azure
azure
battery-usage
battery-usage
battery-usage
bug
cache
cache
cache
client
client
client
client-ui
client-ui
client-ui
cloud
cloud
cloud
cloud-only
cloud-only
cloud-only
cloudflare
cloudflare
cloudflare
community
community
community
compatibility
compatibility
compatibility
config-idp
config-idp
config-idp
config-issue
config-issue
config-issue
connection
connection
connection
contribution
contribution
contribution
coturn
coturn
coturn
cross-vpn
cross-vpn
cross-vpn
dashboard
dashboard
dashboard
data-usage
data-usage
data-usage
distribution
distribution
distribution
dns
dns
dns
docker
docker
docker
documentation
documentation
documentation
duplicate
duplicate
duplicate
enhancement
enhancement
event-stream
event-stream
event-stream
feature-request
feature-request
feature-request
freebsd
freebsd
freebsd
getting-started
getting-started
getting-started
go
go
go
good first issue
good first issue
good first issue
gui
gui
gui
help wanted
help wanted
help wanted
home-assistant
home-assistant
home-assistant
idp
idp
idp
inconsistency
inconsistency
inconsistency
integration
integration
integration
integrations
integrations
integrations
ios
ios
ios
ipv6
ipv6
ipv6
jwt
jwt
jwt
k8s
k8s
k8s
keycloak
keycloak
keycloak
linux
linux
linux
login
login
login
macos
macos
macos
management-service
management-service
management-service
Medium
Medium
Medium
missing-docs
missing-docs
missing-docs
mobile
mobile
mobile
moved-internal
moved-internal
moved-internal
needs-review
needs-review
needs-review
netbird-ui
netbird-ui
netbird-ui
networking
networking
networking
new-platform
new-platform
new-platform
nginx
nginx
nginx
notification
notification
notification
okta
okta
okta
openwrt
openwrt
openwrt
P2
P2
P2
packaging
packaging
packaging
peer-management
peer-management
peer-management
peer-management
peer-management
peer-management
peer-management
peer-management
peer-management
performance
performance
performance
postgres
postgres
postgres
posture-checks
posture-checks
posture-checks
psk
psk
psk
pull-request
Mirrored from GitHub Pull Request
 question
question
question
refactor
refactor
refactor
relay
relay
relay
release
release
release
rfc
rfc
rfc
routes
routes
routes
security
security
security
security-improvement
security-improvement
security-improvement
security-related
security-related
security-related
self-hosting
self-hosting
self-hosting
server
server
server
signal
signal
signal
sleep-issue
sleep-issue
sleep-issue
ssh
ssh
ssh
ssl
ssl
ssl
status
status
status
store
store
store
synology
synology
synology
system-compatibility-issue
system-compatibility-issue
system-compatibility-issue
test-suite
test-suite
test-suite
third-party-integration
third-party-integration
third-party-integration
triage
triage
triage
triage
triage
triage
triage-needed
triage-needed
triage-needed
troubleshooting
troubleshooting
troubleshooting
UX
UX
UX
waiting-feedback
waiting-feedback
waiting-feedback
windows
windows
windows
wontfix
wontfix
wontfix
zitadel
zitadel
zitadel
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dynr-servers saavagebueno
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#24395
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?