From f2878441f67163459d0e0759f250b0e71bff3abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=B2=81=E5=BB=BA=E5=BC=BA?= Date: Fri, 30 Oct 2026 13:54:21 +0800 Subject: [PATCH 1/2] feat: make feature store same-project registration deterministic and warn on conflicts --- persist/fs/featurestore.go | 44 +++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/persist/fs/featurestore.go b/persist/fs/featurestore.go index 080ad9ff..40d9d84c 100644 --- a/persist/fs/featurestore.go +++ b/persist/fs/featurestore.go @@ -2,6 +2,8 @@ package fs import ( "fmt" + "reflect" + "sort" "sync" "github.com/alibaba/pairec/v2/log" @@ -60,7 +62,32 @@ func Load(config *recconf.RecommendConfig) { fsLoadMu.Lock() defer fsLoadMu.Unlock() - for name, conf := range config.FeatureStoreConfs { + // The feature store SDK caches connection pools (hologres/featuredb/...) in + // process-level registries keyed by server-side datasource name with + // first-registration-wins semantics. Iterating the config map in random + // order would make the winning config of a same-project registration + // non-deterministic across restarts, so configs are loaded in sorted key + // order to make the selection stable and reproducible. + names := make([]string, 0, len(config.FeatureStoreConfs)) + for name := range config.FeatureStoreConfs { + names = append(names, name) + } + sort.Strings(names) + + // firstConfs records, per project, the config that registers the shared + // connection pools first (and therefore actually takes effect). + firstConfs := make(map[string]string, len(names)) + for _, name := range names { + conf := config.FeatureStoreConfs[name] + if firstName, ok := firstConfs[conf.ProjectName]; ok { + if diffFields := diffFeatureStoreConfs(config.FeatureStoreConfs[firstName], conf); len(diffFields) > 0 { + log.Warning(fmt.Sprintf("module=FeatureStore\tname=%q\tmsg=project %s is registered by multiple FeatureStoreConfs, the config of %q is used", + name, conf.ProjectName, firstName)) + } + } else { + firstConfs[conf.ProjectName] = name + } + fsInstancesMu.RLock() fs, ok := fsInstances[name] fsInstancesMu.RUnlock() @@ -121,3 +148,18 @@ func Load(config *recconf.RecommendConfig) { fsInstancesMu.Unlock() } } + +// diffFeatureStoreConfs returns the field names that differ between two +// FeatureStoreConfig entries of the same project. Values are never included +// because the configs may contain credentials. +func diffFeatureStoreConfs(first, other recconf.FeatureStoreConfig) []string { + var diffFields []string + firstValue := reflect.ValueOf(first) + otherValue := reflect.ValueOf(other) + for i := 0; i < firstValue.NumField(); i++ { + if !reflect.DeepEqual(firstValue.Field(i).Interface(), otherValue.Field(i).Interface()) { + diffFields = append(diffFields, firstValue.Type().Field(i).Name) + } + } + return diffFields +} From cff0a9e4d585615870c440f0b521a5297ee08b05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=B2=81=E5=BB=BA=E5=BC=BA?= Date: Thu, 5 Nov 2026 10:34:08 +0800 Subject: [PATCH 2/2] refactor: revert FS registration to map order, keep same-project conflict warning Drop the sorted-key deterministic registration: Load() iterates config.FeatureStoreConfs in its natural map order again. Keep the same-project duplicate detection (diffFeatureStoreConfs) and log.Warning, reworded to state the effective config is non-deterministic across restarts and advise converging the configs, since registration order is no longer forced. Removes the now-unused sort import. --- persist/fs/featurestore.go | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/persist/fs/featurestore.go b/persist/fs/featurestore.go index 40d9d84c..15cfb7c8 100644 --- a/persist/fs/featurestore.go +++ b/persist/fs/featurestore.go @@ -3,7 +3,6 @@ package fs import ( "fmt" "reflect" - "sort" "sync" "github.com/alibaba/pairec/v2/log" @@ -64,28 +63,19 @@ func Load(config *recconf.RecommendConfig) { // The feature store SDK caches connection pools (hologres/featuredb/...) in // process-level registries keyed by server-side datasource name with - // first-registration-wins semantics. Iterating the config map in random - // order would make the winning config of a same-project registration - // non-deterministic across restarts, so configs are loaded in sorted key - // order to make the selection stable and reproducible. - names := make([]string, 0, len(config.FeatureStoreConfs)) - for name := range config.FeatureStoreConfs { - names = append(names, name) - } - sort.Strings(names) - - // firstConfs records, per project, the config that registers the shared - // connection pools first (and therefore actually takes effect). - firstConfs := make(map[string]string, len(names)) - for _, name := range names { - conf := config.FeatureStoreConfs[name] - if firstName, ok := firstConfs[conf.ProjectName]; ok { + // first-registration-wins semantics. This loop iterates the config map in its + // natural (random) order, so when several FeatureStoreConfs share one project + // but differ, which config takes effect is non-deterministic across restarts. + // Warn on that conflict so it can be converged instead of silently picking one. + seenProjects := make(map[string]string, len(config.FeatureStoreConfs)) + for name, conf := range config.FeatureStoreConfs { + if firstName, ok := seenProjects[conf.ProjectName]; ok { if diffFields := diffFeatureStoreConfs(config.FeatureStoreConfs[firstName], conf); len(diffFields) > 0 { - log.Warning(fmt.Sprintf("module=FeatureStore\tname=%q\tmsg=project %s is registered by multiple FeatureStoreConfs, the config of %q is used", - name, conf.ProjectName, firstName)) + log.Warning(fmt.Sprintf("module=FeatureStore\tname=%q\tmsg=project %s is registered by multiple FeatureStoreConfs (%q and %q) with differing fields, the effective config is non-deterministic, please converge them", + name, conf.ProjectName, firstName, name)) } } else { - firstConfs[conf.ProjectName] = name + seenProjects[conf.ProjectName] = name } fsInstancesMu.RLock()