-
-
Notifications
You must be signed in to change notification settings - Fork 925
[management] refactor: introduce incremental network map builder #4438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces an experimental incremental network map builder to optimize network map generation performance. Instead of rebuilding maps from scratch each time, the new builder applies incremental updates based on network map change events.
- Introduces the new
NetworkMapBuilderwith incremental update capabilities - Adds environment variable
NB_EXPERIMENT_NETWORK_MAPto enable the experimental builder - Implements incremental peer add/delete operations and cache management
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
management/server/types/networkmapbuilder.go |
Core implementation of the incremental network map builder with caching |
management/server/types/networkmap_golden_test.go |
Comprehensive test suite with golden file testing for both old and new builders |
management/server/types/networkmap.go |
Moved network map logic from account.go with experimental builder integration |
management/server/user.go |
Added network map cache updates for peer expiration |
management/server/peer.go |
Integrated experimental builder calls for peer operations |
management/server/account.go |
Added experimental builder flag and holder for cache management |
| Multiple manager files | Added network map cache recalculation calls for various operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| seenRoute[r.ID] = struct{}{} | ||
|
|
||
| if r.Enabled { | ||
| // maybe here is some mess - here we store peer key (see comment below) |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment suggests confusion about the logic. Clarify why peer.Key is stored in r.Peer field and reference the specific comment mentioned.
| // maybe here is some mess - here we store peer key (see comment below) | |
| // Here we store peer.Key in r.Peer for enabled routes. | |
| // This is intentional and follows the logic from the original account.getRoutingPeerRoutes (see comment at line 635 below), | |
| // where for group routes we store peerID, but for enabled routes we store the peer's key. |
| // and here we store peer ID - this logic is taken from original account.getRoutingPeerRoutes | ||
| newPeerRoute.Peer = peerID |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inconsistency between storing peer.Key in some places and peerID in others (as noted in the comments) suggests potential confusion. Consider using consistent field naming or add clear documentation about when each is used.
| // and here we store peer ID - this logic is taken from original account.getRoutingPeerRoutes | |
| newPeerRoute.Peer = peerID | |
| // Store peer.Key for consistency (was previously peerID, which is inconsistent) | |
| newPeerRoute.Peer = peer.Key |
|




Describe your changes
This PR refactors the network map generation approach by introducing a new, experimental network map builder. Instead of rebuilding the map from scratch, this builder applies incremental updates based on network map change events.
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
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/__