[PR #6331] [MERGED] [client] Persist sync response via pluggable store (disk on iOS) #29364

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6331
Author: @pappz
Created: 6/3/2026
Status: Merged
Merged: 6/3/2026
Merged by: @pappz

Base: mainHead: feature/sync-store


📝 Commits (4)

  • e930b35 Persist sync response via pluggable store (disk on iOS)
  • db1eb34 syncstore: silence nilnil linter on "nothing stored" returns
  • 030bf3d syncstore: hold syncRespMux for the whole store Set/Get
  • 8a5f3c6 syncstore: avoid StateDir "." when state path is empty

📊 Changes

7 files changed (+262 additions, -36 deletions)

View changed files

📝 client/internal/connect.go (+6 -0)
📝 client/internal/engine.go (+54 -36)
client/internal/syncstore/disk.go (+99 -0)
client/internal/syncstore/factory_ios.go (+9 -0)
client/internal/syncstore/factory_other.go (+9 -0)
client/internal/syncstore/memory.go (+56 -0)
client/internal/syncstore/syncstore.go (+29 -0)

📄 Description

The latest Management sync response (which carries the network map) was kept in memory for debug bundle generation. On memory-constrained platforms like iOS the network map can be large enough to matter.

Introduce a syncstore package with a Store interface and two backends: a memory backend (the previous behavior) and a disk backend that serializes the response to a file in the state directory. The backend is selected per-platform at build time: disk on iOS, memory elsewhere.

The disk store clears any leftover file on construction so a fresh store never reads stale data from an earlier run (e.g. another profile's network map).

In the engine, drop the separate persistSyncResponse bool: the store is only instantiated while persistence is enabled, and its presence is what marks persistence as active. The store is also cleared on engine close so the file does not linger on disk.

Describe your changes

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)

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

  • New Features

    • Platform-aware sync persistence: iOS persists sync data to disk; other platforms use an in-memory store.
    • Engine honors a configurable state directory for where sync state is stored.
  • Refactor

    • Sync persistence moved to a pluggable store model with safe read/get/clear semantics; enabling/disabling persistence now initializes or clears persistent storage.
  • Bug Fixes

    • Shutdown clears disabled persisted sync state to avoid stale data.

🔄 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/6331 **Author:** [@pappz](https://github.com/pappz) **Created:** 6/3/2026 **Status:** ✅ Merged **Merged:** 6/3/2026 **Merged by:** [@pappz](https://github.com/pappz) **Base:** `main` ← **Head:** `feature/sync-store` --- ### 📝 Commits (4) - [`e930b35`](https://github.com/netbirdio/netbird/commit/e930b3586dc07fe3bd91afe85a897061ec7fe521) Persist sync response via pluggable store (disk on iOS) - [`db1eb34`](https://github.com/netbirdio/netbird/commit/db1eb34fbdca96aa0ba27e7b88c9f9d4573c3dbc) syncstore: silence nilnil linter on "nothing stored" returns - [`030bf3d`](https://github.com/netbirdio/netbird/commit/030bf3debc88c6f9f80006d234589551da2c9275) syncstore: hold syncRespMux for the whole store Set/Get - [`8a5f3c6`](https://github.com/netbirdio/netbird/commit/8a5f3c6151877527b179ebd612650882a5315291) syncstore: avoid StateDir "." when state path is empty ### 📊 Changes **7 files changed** (+262 additions, -36 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/connect.go` (+6 -0) 📝 `client/internal/engine.go` (+54 -36) ➕ `client/internal/syncstore/disk.go` (+99 -0) ➕ `client/internal/syncstore/factory_ios.go` (+9 -0) ➕ `client/internal/syncstore/factory_other.go` (+9 -0) ➕ `client/internal/syncstore/memory.go` (+56 -0) ➕ `client/internal/syncstore/syncstore.go` (+29 -0) </details> ### 📄 Description The latest Management sync response (which carries the network map) was kept in memory for debug bundle generation. On memory-constrained platforms like iOS the network map can be large enough to matter. Introduce a syncstore package with a Store interface and two backends: a memory backend (the previous behavior) and a disk backend that serializes the response to a file in the state directory. The backend is selected per-platform at build time: disk on iOS, memory elsewhere. The disk store clears any leftover file on construction so a fresh store never reads stale data from an earlier run (e.g. another profile's network map). In the engine, drop the separate persistSyncResponse bool: the store is only instantiated while persistence is enabled, and its presence is what marks persistence as active. The store is also cleared on engine close so the file does not linger on disk. ## Describe your changes ## 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) - [ ] 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) ### 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 * **New Features** * Platform-aware sync persistence: iOS persists sync data to disk; other platforms use an in-memory store. * Engine honors a configurable state directory for where sync state is stored. * **Refactor** * Sync persistence moved to a pluggable store model with safe read/get/clear semantics; enabling/disabling persistence now initializes or clears persistent storage. * **Bug Fixes** * Shutdown clears disabled persisted sync state to avoid stale data. <!-- 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:07:56 -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#29364