[management] shift network map data store onto the read replica (#8302)

This commit is contained in:
Pascal Fischer
2026-10-25 22:34:44 +01:00
committed by GitHub
parent dd38fabc82
commit fdc51d6f95
22 changed files with 1095 additions and 47 deletions
@@ -10,14 +10,21 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"
"testing"
"time"
"github.com/jackc/pgx/v5/pgxpool"
networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/types"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/attribute"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.uber.org/mock/gomock"
)
@@ -29,6 +36,19 @@ var goldenNMap string
const EnvUpdateGoldenData = "NMAP_UPDATE_GOLDEN_DATA"
var seedNetworkMapDataOnce sync.Once
func seedNetworkMapData(t *testing.T, ctx context.Context) {
t.Helper()
seedNetworkMapDataOnce.Do(func() {
for _, query := range strings.Split(nmapData, ";") {
if err := store(t).Exec(ctx, query); err != nil {
log.Fatalf("error initializing nmap test: %s", err.Error())
}
}
})
}
func TestGetNetworkMapData(t *testing.T) {
ctx := context.TODO()
@@ -51,11 +71,7 @@ func TestGetNetworkMapData(t *testing.T) {
IntegratedPeerValidator: peerValidators,
}
for _, query := range strings.Split(nmapData, ";") {
if err := store(t).Exec(ctx, query); err != nil {
log.Fatalf("error initializing nmap test: %s", err.Error())
}
}
seedNetworkMapData(t, ctx)
nmap, err := storeImpl.GetNetworkMapData(ctx, "account-33")
assert.NoError(t, err)
@@ -72,3 +88,52 @@ func TestGetNetworkMapData(t *testing.T) {
}
assert.Equal(t, goldenNMap, string(serializedNMap))
}
func TestGetNetworkMapData_FromReplica(t *testing.T) {
if engine != string(types.PostgresStoreEngine) {
t.Skip("replicas exist only for Postgres")
}
ctx := context.TODO()
extraSettingsManager := settings.NewMockManager(gomock.NewController(t))
extraSettingsManager.EXPECT().GetExtraSettings(gomock.Any(), gomock.Any()).Return(&types.ExtraSettings{}, nil)
peerValidators := integrated_validator.NewMockIntegratedValidator(gomock.NewController(t))
peerValidators.EXPECT().GetValidatedPeers(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[string]struct{}{}, nil)
seedNetworkMapData(t, ctx)
reader := sdkmetric.NewManualReader()
metrics, err := networkmapdb.NewMetrics(sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)).Meter("test"),
map[string]*pgxpool.Pool{"replica": pgstore.Pool})
require.NoError(t, err)
primary := store(t)
storeImpl := networkmapdb.NetworkMapDBStoreImpl{
Metrics: metrics,
Store: primary,
Replica: primary,
ReplicaWait: time.Second,
ExtraSettingsManager: extraSettingsManager,
IntegratedPeerValidator: peerValidators,
}
nmap, err := storeImpl.GetNetworkMapData(ctx, "account-33")
require.NoError(t, err)
assert.NotEmpty(t, nmap.Peers, "data must be read through the replica transaction after its attempt deadline is released")
var rm metricdata.ResourceMetrics
require.NoError(t, reader.Collect(ctx, &rm))
var totalConns int64
for _, sm := range rm.ScopeMetrics {
for _, m := range sm.Metrics {
if gauge, ok := m.Data.(metricdata.Gauge[int64]); ok {
for _, dp := range gauge.DataPoints {
if state, _ := dp.Attributes.Value(attribute.Key("state")); state.AsString() == "total" {
totalConns += dp.Value
}
}
}
}
}
assert.Positive(t, totalConns, "the replica pool gauge reports its open connections")
}
@@ -110,7 +110,7 @@ func NewController(ctx context.Context, store store.Store, metrics telemetry.App
if nmdataStore != nil {
interval := requestbuffer.Interval(ctx, "NB_NETWORK_MAP_DATA_BUFFER_INTERVAL", defaultNetworkMapDataBufferInterval)
log.WithContext(ctx).Infof("set network map data request buffer interval to %s", interval)
c.nmdataBuffer = requestbuffer.New(ctx, "network map data request buffer", interval, c.fetchNetworkMapData)
c.nmdataBuffer = requestbuffer.NewSerialized(ctx, "network map data request buffer", interval, c.fetchNetworkMapData)
}
return c
@@ -199,6 +199,10 @@ func (m *managerImpl) DeletePeers(ctx context.Context, accountID string, peerIDs
return err
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
log.WithContext(ctx).Debugf("DeletePeers: deleted peer %s", peerID)
if !(peer.ProxyMeta.Embedded || peer.Meta.KernelVersion == "wasm") {
@@ -5,6 +5,8 @@ import (
"fmt"
"testing"
"github.com/netbirdio/netbird/management/internals/controllers/network_map"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
@@ -195,3 +197,33 @@ func TestManager_GetPeersPaginated_NoPermission_User_IsRestrictable_ViewBlocked(
assert.Equal(t, 0, total)
assert.NoError(t, err)
}
func TestManager_DeletePeers_IncrementsNetworkSerial(t *testing.T) {
ctrl := gomock.NewController(t)
mockStore := store.NewMockStore(ctrl)
txStore := store.NewMockStore(ctrl)
accountManager := account.NewMockManager(ctrl)
networkMapController := network_map.NewMockController(ctrl)
m := NewManager(mockStore, permissions.NewMockManager(ctrl)).(*managerImpl)
m.accountManager = accountManager
m.networkMapController = networkMapController
ctx := context.Background()
mockStore.EXPECT().GetAccountSettings(gomock.Any(), store.LockingStrengthNone, "test-account").Return(&types.Settings{}, nil)
networkMapController.EXPECT().GetDNSDomain(gomock.Any()).Return("netbird.selfhosted")
mockStore.EXPECT().ExecuteInTransaction(gomock.Any(), gomock.Any()).Times(2).
DoAndReturn(func(_ context.Context, fn func(store.Store) error) error { return fn(txStore) })
for _, id := range []string{"peer-1", "peer-2"} {
txStore.EXPECT().GetPeerByID(gomock.Any(), store.LockingStrengthNone, "test-account", id).Return(&peer.Peer{ID: id, Status: &peer.PeerStatus{}}, nil)
txStore.EXPECT().RemovePeerFromAllGroups(gomock.Any(), id).Return(nil)
txStore.EXPECT().GetPolicyRulesByResourceID(gomock.Any(), store.LockingStrengthNone, "test-account", id).Return(nil, nil)
txStore.EXPECT().DeletePeer(gomock.Any(), "test-account", id).Return(nil)
}
txStore.EXPECT().IncrementNetworkSerial(gomock.Any(), "test-account").Times(2).Return(nil)
accountManager.EXPECT().StoreEvent(gomock.Any(), "test-user", gomock.Any(), "test-account", gomock.Any(), gomock.Any()).Times(2)
accountManager.EXPECT().UpdateAccountPeers(gomock.Any(), "test-account", gomock.Any())
assert.NoError(t, m.DeletePeers(ctx, "test-account", []string{"peer-1", "peer-2"}, "test-user", false))
}
@@ -6,9 +6,9 @@ import (
"testing"
"time"
"go.uber.org/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
@@ -38,6 +38,7 @@ func setupL4Test(t *testing.T, customPortsSupported *bool) (*Manager, store.Stor
err = testStore.SaveAccount(ctx, &types.Account{
Id: testAccountID,
CreatedBy: testUserID,
Network: types.NewNetwork(),
Settings: &types.Settings{
PeerExposeEnabled: true,
PeerExposeGroups: []string{testGroupID},
@@ -363,6 +363,10 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *
return fmt.Errorf("create service: %w", err)
}
if err := transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
}
@@ -534,6 +538,10 @@ func (m *Manager) persistNewEphemeralService(ctx context.Context, accountID, pee
return fmt.Errorf("create service: %w", err)
}
if err := transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
}
@@ -744,6 +752,10 @@ func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.St
return fmt.Errorf("update service: %w", err)
}
if err := transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
}
@@ -1027,6 +1039,10 @@ func (m *Manager) DeleteService(ctx context.Context, accountID, userID, serviceI
return fmt.Errorf("failed to delete service: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
if err != nil {
@@ -1069,6 +1085,12 @@ func (m *Manager) DeleteAllServices(ctx context.Context, accountID, userID strin
}
}
if len(services) > 0 {
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
}
return nil
})
if err != nil {
@@ -1425,6 +1447,10 @@ func (m *Manager) deletePeerService(ctx context.Context, accountID, peerID, serv
return fmt.Errorf("delete service: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
if err != nil {
@@ -1477,6 +1503,11 @@ func (m *Manager) deleteExpiredPeerService(ctx context.Context, accountID, peerI
if err = transaction.DeleteService(ctx, accountID, serviceID); err != nil {
return fmt.Errorf("delete service: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
deleted = true
return nil
@@ -227,6 +227,13 @@ func TestCheckDomainAvailable_EdgeCases(t *testing.T) {
})
}
func networkSerial(t *testing.T, s store.Store, accountID string) uint64 {
t.Helper()
account, err := s.GetAccount(context.Background(), accountID)
require.NoError(t, err)
return account.Network.Serial
}
func TestPersistNewService(t *testing.T) {
ctx := context.Background()
accountID := "test-account"
@@ -254,6 +261,9 @@ func TestPersistNewService(t *testing.T) {
txMock.EXPECT().
CreateService(ctx, service).
Return(nil)
txMock.EXPECT().
IncrementNetworkSerial(ctx, accountID).
Return(nil)
return fn(txMock)
})
@@ -465,6 +475,9 @@ func TestDeletePeerService_SourcePeerValidation(t *testing.T) {
txMock.EXPECT().
DeleteService(ctx, accountID, serviceID).
Return(nil)
txMock.EXPECT().
IncrementNetworkSerial(ctx, accountID).
Return(nil)
return fn(txMock)
})
mockStore.EXPECT().
@@ -570,6 +583,9 @@ func TestDeletePeerService_SourcePeerValidation(t *testing.T) {
txMock.EXPECT().
DeleteService(ctx, accountID, serviceID).
Return(nil)
txMock.EXPECT().
IncrementNetworkSerial(ctx, accountID).
Return(nil)
return fn(txMock)
})
mockStore.EXPECT().
@@ -617,6 +633,9 @@ func TestDeletePeerService_SourcePeerValidation(t *testing.T) {
txMock.EXPECT().
DeleteService(ctx, accountID, serviceID).
Return(nil)
txMock.EXPECT().
IncrementNetworkSerial(ctx, accountID).
Return(nil)
return fn(txMock)
})
mockStore.EXPECT().
@@ -679,6 +698,7 @@ func setupIntegrationTest(t *testing.T) (*Manager, store.Store) {
err = testStore.SaveAccount(ctx, &types.Account{
Id: testAccountID,
CreatedBy: testUserID,
Network: types.NewNetwork(),
Settings: &types.Settings{
PeerExposeEnabled: true,
PeerExposeGroups: []string{testGroupID},
@@ -843,6 +863,7 @@ func TestCreateServiceFromPeer(t *testing.T) {
assert.Equal(t, rpservice.SourceEphemeral, persisted.Source, "source should be ephemeral")
assert.Equal(t, testPeerID, persisted.SourcePeer, "source peer should be set")
assert.NotNil(t, persisted.Meta.LastRenewedAt, "last renewed should be set")
assert.Equal(t, uint64(1), networkSerial(t, testStore, testAccountID), "network serial should be incremented")
})
t.Run("creates service with custom domain", func(t *testing.T) {
@@ -1049,8 +1070,10 @@ func TestDeleteService_DeletesEphemeralExpose(t *testing.T) {
svc, err := testStore.GetServiceByDomain(ctx, resp.Domain)
require.NoError(t, err)
serial := networkSerial(t, testStore, testAccountID)
err = mgr.DeleteService(ctx, testAccountID, testUserID, svc.ID)
require.NoError(t, err)
assert.Equal(t, serial+1, networkSerial(t, testStore, testAccountID), "network serial should be incremented")
count, err = mgr.store.CountEphemeralServicesByPeer(ctx, store.LockingStrengthNone, testAccountID, testPeerID)
require.NoError(t, err)
@@ -1065,7 +1088,7 @@ func TestDeleteService_DeletesEphemeralExpose(t *testing.T) {
func TestDeleteAllServices_DeletesEphemeralExposes(t *testing.T) {
ctx := context.Background()
mgr, _ := setupIntegrationTest(t)
mgr, testStore := setupIntegrationTest(t)
for i := range 3 {
_, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
@@ -1079,8 +1102,10 @@ func TestDeleteAllServices_DeletesEphemeralExposes(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, int64(3), count, "all ephemeral services should exist")
serial := networkSerial(t, testStore, testAccountID)
err = mgr.DeleteAllServices(ctx, testAccountID, testUserID)
require.NoError(t, err)
assert.Equal(t, serial+1, networkSerial(t, testStore, testAccountID), "network serial should be incremented")
count, err = mgr.store.CountEphemeralServicesByPeer(ctx, store.LockingStrengthNone, testAccountID, testPeerID)
require.NoError(t, err)
@@ -1223,8 +1248,10 @@ func TestDeleteExpiredPeerService_DeletesTargets(t *testing.T) {
require.Len(t, targets, 1, "ephemeral peer-exposed service should have exactly one persisted target before reaping")
expireEphemeralService(t, testStore, testAccountID, resp.Domain)
serial := networkSerial(t, testStore, testAccountID)
err = mgr.deleteExpiredPeerService(ctx, testAccountID, testPeerID, svcID)
require.NoError(t, err)
assert.Equal(t, serial+1, networkSerial(t, testStore, testAccountID), "network serial should be incremented")
_, err = testStore.GetServiceByDomain(ctx, resp.Domain)
require.Error(t, err, "expired peer-exposed service should be deleted")
@@ -1253,8 +1280,10 @@ func TestDeleteServiceFromPeer_DeletesTargets(t *testing.T) {
require.NoError(t, err)
require.Len(t, targets, 1, "ephemeral peer-exposed service should have exactly one persisted target before stopping")
serial := networkSerial(t, testStore, testAccountID)
err = mgr.StopServiceFromPeer(ctx, testAccountID, testPeerID, svcID)
require.NoError(t, err)
assert.Equal(t, serial+1, networkSerial(t, testStore, testAccountID), "network serial should be incremented")
_, err = testStore.GetServiceByDomain(ctx, resp.Domain)
require.Error(t, err, "stopped peer-exposed service should be deleted")
@@ -1521,3 +1550,30 @@ func TestReloadAllServicesForAccount_ContinuesAfterFailedService(t *testing.T) {
assert.Contains(t, err.Error(), "svc-broken")
assert.NotContains(t, err.Error(), "svc-healthy")
}
func TestServiceWrites_IncrementNetworkSerial(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
created, err := mgr.CreateService(ctx, testAccountID, testUserID, &rpservice.Service{
Name: "serial", Domain: "serial.test.netbird.io", Mode: rpservice.ModeHTTP, Enabled: true,
Targets: []*rpservice.Target{{
AccountID: testAccountID, TargetId: testPeerID,
TargetType: rpservice.TargetTypePeer, Protocol: "http", Port: 8080, Enabled: true,
}},
})
require.NoError(t, err)
serial := networkSerial(t, testStore, testAccountID)
assert.Equal(t, uint64(1), serial, "create should increment the network serial")
updated := created.Copy()
updated.Name = "serial renamed"
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
assert.Equal(t, serial+1, networkSerial(t, testStore, testAccountID), "update should increment the network serial")
serial++
require.NoError(t, mgr.SetStatus(ctx, testAccountID, created.ID, rpservice.StatusActive))
require.NoError(t, mgr.SetCertificateIssuedAt(ctx, testAccountID, created.ID))
assert.Equal(t, serial, networkSerial(t, testStore, testAccountID), "meta-only writes must not increment the network serial")
}
@@ -5,10 +5,15 @@ import (
"errors"
"fmt"
"os"
"time"
"github.com/jackc/pgx/v5/pgxpool"
"go.opentelemetry.io/otel/metric"
networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
networkmap_pgsql "github.com/netbirdio/netbird/management/internals/network_map_db/pgsql"
networkmap_sqlite "github.com/netbirdio/netbird/management/internals/network_map_db/sqlite"
"github.com/netbirdio/netbird/management/internals/shared/requestbuffer"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
@@ -16,7 +21,16 @@ import (
log "github.com/sirupsen/logrus"
)
const storeSqliteFileName = "store.db"
const (
storeSqliteFileName = "store.db"
// ReplicaDsnEnv names the Postgres DSN of a read replica serving network map data reads.
ReplicaDsnEnv = "NB_STORE_ENGINE_POSTGRES_REPLICA_DSN"
// replicaWaitEnv bounds how long a network map data read waits for the replica.
replicaWaitEnv = "NB_NETWORK_MAP_DATA_REPLICA_WAIT"
defaultReplicaWait = 2 * time.Second
)
var ErrNotSupportedStoreEngine = errors.New("unsupported store engine")
@@ -25,7 +39,9 @@ func NewNetworkMapDBStore(
kind types.Engine,
dataDir string,
integratedPeerValidator integrated_validator.IntegratedValidator,
extraSettingsManager settings.Manager) (*networkmapdb.NetworkMapDBStoreImpl, error) {
extraSettingsManager settings.Manager,
replicaPool *pgxpool.Pool,
meter metric.Meter) (*networkmapdb.NetworkMapDBStoreImpl, error) {
switch kind {
case types.SqliteStoreEngine:
log.WithContext(ctx).Info("networkmap store is using SQLite")
@@ -54,11 +70,32 @@ func NewNetworkMapDBStore(
return nil, err
}
return &networkmapdb.NetworkMapDBStoreImpl{
impl := &networkmapdb.NetworkMapDBStoreImpl{
Store: store,
IntegratedPeerValidator: integratedPeerValidator,
ExtraSettingsManager: extraSettingsManager,
}, nil
}
pools := map[string]*pgxpool.Pool{"primary": store.Pool}
if replicaPool != nil {
impl.Replica = &networkmap_pgsql.PgStore{Pool: replicaPool}
impl.ReplicaWait = requestbuffer.Interval(ctx, replicaWaitEnv, defaultReplicaWait)
pools["replica"] = replicaPool
replicaConfig := replicaPool.Config().ConnConfig
log.WithContext(ctx).Infof("networkmap store reads from Postgres replica %s:%d/%s, waiting up to %s for replication",
replicaConfig.Host, replicaConfig.Port, replicaConfig.Database, impl.ReplicaWait)
} else {
log.WithContext(ctx).Info("networkmap store reads from the primary, no replica configured")
}
if meter != nil {
impl.Metrics, err = networkmapdb.NewMetrics(meter, pools)
if err != nil {
return nil, fmt.Errorf("create network map data metrics: %w", err)
}
}
return impl, nil
}
return nil, fmt.Errorf("networkmap store doesn't support engine %s, %w", kind, ErrNotSupportedStoreEngine)
@@ -0,0 +1,96 @@
package networkmapdb
import (
"context"
"fmt"
"time"
"github.com/jackc/pgx/v5/pgxpool"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
)
const (
readSourceReplica = "replica"
readSourcePrimary = "primary"
readReasonNoReplica = "no_replica"
readReasonCaughtUp = "caught_up"
readReasonLagTimeout = "lag_timeout"
readReasonReplicaError = "replica_error"
readReasonSerialReadError = "serial_read_error"
)
// Metrics records which database serves network map data reads, how long reads
// wait for the replica and the state of the connection pools.
type Metrics struct {
reads metric.Int64Counter
replicaWait metric.Int64Histogram
}
// NewMetrics creates the network map data metrics on meter and observes the
// connection pools keyed by pool name.
func NewMetrics(meter metric.Meter, pools map[string]*pgxpool.Pool) (*Metrics, error) {
reads, err := meter.Int64Counter("management.nmdata.reads",
metric.WithUnit("1"),
metric.WithDescription("Network map data reads by the database that served them and the reason it was chosen"),
)
if err != nil {
return nil, fmt.Errorf("create reads counter: %w", err)
}
replicaWait, err := meter.Int64Histogram("management.nmdata.replica.wait.ms",
metric.WithUnit("milliseconds"),
metric.WithDescription("Time a network map data read waited for the replica to replicate the primary's network serial"),
)
if err != nil {
return nil, fmt.Errorf("create replica wait histogram: %w", err)
}
connections, err := meter.Int64ObservableGauge("management.nmdata.pool.connections",
metric.WithUnit("1"),
metric.WithDescription("Connections of the network map data connection pools by state"),
)
if err != nil {
return nil, fmt.Errorf("create pool connections gauge: %w", err)
}
_, err = meter.RegisterCallback(func(_ context.Context, observer metric.Observer) error {
for name, pool := range pools {
stat := pool.Stat()
observePool(observer, connections, name, "acquired", stat.AcquiredConns())
observePool(observer, connections, name, "idle", stat.IdleConns())
observePool(observer, connections, name, "total", stat.TotalConns())
}
return nil
}, connections)
if err != nil {
return nil, fmt.Errorf("create pool connections callback: %w", err)
}
return &Metrics{reads: reads, replicaWait: replicaWait}, nil
}
func observePool(observer metric.Observer, gauge metric.Int64ObservableGauge, pool, state string, value int32) {
observer.ObserveInt64(gauge, int64(value), metric.WithAttributes(
attribute.String("pool", pool),
attribute.String("state", state),
))
}
func (m *Metrics) countRead(ctx context.Context, source, reason string) {
if m == nil {
return
}
m.reads.Add(ctx, 1, metric.WithAttributes(
attribute.String("source", source),
attribute.String("reason", reason),
))
}
func (m *Metrics) recordReplicaWait(ctx context.Context, wait time.Duration) {
if m == nil {
return
}
m.replicaWait.Record(ctx, wait.Milliseconds())
}
@@ -2,7 +2,9 @@ package networkmapdb
import (
"context"
"errors"
"fmt"
"time"
log "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
@@ -11,12 +13,147 @@ import (
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
)
// GetNetworkMapData reads the network map data of the account. With a Replica
// configured it reads from the replica once that has replicated the account's
// current network serial, and from Store when it does not catch up within
// ReplicaWait or the replica read fails.
func (s *NetworkMapDBStoreImpl) GetNetworkMapData(ctx context.Context, accountId string) (*networkmap.NetworkMapData, error) {
tx, err := s.Store.BeginTx(ctx)
toret, err := s.readNetworkMapData(ctx, accountId)
if err != nil {
return nil, err
}
extraSettings, err := s.ExtraSettingsManager.GetExtraSettings(ctx, accountId)
if err != nil {
return nil, err
}
toret.ValidatedPeers, err = s.IntegratedPeerValidator.GetValidatedPeers(ctx, accountId, maps.Values(toret.Groups), maps.Values(toret.Peers), extraSettings)
if err != nil {
return nil, err
}
return toret, nil
}
func (s *NetworkMapDBStoreImpl) readNetworkMapData(ctx context.Context, accountId string) (*networkmap.NetworkMapData, error) {
reason := readReasonNoReplica
if s.Replica != nil {
toret, err := s.readFromReplica(ctx, accountId)
if err == nil {
s.Metrics.countRead(ctx, readSourceReplica, readReasonCaughtUp)
return toret, nil
}
if ctx.Err() != nil {
return nil, ctx.Err()
}
reason = fallbackReason(err)
if reason == readReasonLagTimeout {
log.WithContext(ctx).Debugf("reading network map data of account %s from primary: %v", accountId, err)
} else {
log.WithContext(ctx).Warnf("reading network map data of account %s from primary: %v", accountId, err)
}
}
tx, network, err := beginRead(ctx, s.Store, accountId)
if err != nil {
return nil, err
}
toret, err := readFromTx(ctx, tx, accountId, network)
if err != nil {
return nil, err
}
s.Metrics.countRead(ctx, readSourcePrimary, reason)
return toret, nil
}
func fallbackReason(err error) string {
switch {
case errors.Is(err, errReplicaLagging):
return readReasonLagTimeout
case errors.Is(err, errPrimarySerialRead):
return readReasonSerialReadError
default:
return readReasonReplicaError
}
}
func (s *NetworkMapDBStoreImpl) readFromReplica(ctx context.Context, accountId string) (*networkmap.NetworkMapData, error) {
tx, network, err := s.beginOnReplica(ctx, accountId)
if err != nil {
return nil, err
}
return readFromTx(ctx, tx, accountId, network)
}
// beginOnReplica opens a read transaction on the replica whose snapshot holds at
// least the account's network serial on the primary at the time of the call.
func (s *NetworkMapDBStoreImpl) beginOnReplica(ctx context.Context, accountId string) (NetworkMapDBStoreConn, nmdata.Network, error) {
primaryTx, primaryNetwork, err := beginRead(ctx, s.Store, accountId)
if err != nil {
return nil, nmdata.Network{}, fmt.Errorf("%w: %w", errPrimarySerialRead, err)
}
rollbackTx(ctx, primaryTx)
start := time.Now()
deadline := start.Add(s.ReplicaWait)
for {
tx, network, err := beginReadBefore(ctx, s.Replica, accountId, deadline)
if err != nil {
if ctx.Err() == nil && errors.Is(err, context.DeadlineExceeded) {
s.Metrics.recordReplicaWait(ctx, time.Since(start))
return nil, nmdata.Network{}, fmt.Errorf("%w: no answer within %s: %w", errReplicaLagging, s.ReplicaWait, err)
}
return nil, nmdata.Network{}, err
}
if network.Serial >= primaryNetwork.Serial {
s.Metrics.recordReplicaWait(ctx, time.Since(start))
return tx, network, nil
}
rollbackTx(ctx, tx)
remaining := time.Until(deadline)
if remaining <= 0 {
s.Metrics.recordReplicaWait(ctx, time.Since(start))
return nil, nmdata.Network{}, fmt.Errorf("%w: at network serial %d after %s, primary at %d", errReplicaLagging, network.Serial, s.ReplicaWait, primaryNetwork.Serial)
}
select {
case <-ctx.Done():
return nil, nmdata.Network{}, ctx.Err()
case <-time.After(min(replicaPollInterval, remaining)):
}
}
}
// beginReadBefore runs beginRead with its connect, begin and network read bounded
// by deadline, so a stalled replica cannot hold the read past it.
func beginReadBefore(ctx context.Context, store NetworkMapDBStore, accountId string, deadline time.Time) (NetworkMapDBStoreConn, nmdata.Network, error) {
attemptCtx, cancel := context.WithDeadline(ctx, deadline)
defer cancel()
return beginRead(attemptCtx, store, accountId)
}
// beginRead opens a read transaction and reads the account's network as its
// first statement, so the network serial belongs to the transaction's snapshot.
func beginRead(ctx context.Context, store NetworkMapDBStore, accountId string) (NetworkMapDBStoreConn, nmdata.Network, error) {
tx, err := store.BeginTx(ctx)
if err != nil {
return nil, nmdata.Network{}, err
}
network, err := tx.GetNetwork(ctx, accountId)
if err != nil {
rollbackTx(ctx, tx)
return nil, nmdata.Network{}, fmt.Errorf("get network: %w", err)
}
return tx, network, nil
}
// readFromTx reads the remaining network map data in tx and ends it.
func readFromTx(ctx context.Context, tx NetworkMapDBStoreConn, accountId string, network nmdata.Network) (*networkmap.NetworkMapData, error) {
acctSettings, err := tx.GetAccountSettings(ctx, accountId)
if err != nil {
return rollbackAndReturnError(ctx, tx, fmt.Errorf("failed to get account settings: %w", err))
@@ -41,10 +178,6 @@ func (s *NetworkMapDBStoreImpl) GetNetworkMapData(ctx context.Context, accountId
if err != nil {
return rollbackAndReturnError(ctx, tx, fmt.Errorf("failed to get network routers: %w", err))
}
network, err := tx.GetNetwork(ctx, accountId)
if err != nil {
return rollbackAndReturnError(ctx, tx, fmt.Errorf("failed to get network: %w", err))
}
peers, _, err := tx.GetPeers(ctx, accountId)
if err != nil {
return rollbackAndReturnError(ctx, tx, fmt.Errorf("failed to get peers: %w", err))
@@ -111,26 +244,20 @@ func (s *NetworkMapDBStoreImpl) GetNetworkMapData(ctx context.Context, accountId
ProxyTargetedDomainResourceIDs: proxyTargetedDomainResourceIDs,
}
extraSettings, err := s.ExtraSettingsManager.GetExtraSettings(ctx, accountId)
if err != nil {
return nil, err
}
toret.ValidatedPeers, err = s.IntegratedPeerValidator.GetValidatedPeers(ctx, accountId, maps.Values(toret.Groups), maps.Values(toret.Peers), extraSettings)
if err != nil {
return nil, err
}
return &toret, nil
}
func rollbackAndReturnError(ctx context.Context, tx NetworkMapDBStoreConn, err error) (*networkmap.NetworkMapData, error) {
if errr := tx.RollbackTx(ctx); errr != nil {
log.WithContext(ctx).Warnf("failed to rollback network map read transaction: %v", errr)
}
rollbackTx(ctx, tx)
return nil, err
}
func rollbackTx(ctx context.Context, tx NetworkMapDBStoreConn) {
if err := tx.RollbackTx(ctx); err != nil {
log.WithContext(ctx).Warnf("failed to rollback network map read transaction: %v", err)
}
}
func toMap[T any](all []T, id func(t T) string) map[string]*T {
toret := make(map[string]*T, len(all))
for _, t := range all {
@@ -1,10 +1,21 @@
package networkmapdb
import (
"context"
"errors"
"sync/atomic"
"testing"
"testing/synctest"
"time"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/attribute"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"github.com/netbirdio/netbird/shared/management/networkmap"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
)
// disabled network resource shouldn't be in the resulting map
@@ -156,3 +167,268 @@ func TestBuildResourcePolicies_ViaPolicyToDestinationGroupIdx(t *testing.T) {
},
}, resourceToPolicies)
}
type fakeNmdataStore struct {
name string
serial atomic.Int64
readErr error
stall bool
begins atomic.Int32
finishes atomic.Int32
}
func (f *fakeNmdataStore) BeginTx(ctx context.Context) (NetworkMapDBStoreConn, error) {
if f.stall {
<-ctx.Done()
return nil, ctx.Err()
}
f.begins.Add(1)
return &fakeNmdataConn{store: f}, nil
}
func (f *fakeNmdataStore) Exec(context.Context, string, ...any) error { return nil }
type fakeNmdataConn struct {
store *fakeNmdataStore
}
func (c *fakeNmdataConn) GetNetwork(context.Context, string) (nmdata.Network, error) {
return nmdata.Network{Identifier: c.store.name, Serial: c.store.serial.Load()}, nil
}
func (c *fakeNmdataConn) GetPeers(context.Context, string) ([]nmdata.Peer, map[string][]*nmdata.Peer, error) {
return nil, nil, c.store.readErr
}
func (c *fakeNmdataConn) GetGroups(context.Context, string) ([]nmdata.Group, map[string]map[string]any, error) {
return nil, nil, nil
}
func (c *fakeNmdataConn) GetDomains(context.Context, string) ([]Domain, error) { return nil, nil }
func (c *fakeNmdataConn) GetPolicies(context.Context, string) ([]nmdata.Policy, map[string]map[string]any, map[string]map[string]any, error) {
return nil, nil, nil, nil
}
func (c *fakeNmdataConn) GetRoutes(context.Context, string) ([]nmdata.Route, error) { return nil, nil }
func (c *fakeNmdataConn) GetNameServerGroups(context.Context, string) ([]nmdata.NameServerGroup, error) {
return nil, nil
}
func (c *fakeNmdataConn) GetNetworkResources(context.Context, string) ([]nmdata.NetworkResource, error) {
return nil, nil
}
func (c *fakeNmdataConn) GetNetworkRouters(context.Context, string) (map[string]map[string]*nmdata.NetworkRouter, error) {
return map[string]map[string]*nmdata.NetworkRouter{}, nil
}
func (c *fakeNmdataConn) GetAppliedZoneCandidates(context.Context, string) ([]networkmap.AppliedZoneCandidate, error) {
return nil, nil
}
func (c *fakeNmdataConn) GetAccountSettings(context.Context, string) (nmdata.AccountSettingsInfo, error) {
return nmdata.AccountSettingsInfo{}, nil
}
func (c *fakeNmdataConn) GetPostureChecks(context.Context, string) ([]nmdata.PostureChecks, map[string]string, error) {
return nil, nil, nil
}
func (c *fakeNmdataConn) GetAllowedUsers(context.Context, string) (map[string]struct{}, map[string][]string, error) {
return nil, nil, nil
}
func (c *fakeNmdataConn) GetDnsSettings(context.Context, string) (nmdata.DNSSettings, error) {
return nmdata.DNSSettings{}, nil
}
func (c *fakeNmdataConn) GetNetworkXIDToPublicIdMap(context.Context, string) (map[string]string, error) {
return map[string]string{}, nil
}
func (c *fakeNmdataConn) GetPrivateServices(context.Context, string) ([]Service, error) {
return nil, nil
}
func (c *fakeNmdataConn) GetProxyTargetedDomainResourceIDs(context.Context, string) (map[string]struct{}, error) {
return map[string]struct{}{}, nil
}
func (c *fakeNmdataConn) CommitTx(context.Context) error {
c.store.finishes.Add(1)
return nil
}
func (c *fakeNmdataConn) RollbackTx(context.Context) error {
c.store.finishes.Add(1)
return nil
}
func newReplicaTestStores(primarySerial, replicaSerial int64) (*NetworkMapDBStoreImpl, *fakeNmdataStore, *fakeNmdataStore) {
primary := &fakeNmdataStore{name: "primary"}
primary.serial.Store(primarySerial)
replica := &fakeNmdataStore{name: "replica"}
replica.serial.Store(replicaSerial)
return &NetworkMapDBStoreImpl{Store: primary, Replica: replica, ReplicaWait: time.Second}, primary, replica
}
func TestReadNetworkMapData_ReplicaCaughtUp(t *testing.T) {
s, primary, replica := newReplicaTestStores(5, 5)
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "replica", data.Network.Identifier)
assert.Equal(t, int32(1), replica.begins.Load())
assert.Equal(t, primary.begins.Load(), primary.finishes.Load())
assert.Equal(t, replica.begins.Load(), replica.finishes.Load())
}
func TestReadNetworkMapData_WaitsForReplica(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
s, _, replica := newReplicaTestStores(5, 4)
time.AfterFunc(100*time.Millisecond, func() { replica.serial.Store(5) })
start := time.Now()
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "replica", data.Network.Identifier)
assert.Equal(t, int64(5), data.Network.Serial)
assert.GreaterOrEqual(t, time.Since(start), 100*time.Millisecond)
assert.Equal(t, replica.begins.Load(), replica.finishes.Load())
})
}
func TestReadNetworkMapData_FallsBackToPrimaryWhenReplicaLags(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
s, primary, replica := newReplicaTestStores(5, 4)
start := time.Now()
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "primary", data.Network.Identifier)
assert.GreaterOrEqual(t, time.Since(start), s.ReplicaWait)
assert.Equal(t, primary.begins.Load(), primary.finishes.Load())
assert.Equal(t, replica.begins.Load(), replica.finishes.Load())
})
}
func TestReadNetworkMapData_FallsBackToPrimaryOnReplicaReadError(t *testing.T) {
s, primary, replica := newReplicaTestStores(5, 5)
replica.readErr = errors.New("canceling statement due to conflict with recovery")
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "primary", data.Network.Identifier)
assert.Equal(t, primary.begins.Load(), primary.finishes.Load())
assert.Equal(t, replica.begins.Load(), replica.finishes.Load())
}
func TestReadNetworkMapData_WithoutReplicaReadsPrimaryOnce(t *testing.T) {
primary := &fakeNmdataStore{name: "primary"}
s := &NetworkMapDBStoreImpl{Store: primary}
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "primary", data.Network.Identifier)
assert.Equal(t, int32(1), primary.begins.Load())
assert.Equal(t, int32(1), primary.finishes.Load())
}
func TestReadNetworkMapData_CancelledWhileWaitingForReplica(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
s, primary, replica := newReplicaTestStores(5, 4)
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
_, err := s.readNetworkMapData(ctx, "account")
require.ErrorIs(t, err, context.DeadlineExceeded)
assert.Equal(t, int32(1), primary.begins.Load(), "primary should only be read for the serial")
assert.Equal(t, primary.begins.Load(), primary.finishes.Load())
assert.Equal(t, replica.begins.Load(), replica.finishes.Load())
})
}
func TestReadNetworkMapData_FallsBackToPrimaryWhenReplicaStalls(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
reader := sdkmetric.NewManualReader()
metrics, err := NewMetrics(sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)).Meter("test"), nil)
require.NoError(t, err)
s, primary, replica := newReplicaTestStores(5, 5)
s.Metrics = metrics
replica.stall = true
start := time.Now()
data, err := s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
assert.Equal(t, "primary", data.Network.Identifier)
assert.Equal(t, s.ReplicaWait, time.Since(start), "a stalled replica must be abandoned at the wait deadline")
assert.Equal(t, primary.begins.Load(), primary.finishes.Load())
reads, waits := collectReadMetrics(t, reader)
assert.Equal(t, map[[2]string]int64{{readSourcePrimary, readReasonLagTimeout}: 1}, reads)
assert.Equal(t, uint64(1), waits)
})
}
func TestReadNetworkMapData_CountsReadsBySourceAndReason(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
reader := sdkmetric.NewManualReader()
metrics, err := NewMetrics(sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)).Meter("test"), nil)
require.NoError(t, err)
s, _, replica := newReplicaTestStores(5, 5)
s.Metrics = metrics
_, err = s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
replica.serial.Store(4)
_, err = s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
replica.serial.Store(5)
replica.readErr = errors.New("canceling statement due to conflict with recovery")
_, err = s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
s.Replica = nil
_, err = s.readNetworkMapData(context.Background(), "account")
require.NoError(t, err)
reads, waits := collectReadMetrics(t, reader)
assert.Equal(t, map[[2]string]int64{
{readSourceReplica, readReasonCaughtUp}: 1,
{readSourcePrimary, readReasonLagTimeout}: 1,
{readSourcePrimary, readReasonReplicaError}: 1,
{readSourcePrimary, readReasonNoReplica}: 1,
}, reads)
assert.Equal(t, uint64(3), waits, "every replica attempt that reached the serial check records its wait")
})
}
// collectReadMetrics returns the read counts keyed by source and reason and the
// number of recorded replica waits.
func collectReadMetrics(t *testing.T, reader *sdkmetric.ManualReader) (map[[2]string]int64, uint64) {
t.Helper()
var rm metricdata.ResourceMetrics
require.NoError(t, reader.Collect(context.Background(), &rm))
reads := map[[2]string]int64{}
var waits uint64
for _, sm := range rm.ScopeMetrics {
for _, m := range sm.Metrics {
switch data := m.Data.(type) {
case metricdata.Sum[int64]:
for _, dp := range data.DataPoints {
source, _ := dp.Attributes.Value(attribute.Key("source"))
reason, _ := dp.Attributes.Value(attribute.Key("reason"))
reads[[2]string{source.AsString(), reason.AsString()}] = dp.Value
}
case metricdata.Histogram[int64]:
for _, dp := range data.DataPoints {
waits += dp.Count
}
}
}
}
return reads, waits
}
@@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"reflect"
"time"
"github.com/miekg/dns"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
@@ -17,6 +18,13 @@ import (
var ErrDnsUnsupportedRecordType = errors.New("unsupported record type")
const replicaPollInterval = 10 * time.Millisecond
var (
errReplicaLagging = errors.New("replica lagging")
errPrimarySerialRead = errors.New("read primary network serial")
)
type NetworkMapDBStore interface { //nolint:revive // established name across the codebase
BeginTx(ctx context.Context) (NetworkMapDBStoreConn, error)
Exec(ctx context.Context, query string, args ...any) error
@@ -46,7 +54,14 @@ type NetworkMapDBStoreConn interface { //nolint:revive // established name acros
}
type NetworkMapDBStoreImpl struct { //nolint:revive // established name across the codebase
Store NetworkMapDBStore
Store NetworkMapDBStore
// Replica is an optional read replica of Store that serves reads once it has
// replicated the account's current network serial.
Replica NetworkMapDBStore
// ReplicaWait bounds how long a read waits for Replica before it falls back to Store.
ReplicaWait time.Duration
// Metrics records how reads are served. It is optional.
Metrics *Metrics
IntegratedPeerValidator integrated_validator.IntegratedValidator
ExtraSettingsManager settings.Manager
}
+24
View File
@@ -8,12 +8,14 @@ import (
"errors"
"net/http"
"net/netip"
"os"
"slices"
"time"
"github.com/gorilla/mux"
grpcMiddleware "github.com/grpc-ecosystem/go-grpc-middleware/v2"
"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/realip"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/rs/cors"
"github.com/rs/xid"
log "github.com/sirupsen/logrus"
@@ -44,6 +46,7 @@ import (
"github.com/netbirdio/netbird/management/server/idp"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
"github.com/netbirdio/netbird/management/server/types"
mgmtProto "github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/shared/ratelimit"
"github.com/netbirdio/netbird/util/crypt"
@@ -130,6 +133,8 @@ func (s *BaseServer) NetworkMapStore() *networkmapdb.NetworkMapDBStoreImpl {
s.Config.Datadir,
s.IntegratedValidator(),
s.SettingsManager(),
s.NetworkMapReplicaPool(),
s.Metrics().GetMeter(),
)
// networkmap db store supports postgres and sqlite backends only
// for other backends a fallback is used, so NotSupportedStoreEngineError
@@ -141,6 +146,25 @@ func (s *BaseServer) NetworkMapStore() *networkmapdb.NetworkMapDBStoreImpl {
})
}
// NetworkMapReplicaPool opens the pgx pool to the Postgres read replica serving
// network map data reads. It is nil unless the store engine is Postgres and
// networkmapdbfactory.ReplicaDsnEnv is set.
func (s *BaseServer) NetworkMapReplicaPool() *pgxpool.Pool {
return CreateNamed(s, "NetworkMapReplica", func() *pgxpool.Pool {
dsn := os.Getenv(networkmapdbfactory.ReplicaDsnEnv)
if dsn == "" || s.Config.StoreConfig.Engine != types.PostgresStoreEngine {
return nil
}
pool, err := db.OpenPgxPool(context.Background(), dsn, db.DefaultPoolConfig)
if err != nil {
log.Errorf("network map data reads use the primary, connecting to the replica failed: %v", err)
return nil
}
s.OnStop(pool.Close)
return pool
})
}
func (s *BaseServer) EventStore() activity.Store {
return Create(s, func() activity.Store {
var err error
+3 -2
View File
@@ -106,7 +106,7 @@ func OpenPostgres(ctx context.Context, dsn string, pool PoolConfig, opts ...Opti
if err != nil {
return nil, err
}
pgxPool, err := newPgxPool(ctx, dsn, pool)
pgxPool, err := OpenPgxPool(ctx, dsn, pool)
if err != nil {
closeGorm(gormDB)
return nil, err
@@ -135,7 +135,8 @@ func OpenPostgresWithoutPool(ctx context.Context, dsn string, opts ...Option) (*
return conn, nil
}
func newPgxPool(ctx context.Context, dsn string, cfg PoolConfig) (*pgxpool.Pool, error) {
// OpenPgxPool opens a pgx pool to dsn sized by cfg and verifies it with a ping.
func OpenPgxPool(ctx context.Context, dsn string, cfg PoolConfig) (*pgxpool.Pool, error) {
config, err := pgxpool.ParseConfig(dsn)
if err != nil {
return nil, fmt.Errorf("unable to parse database config: %w", err)
@@ -26,6 +26,9 @@ type Buffer[T any] struct {
mu sync.Mutex
waiting map[string][]chan result[T]
// inFlight holds the keys with a running fetch and whether their next flush
// is due once it ends. It is nil unless fetches are serialized.
inFlight map[string]bool
}
type result[T any] struct {
@@ -45,6 +48,15 @@ func New[T any](ctx context.Context, name string, interval time.Duration, fetch
}
}
// NewSerialized returns a Buffer like New that runs at most one fetch per key at
// a time. Requests arriving during a fetch form the next batch, which is fetched
// once both its window closed and the running fetch ended.
func NewSerialized[T any](ctx context.Context, name string, interval time.Duration, fetch FetchFunc[T]) *Buffer[T] {
b := New(ctx, name, interval, fetch)
b.inFlight = make(map[string]bool)
return b
}
// Get returns the value for key, sharing one fetch with the other callers of
// the current batch. The value is shared as is, so callers must treat it as
// read-only unless the fetch hands out copies.
@@ -71,8 +83,16 @@ func (b *Buffer[T]) Get(ctx context.Context, key string) (T, error) {
func (b *Buffer[T]) flush(key string) {
b.mu.Lock()
if _, running := b.inFlight[key]; running {
b.inFlight[key] = true
b.mu.Unlock()
return
}
waiting := b.waiting[key]
delete(b.waiting, key)
if b.inFlight != nil && len(waiting) > 0 {
b.inFlight[key] = false
}
b.mu.Unlock()
if len(waiting) == 0 {
@@ -86,6 +106,17 @@ func (b *Buffer[T]) flush(key string) {
for _, ch := range waiting {
ch <- result[T]{value: value, err: err}
}
if b.inFlight == nil {
return
}
b.mu.Lock()
due := b.inFlight[key]
delete(b.inFlight, key)
b.mu.Unlock()
if due {
go b.flush(key)
}
}
// Interval reads a buffer interval from envVar, falling back to def.
@@ -6,6 +6,7 @@ import (
"sync"
"sync/atomic"
"testing"
"testing/synctest"
"time"
"github.com/stretchr/testify/assert"
@@ -104,3 +105,73 @@ func TestBufferHonorsCallerContext(t *testing.T) {
_, err := buffer.Get(ctx, "account")
assert.ErrorIs(t, err, context.DeadlineExceeded)
}
func TestSerializedBufferQueuesRequestsDuringFetch(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
var fetches, running, maxRunning atomic.Int32
buffer := NewSerialized(context.Background(), "test", 10*time.Millisecond,
func(ctx context.Context, key string) (int32, error) {
n := running.Add(1)
defer running.Add(-1)
if n > maxRunning.Load() {
maxRunning.Store(n)
}
time.Sleep(time.Second)
return fetches.Add(1), nil
})
var wg sync.WaitGroup
get := func(want int32) {
wg.Add(1)
go func() {
defer wg.Done()
value, err := buffer.Get(context.Background(), "account")
assert.NoError(t, err)
assert.Equal(t, want, value)
}()
}
get(1)
time.Sleep(100 * time.Millisecond)
synctest.Wait()
require.Equal(t, int32(1), running.Load(), "first fetch should be running")
for range 5 {
get(2)
}
wg.Wait()
assert.Equal(t, int32(2), fetches.Load(), "requests queued during the fetch should share one fetch")
assert.Equal(t, int32(1), maxRunning.Load(), "fetches of a key should not overlap")
})
}
func TestUnserializedBufferOverlapsFetches(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
var running, maxRunning atomic.Int32
buffer := New(context.Background(), "test", 10*time.Millisecond,
func(ctx context.Context, key string) (string, error) {
n := running.Add(1)
defer running.Add(-1)
if n > maxRunning.Load() {
maxRunning.Store(n)
}
time.Sleep(time.Second)
return key, nil
})
var wg sync.WaitGroup
wg.Go(func() {
_, err := buffer.Get(context.Background(), "account")
assert.NoError(t, err)
})
time.Sleep(100 * time.Millisecond)
wg.Go(func() {
_, err := buffer.Get(context.Background(), "account")
assert.NoError(t, err)
})
wg.Wait()
assert.Equal(t, int32(2), maxRunning.Load())
})
}
+13 -4
View File
@@ -1825,10 +1825,10 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
return fmt.Errorf("reconcile IPv6 for group changes: %w", err)
}
change.ChangedPeerIDs = append(change.ChangedPeerIDs, ipv6Changed...)
}
if err = transaction.IncrementNetworkSerial(ctx, userAuth.AccountId); err != nil {
return fmt.Errorf("error incrementing network serial: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, userAuth.AccountId); err != nil {
return fmt.Errorf("error incrementing network serial: %w", err)
}
if snap, err = affectedpeers.Load(ctx, transaction, userAuth.AccountId, change); err != nil {
@@ -2844,6 +2844,10 @@ func (am *DefaultAccountManager) savePeerIPUpdate(ctx context.Context, transacti
return fmt.Errorf("save peer: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
eventMeta["old_ip"] = oldIP
eventMeta["ip"] = newIP.String()
am.StoreEvent(ctx, userID, peer.ID, accountID, activity.PeerIPUpdated, eventMeta)
@@ -2884,7 +2888,8 @@ func (am *DefaultAccountManager) UpdatePeerIPv6(ctx context.Context, accountID,
// updatePeerIPv6InTransaction validates and applies an IPv6 address change within a store transaction.
func (am *DefaultAccountManager) updatePeerIPv6InTransaction(ctx context.Context, transaction store.Store, accountID, peerID string, newIPv6 netip.Addr) (bool, error) {
network, err := transaction.GetAccountNetwork(ctx, store.LockingStrengthShare, accountID)
// Locked for update because the serial increment below writes the same row.
network, err := transaction.GetAccountNetwork(ctx, store.LockingStrengthUpdate, accountID)
if err != nil {
return false, fmt.Errorf("get network: %w", err)
}
@@ -2928,6 +2933,10 @@ func (am *DefaultAccountManager) updatePeerIPv6InTransaction(ctx context.Context
return false, fmt.Errorf("save peer: %w", err)
}
if err := transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return false, fmt.Errorf("increment network serial: %w", err)
}
return true, nil
}
+44 -1
View File
@@ -3690,7 +3690,7 @@ func createManagerWithNetworkMapStore(t testing.TB) (*DefaultAccountManager, *up
store, err := createStoreAt(t, dataDir)
require.NoError(t, err)
nmdataStore, err := networkmapdbfactory.NewNetworkMapDBStore(context.Background(), types.SqliteStoreEngine, dataDir, MockIntegratedValidator{}, newSettingsMockManager(t))
nmdataStore, err := networkmapdbfactory.NewNetworkMapDBStore(context.Background(), types.SqliteStoreEngine, dataDir, MockIntegratedValidator{}, newSettingsMockManager(t), nil, nil)
require.NoError(t, err)
sqliteStore, ok := nmdataStore.Store.(*networkmap_sqlite.SqliteStore)
require.True(t, ok, "network map store is %T, want *networkmap_sqlite.SqliteStore", nmdataStore.Store)
@@ -5110,3 +5110,46 @@ func TestDefaultAccountManager_ExpirationJobsStopForDeletedAccount(t *testing.T)
})
}
}
func networkSerial(t *testing.T, manager *DefaultAccountManager, accountID string) uint64 {
t.Helper()
network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, accountID)
require.NoError(t, err)
return network.Serial
}
func TestDefaultAccountManager_UpdatePeerIP_BumpsNetworkSerial(t *testing.T) {
manager, _, account, peer1, peer2, peer3 := setupNetworkMapTest(t)
newIP, err := types.AllocatePeerIP(netip.MustParsePrefix(account.Network.Net.String()), []netip.Addr{peer1.IP, peer2.IP, peer3.IP})
require.NoError(t, err)
before := networkSerial(t, manager, account.Id)
require.NoError(t, manager.UpdatePeerIP(context.Background(), account.Id, userID, peer1.ID, newIP))
assert.Greater(t, networkSerial(t, manager, account.Id), before)
}
func TestDefaultAccountManager_SyncUserJWTGroups_BumpsNetworkSerialWithoutPropagation(t *testing.T) {
manager, _, err := createManager(t)
require.NoError(t, err)
accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: "jwt-user", Domain: "test.domain"})
require.NoError(t, err)
acc, err := manager.Store.GetAccount(context.Background(), accountID)
require.NoError(t, err)
acc.Settings.JWTGroupsEnabled = true
acc.Settings.JWTGroupsClaimName = "idp-groups"
acc.Settings.GroupsPropagationEnabled = false
require.NoError(t, manager.Store.SaveAccount(context.Background(), acc))
before := networkSerial(t, manager, accountID)
err = manager.SyncUserJWTGroups(context.Background(), auth.UserAuth{
AccountId: accountID,
Domain: "test.domain",
UserId: "jwt-user",
Groups: []string{"group1"},
})
require.NoError(t, err)
assert.Greater(t, networkSerial(t, manager, accountID), before)
}
+37 -5
View File
@@ -103,7 +103,9 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK
return err
}
updated, err := am.Store.MarkPeerConnectedIfNewerSession(ctx, accountID, peer.ID, sessionStartedAt)
updated, err := am.writePeerStatus(ctx, accountID, peer, func(st store.Store) (bool, error) {
return st.MarkPeerConnectedIfNewerSession(ctx, accountID, peer.ID, sessionStartedAt)
})
if err != nil {
am.metrics.AccountManagerMetrics().CountPeerStatusUpdate(telemetry.PeerStatusConnect, telemetry.PeerStatusError)
return err
@@ -140,6 +142,28 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK
return nil
}
// writePeerStatus runs a peer status write. The connection state of an embedded
// proxy peer is part of the network map, so for those peers an applied write also
// increments the account network serial in the same transaction.
func (am *DefaultAccountManager) writePeerStatus(ctx context.Context, accountID string, peer *nbpeer.Peer, write func(store.Store) (bool, error)) (bool, error) {
if !peer.ProxyMeta.Embedded {
return write(am.Store)
}
var updated bool
err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
var err error
if updated, err = write(transaction); err != nil || !updated {
return err
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
return updated, err
}
// schedulePeerExpirations arms the account's login/inactivity expiration
// timers for an SSO peer that just connected.
func (am *DefaultAccountManager) schedulePeerExpirations(ctx context.Context, accountID string, peer *nbpeer.Peer) error {
@@ -179,7 +203,9 @@ func (am *DefaultAccountManager) MarkPeerDisconnected(ctx context.Context, peerP
return err
}
updated, err := am.Store.MarkPeerDisconnectedIfSameSession(ctx, accountID, peer.ID, sessionStartedAt)
updated, err := am.writePeerStatus(ctx, accountID, peer, func(st store.Store) (bool, error) {
return st.MarkPeerDisconnectedIfSameSession(ctx, accountID, peer.ID, sessionStartedAt)
})
if err != nil {
am.metrics.AccountManagerMetrics().CountPeerStatusUpdate(telemetry.PeerStatusDisconnect, telemetry.PeerStatusError)
return err
@@ -349,7 +375,7 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
return fmt.Errorf("increment network serial: %w", err)
}
return transaction.SavePeer(ctx, accountID, peer)
@@ -618,7 +644,7 @@ func (am *DefaultAccountManager) deletePeerInTransaction(ctx context.Context, ac
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
return fmt.Errorf("increment network serial: %w", err)
}
return nil
@@ -956,7 +982,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe
err = transaction.IncrementNetworkSerial(ctx, accountID)
if err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
return fmt.Errorf("increment network serial: %w", err)
}
if ephemeral {
@@ -1097,6 +1123,9 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy
if err = transaction.SavePeer(ctx, accountID, peer); err != nil {
return err
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
}
return nil
})
@@ -1272,6 +1301,9 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.Peer
if err = transaction.SavePeer(ctx, accountID, peer); err != nil {
return err
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
}
return nil
+68
View File
@@ -3255,3 +3255,71 @@ func TestAddPeer_ExhaustedIPAllocationFails(t *testing.T) {
require.Len(t, peers, 1)
assert.Equal(t, netip.MustParseAddr("100.64.0.1"), peers[0].IP)
}
func TestPeerWrites_BumpNetworkSerial(t *testing.T) {
manager, _, account, peer1, peer2, _ := setupNetworkMapTest(t)
ctx := context.Background()
t.Run("SyncPeer with changed meta bumps, unchanged does not", func(t *testing.T) {
current, err := manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, account.Id, peer1.ID)
require.NoError(t, err)
before := networkSerial(t, manager, account.Id)
_, _, _, _, err = manager.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peer1.Key, Meta: current.Meta}, account.Id)
require.NoError(t, err)
assert.Equal(t, before, networkSerial(t, manager, account.Id))
meta := current.Meta
meta.Hostname = "renamed-host"
_, _, _, _, err = manager.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peer1.Key, Meta: meta}, account.Id)
require.NoError(t, err)
assert.Greater(t, networkSerial(t, manager, account.Id), before)
})
t.Run("LoginPeer with changed ssh key bumps, unchanged does not", func(t *testing.T) {
current, err := manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, account.Id, peer1.ID)
require.NoError(t, err)
before := networkSerial(t, manager, account.Id)
_, _, _, _, err = manager.LoginPeer(ctx, types.PeerLogin{WireGuardPubKey: peer1.Key, SSHKey: current.SSHKey, Meta: current.Meta})
require.NoError(t, err)
assert.Equal(t, before, networkSerial(t, manager, account.Id))
_, _, _, _, err = manager.LoginPeer(ctx, types.PeerLogin{WireGuardPubKey: peer1.Key, SSHKey: "ssh-ed25519 AAAAnew", Meta: current.Meta})
require.NoError(t, err)
assert.Greater(t, networkSerial(t, manager, account.Id), before)
})
t.Run("MarkPeerConnected and MarkPeerDisconnected", func(t *testing.T) {
session := time.Now().UTC().UnixNano()
before := networkSerial(t, manager, account.Id)
require.NoError(t, manager.MarkPeerConnected(ctx, peer2.Key, account.Id, session, nil))
require.NoError(t, manager.MarkPeerDisconnected(ctx, peer2.Key, account.Id, session))
assert.Equal(t, before, networkSerial(t, manager, account.Id), "regular peers must not bump")
expired, err := manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, account.Id, peer2.ID)
require.NoError(t, err)
expired.Status.LoginExpired = true
require.NoError(t, manager.Store.SavePeer(ctx, account.Id, expired))
session++
require.NoError(t, manager.MarkPeerConnected(ctx, peer2.Key, account.Id, session, nil))
afterExpired := networkSerial(t, manager, account.Id)
assert.Equal(t, before, afterExpired, "login-expired reconnect writes no network map data")
embedded, err := manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, account.Id, peer2.ID)
require.NoError(t, err)
embedded.Status.LoginExpired = false
embedded.ProxyMeta.Embedded = true
require.NoError(t, manager.Store.SavePeer(ctx, account.Id, embedded))
require.NoError(t, manager.MarkPeerDisconnected(ctx, peer2.Key, account.Id, session))
afterDisconnect := networkSerial(t, manager, account.Id)
assert.Greater(t, afterDisconnect, afterExpired, "embedded proxy disconnect must bump")
session++
require.NoError(t, manager.MarkPeerConnected(ctx, peer2.Key, account.Id, session, nil))
assert.Greater(t, networkSerial(t, manager, account.Id), afterDisconnect, "embedded proxy connect must bump")
})
}
+10 -3
View File
@@ -670,6 +670,12 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
var updatedUsersInfo = make([]*types.UserInfo, 0, len(usersToSave))
if len(usersToSave) > 0 {
if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil {
return nil, fmt.Errorf("increment network serial: %w", err)
}
}
userInfos, err := am.GetUsersFromAccount(ctx, accountID, initiatorUserID)
if err != nil {
return nil, err
@@ -693,9 +699,6 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
return nil, err
}
} else if len(usersToSave) > 0 {
if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil {
return nil, fmt.Errorf("failed to increment network serial: %w", err)
}
go am.dispatchAffectedPeerIDs(ctx, accountID, affected.ids())
}
@@ -1425,6 +1428,10 @@ func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, accountI
return fmt.Errorf("failed to delete user: %s %w", targetUserInfo.ID, err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
})
if err != nil {
+22
View File
@@ -14,6 +14,7 @@ import (
"github.com/netbirdio/netbird/management/internals/controllers/network_map"
nbcache "github.com/netbirdio/netbird/management/server/cache"
nbcontext "github.com/netbirdio/netbird/management/server/context"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
"github.com/netbirdio/netbird/management/server/permissions/roles"
@@ -2352,3 +2353,24 @@ func TestUser_Operations_WithEmbeddedIDP(t *testing.T) {
t.Logf("Duplicate email error: %v", err)
})
}
func TestDefaultAccountManager_UserMutations_BumpNetworkSerial(t *testing.T) {
manager, _, account, _, _, _ := setupNetworkMapTest(t)
account, err := manager.Store.GetAccount(context.Background(), account.Id)
require.NoError(t, err)
account.Users["regular-user"] = types.NewRegularUser("regular-user", "", "")
require.NoError(t, manager.Store.SaveAccount(context.Background(), account))
ctx := nbcontext.WithRole(context.Background(), string(types.UserRoleOwner))
before := networkSerial(t, manager, account.Id)
_, err = manager.SaveUser(ctx, account.Id, userID, &types.User{Id: "regular-user", Role: types.UserRoleUser, Blocked: true})
require.NoError(t, err)
afterSave := networkSerial(t, manager, account.Id)
assert.Greater(t, afterSave, before, "SaveUser must bump the serial")
_, err = manager.deleteRegularUser(ctx, account.Id, userID, &types.UserInfo{ID: "regular-user"})
require.NoError(t, err)
assert.Greater(t, networkSerial(t, manager, account.Id), afterSave, "deleteRegularUser must bump the serial")
}