Improve profile getting and updating to fix interference

This commit is contained in:
Daniel 2021-12-03 10:22:17 +01:00
parent 8ce3bb1cfc
commit 4fb3bf0645
4 changed files with 118 additions and 107 deletions

View file

@ -7,8 +7,8 @@ import (
) )
const ( const (
activeProfileCleanerTickDuration = 1 * time.Minute activeProfileCleanerTickDuration = 5 * time.Minute
activeProfileCleanerThreshold = 5 * time.Minute activeProfileCleanerThreshold = 1 * time.Hour
) )
var ( var (

View file

@ -6,6 +6,7 @@ import (
"strings" "strings"
"github.com/safing/portbase/config" "github.com/safing/portbase/config"
"github.com/safing/portbase/log"
"github.com/safing/portbase/database" "github.com/safing/portbase/database"
"github.com/safing/portbase/database/query" "github.com/safing/portbase/database/query"
@ -41,24 +42,37 @@ func registerValidationDBHook() (err error) {
} }
func startProfileUpdateChecker() error { func startProfileUpdateChecker() error {
module.StartServiceWorker("update active profiles", 0, func(ctx context.Context) (err error) {
profilesSub, err := profileDB.Subscribe(query.New(profilesDBPath)) profilesSub, err := profileDB.Subscribe(query.New(profilesDBPath))
if err != nil { if err != nil {
return err return err
} }
defer func() {
err := profilesSub.Cancel()
if err != nil {
log.Warningf("profile: failed to cancel subscription for updating active profiles: %s", err)
}
}()
module.StartServiceWorker("update active profiles", 0, func(ctx context.Context) (err error) {
for { for {
select { select {
case r := <-profilesSub.Feed: case r := <-profilesSub.Feed:
// check if nil // Check if subscription was canceled.
if r == nil { if r == nil {
return errors.New("subscription canceled") return errors.New("subscription canceled")
} }
// mark as outdated // Don't mark profiles as outdated that are saved internally, as
// profiles only exist once in memory.
p, ok := r.(*Profile)
if ok && p.savedInternally {
return
}
// Mark profile as outdated.
markActiveProfileAsOutdated(strings.TrimPrefix(r.Key(), profilesDBPath)) markActiveProfileAsOutdated(strings.TrimPrefix(r.Key(), profilesDBPath))
case <-ctx.Done(): case <-ctx.Done():
return profilesSub.Cancel() return nil
} }
} }
}) })

View file

@ -2,16 +2,16 @@ package profile
import ( import (
"errors" "errors"
"sync"
"github.com/safing/portbase/database" "github.com/safing/portbase/database"
"github.com/safing/portbase/database/query" "github.com/safing/portbase/database/query"
"github.com/safing/portbase/database/record" "github.com/safing/portbase/database/record"
"github.com/safing/portbase/log" "github.com/safing/portbase/log"
"golang.org/x/sync/singleflight"
) )
var getProfileSingleInflight singleflight.Group var getProfileLock sync.Mutex
// GetProfile fetches a profile. This function ensures that the loaded profile // GetProfile fetches a profile. This function ensures that the loaded profile
// is shared among all callers. You must always supply both the scopedID and // is shared among all callers. You must always supply both the scopedID and
@ -22,13 +22,12 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit
profile *Profile, profile *Profile,
err error, err error,
) { ) {
// Select correct key for single in flight. // Globally lock getting a profile.
singleInflightKey := linkedPath // This does not happen too often, and it ensures we really have integrity
if singleInflightKey == "" { // and no race conditions.
singleInflightKey = makeScopedID(source, id) getProfileLock.Lock()
} defer getProfileLock.Unlock()
p, err, _ := getProfileSingleInflight.Do(singleInflightKey, func() (interface{}, error) {
var previousVersion *Profile var previousVersion *Profile
// Fetch profile depending on the available information. // Fetch profile depending on the available information.
@ -109,15 +108,6 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit
addActiveProfile(profile) addActiveProfile(profile)
return profile, nil return profile, nil
})
if err != nil {
return nil, err
}
if p == nil {
return nil, errors.New("profile getter returned nil")
}
return p.(*Profile), nil
} }
// getProfile fetches the profile for the given scoped ID. // getProfile fetches the profile for the given scoped ID.
@ -181,6 +171,9 @@ func prepProfile(r record.Record) (*Profile, error) {
log.Errorf("profiles: profile %s has (partly) invalid configuration: %s", profile.ID, err) log.Errorf("profiles: profile %s has (partly) invalid configuration: %s", profile.ID, err)
} }
// Set saved internally to suppress outdating profiles if saving internally.
profile.savedInternally = true
// return parsed profile // return parsed profile
return profile, nil return profile, nil
} }

View file

@ -140,6 +140,9 @@ type Profile struct { //nolint:maligned // not worth the effort
// Lifecycle Management // Lifecycle Management
outdated *abool.AtomicBool outdated *abool.AtomicBool
lastActive *int64 lastActive *int64
// savedInternally is set to true for profiles that are saved internally.
savedInternally bool
} }
func (profile *Profile) prepConfig() (err error) { func (profile *Profile) prepConfig() (err error) {
@ -231,6 +234,7 @@ func New(
LinkedPath: linkedPath, LinkedPath: linkedPath,
Created: time.Now().Unix(), Created: time.Now().Unix(),
Config: customConfig, Config: customConfig,
savedInternally: true,
} }
// Generate random ID if none is given. // Generate random ID if none is given.