Improve updateAccountPeers by bypassing AM and using account directly (#1193)

Improve updateAccountPeers performance by bypassing AM and using the account directly

management/server/account.go

@@ -1491,9 +1491,7 @@ func (am *DefaultAccountManager) GetAccountFromToken(claims jwtclaims.Authorizat
 							if err := am.Store.SaveAccount(account); err != nil {
 								log.Errorf("failed to save account: %v", err)
 							} else {
-								if err := am.updateAccountPeers(account); err != nil {
+								am.updateAccountPeers(account)
-									log.Errorf("failed updating account peers while updating user %s", account.Id)
-								}
 								for _, g := range addNewGroups {
 									if group := account.GetGroup(g); group != nil {
 										am.storeEvent(user.Id, user.Id, account.Id, activity.GroupAddedToUser,

management/server/dns.go

@@ -122,7 +122,9 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string
 		am.storeEvent(userID, accountID, accountID, activity.GroupRemovedFromDisabledManagementGroups, meta)
 	}
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 func toProtocolDNSConfig(update nbdns.Config) *proto.DNSConfig {

management/server/group.go

@@ -84,10 +84,7 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
 		return err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return err
-	}
 
 	// the following snippet tracks the activity and stores the group events in the event store.
 	// It has to happen after all the operations have been successfully performed.
@@ -229,7 +226,9 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string)
 
 	am.storeEvent(userId, groupID, accountId, activity.GroupDeleted, g.EventMeta())
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // ListGroups objects of the peers
@@ -281,7 +280,9 @@ func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerID string)
 		return err
 	}
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // GroupDeletePeer removes peer from the group
@@ -309,7 +310,9 @@ func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerID stri
 		}
 	}
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // GroupListPeers returns list of the peers from the group

management/server/nameserver.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/miekg/dns"
 	"github.com/rs/xid"
-	log "github.com/sirupsen/logrus"
 
 	nbdns "github.com/netbirdio/netbird/dns"
 	"github.com/netbirdio/netbird/management/server/activity"
@@ -74,11 +73,7 @@ func (am *DefaultAccountManager) CreateNameServerGroup(accountID string, name, d
 		return nil, err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		log.Error(err)
-		return newNSGroup.Copy(), status.Errorf(status.Internal, "failed to update peers after create nameserver %s", name)
-	}
 
 	am.storeEvent(userID, newNSGroup.ID, accountID, activity.NameserverGroupCreated, newNSGroup.EventMeta())
 
@@ -113,11 +108,7 @@ func (am *DefaultAccountManager) SaveNameServerGroup(accountID, userID string, n
 		return err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		log.Error(err)
-		return status.Errorf(status.Internal, "failed to update peers after update nameserver %s", nsGroupToSave.Name)
-	}
 
 	am.storeEvent(userID, nsGroupToSave.ID, accountID, activity.NameserverGroupUpdated, nsGroupToSave.EventMeta())
 
@@ -147,10 +138,7 @@ func (am *DefaultAccountManager) DeleteNameServerGroup(accountID, nsGroupID, use
 		return err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return status.Errorf(status.Internal, "failed to update peers after deleting nameserver %s", nsGroupID)
-	}
 
 	am.storeEvent(userID, nsGroup.ID, accountID, activity.NameserverGroupDeleted, nsGroup.EventMeta())
 

management/server/peer.go

@@ -290,10 +290,7 @@ func (am *DefaultAccountManager) MarkPeerConnected(peerPubKey string, connected
 	if oldStatus.LoginExpired {
 		// we need to update other peers because when peer login expires all other peers are notified to disconnect from
 		// the expired one. Here we notify them that connection is now allowed again.
-		err = am.updateAccountPeers(account)
+		am.updateAccountPeers(account)
-		if err != nil {
-			return err
-		}
 	}
 
 	return nil
@@ -364,10 +361,7 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe
 		return nil, err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return nil, err
-	}
 
 	return peer, nil
 }
@@ -433,7 +427,9 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) er
 		return err
 	}
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // GetPeerByIP returns peer by its IP
@@ -622,10 +618,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (*
 	opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain())
 	am.storeEvent(opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta)
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return nil, nil, err
-	}
 
 	networkMap := account.GetPeerNetworkMap(newPeer.ID, am.dnsDomain)
 	return newPeer, networkMap, nil
@@ -740,10 +733,7 @@ func (am *DefaultAccountManager) LoginPeer(login PeerLogin) (*Peer, *NetworkMap,
 	}
 
 	if updateRemotePeers {
-		err = am.updateAccountPeers(account)
+		am.updateAccountPeers(account)
-		if err != nil {
-			return nil, nil, err
-		}
 	}
 	return peer, account.GetPeerNetworkMap(peer.ID, am.dnsDomain), nil
 }
@@ -817,10 +807,7 @@ func (am *DefaultAccountManager) checkAndUpdatePeerSSHKey(peer *Peer, account *A
 	}
 
 	// trigger network map update
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return nil, err
-	}
 
 	return peer, nil
 }
@@ -865,7 +852,9 @@ func (am *DefaultAccountManager) UpdatePeerSSHKey(peerID string, sshKey string)
 	}
 
 	// trigger network map update
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // GetPeer for a given accountID, peerID and userID error if not found.
@@ -922,18 +911,12 @@ func updatePeerMeta(peer *Peer, meta PeerSystemMeta, account *Account) (*Peer, b
 
 // updateAccountPeers updates all peers that belong to an account.
 // Should be called when changes have to be synced to peers.
-func (am *DefaultAccountManager) updateAccountPeers(account *Account) error {
+func (am *DefaultAccountManager) updateAccountPeers(account *Account) {
 	peers := account.GetPeers()
 
 	for _, peer := range peers {
-		remotePeerNetworkMap, err := am.GetNetworkMap(peer.ID)
+		remotePeerNetworkMap := account.GetPeerNetworkMap(peer.ID, am.dnsDomain)
-		if err != nil {
-			return err
-		}
-
 		update := toSyncResponse(nil, peer, nil, remotePeerNetworkMap, am.GetDNSDomain())
 		am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{Update: update})
 	}
-
-	return nil
 }

management/server/policy.go

@@ -350,7 +350,9 @@ func (am *DefaultAccountManager) SavePolicy(accountID, userID string, policy *Po
 	}
 	am.storeEvent(userID, policy.ID, accountID, action, policy.EventMeta())
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // DeletePolicy from the store
@@ -375,7 +377,9 @@ func (am *DefaultAccountManager) DeletePolicy(accountID, policyID, userID string
 
 	am.storeEvent(userID, policy.ID, accountID, activity.PolicyRemoved, policy.EventMeta())
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // ListPolicies from the store

management/server/route.go

@@ -9,7 +9,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/status"
 	"github.com/netbirdio/netbird/route"
 	"github.com/rs/xid"
-	log "github.com/sirupsen/logrus"
 )
 
 // GetRoute gets a route object from account and route IDs
@@ -185,11 +184,7 @@ func (am *DefaultAccountManager) CreateRoute(accountID, network, peerID string,
 		return nil, err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		log.Error(err)
-		return &newRoute, status.Errorf(status.Internal, "failed to update peers after create route %s", newPrefix)
-	}
 
 	am.storeEvent(userID, newRoute.ID, accountID, activity.RouteCreated, newRoute.EventMeta())
 
@@ -250,10 +245,7 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave
 		return err
 	}
 
-	err = am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
-	if err != nil {
-		return err
-	}
 
 	am.storeEvent(userID, routeToSave.ID, accountID, activity.RouteUpdated, routeToSave.EventMeta())
 
@@ -283,7 +275,9 @@ func (am *DefaultAccountManager) DeleteRoute(accountID, routeID, userID string)
 
 	am.storeEvent(userID, routy.ID, accountID, activity.RouteRemoved, routy.EventMeta())
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 // ListRoutes returns a list of routes from account

management/server/setupkey.go

@@ -317,7 +317,9 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup
 		}
 	}()
 
-	return newKey, am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return newKey, nil
 }
 
 // ListSetupKeys returns a list of all setup keys of the account

management/server/user.go

@@ -377,7 +377,9 @@ func (am *DefaultAccountManager) deleteRegularUser(account *Account, initiatorUs
 	meta := map[string]any{"name": tuName, "email": tuEmail}
 	am.storeEvent(initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta)
 
-	return am.updateAccountPeers(account)
+	am.updateAccountPeers(account)
+
+	return nil
 }
 
 func (am *DefaultAccountManager) deleteUserPeers(initiatorUserID string, targetUserID string, account *Account) error {
@@ -674,9 +676,7 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd
 			return nil, err
 		}
 
-		if err := am.updateAccountPeers(account); err != nil {
+		am.updateAccountPeers(account)
-			log.Errorf("failed updating account peers while updating user %s", accountID)
-		}
 	} else {
 		if err = am.Store.SaveAccount(account); err != nil {
 			return nil, err
@@ -870,9 +870,7 @@ func (am *DefaultAccountManager) expireAndUpdatePeers(account *Account, peers []
 	if len(peerIDs) != 0 {
 		// this will trigger peer disconnect from the management service
 		am.peersUpdateManager.CloseChannels(peerIDs)
-		if err := am.updateAccountPeers(account); err != nil {
+		am.updateAccountPeers(account)
-			return err
-		}
 	}
 	return nil
 }