From 1602793d18e08ff6ac3fbca927980471ad2a5aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=88=9A=E8=BE=85=E5=85=89?= <9330@9ji.com> Date: Fri, 17 Dec 2021 13:57:56 +0800 Subject: [PATCH 1/4] Fix concurrency write map panic --- config/config.go | 28 ++++++++++++++++++++-------- worker/worker.go | 4 ++-- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/config/config.go b/config/config.go index 76b03b425..e0f9eeb8f 100644 --- a/config/config.go +++ b/config/config.go @@ -16,6 +16,7 @@ package config import ( "github.com/BurntSushi/toml" + "sync" ) const ( @@ -126,14 +127,25 @@ type RecommendConfig struct { RefreshRecommendPeriod int `toml:"refresh_recommend_period"` FallbackRecommend []string `toml:"fallback_recommend"` ExploreRecommend map[string]float64 `toml:"explore_recommend"` - ItemNeighborType string `toml:"item_neighbor_type"` - UserNeighborType string `toml:"user_neighbor_type"` - EnableLatestRecommend bool `toml:"enable_latest_recommend"` - EnablePopularRecommend bool `toml:"enable_popular_recommend"` - EnableUserBasedRecommend bool `toml:"enable_user_based_recommend"` - EnableItemBasedRecommend bool `toml:"enable_item_based_recommend"` - EnableColRecommend bool `toml:"enable_collaborative_recommend"` - EnableClickThroughPrediction bool `toml:"enable_click_through_prediction"` + ExploreRecommendLock sync.Mutex + ItemNeighborType string `toml:"item_neighbor_type"` + UserNeighborType string `toml:"user_neighbor_type"` + EnableLatestRecommend bool `toml:"enable_latest_recommend"` + EnablePopularRecommend bool `toml:"enable_popular_recommend"` + EnableUserBasedRecommend bool `toml:"enable_user_based_recommend"` + EnableItemBasedRecommend bool `toml:"enable_item_based_recommend"` + EnableColRecommend bool `toml:"enable_collaborative_recommend"` + EnableClickThroughPrediction bool `toml:"enable_click_through_prediction"` +} + +func (config *RecommendConfig) GetExploreRecommend(key string) (value float64, exist bool) { + if config == nil { + return 0.0, false + } + config.ExploreRecommendLock.Lock() + defer config.ExploreRecommendLock.Unlock() + value, exist = config.ExploreRecommend[key] + return } // LoadDefaultIfNil loads default settings if config is nil. diff --git a/worker/worker.go b/worker/worker.go index d7a50d9f2..b63899d07 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -713,11 +713,11 @@ func (w *Worker) exploreRecommend(exploitRecommend []cache.Scored, excludeSet *s localExcludeSet := excludeSet.Copy() // create thresholds explorePopularThreshold := 0.0 - if threshold, exist := w.cfg.Recommend.ExploreRecommend["popular"]; exist { + if threshold, exist := w.cfg.Recommend.GetExploreRecommend("popular"); exist { explorePopularThreshold = threshold } exploreLatestThreshold := explorePopularThreshold - if threshold, exist := w.cfg.Recommend.ExploreRecommend["latest"]; exist { + if threshold, exist := w.cfg.Recommend.GetExploreRecommend("latest"); exist { exploreLatestThreshold += threshold } // load popular items From 17e861be3d7a8feaef84fc16c10da2a6f1cd184e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=88=9A=E8=BE=85=E5=85=89?= <9330@9ji.com> Date: Fri, 17 Dec 2021 14:26:53 +0800 Subject: [PATCH 2/4] Add unit test --- config/config_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index c7216ae80..4dcd2cf04 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -56,6 +56,14 @@ func TestLoadConfig(t *testing.T) { assert.Equal(t, 1, config.Recommend.RefreshRecommendPeriod) assert.Equal(t, []string{"item_based", "latest"}, config.Recommend.FallbackRecommend) assert.Equal(t, map[string]float64{"popular": 0.1, "latest": 0.2}, config.Recommend.ExploreRecommend) + value, exist := config.Recommend.GetExploreRecommend("popular") + assert.Equal(t, true, exist) + assert.Equal(t, 0.1, value) + value, exist = config.Recommend.GetExploreRecommend("latest") + assert.Equal(t, true, exist) + assert.Equal(t, 0.2, value) + value, exist = config.Recommend.GetExploreRecommend("unknown") + assert.Equal(t, false, exist) assert.Equal(t, "similar", config.Recommend.ItemNeighborType) assert.Equal(t, "similar", config.Recommend.UserNeighborType) assert.True(t, config.Recommend.EnableColRecommend) From 6606c1c1d672724fa705bebb185d48df7c96e6cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=88=9A=E8=BE=85=E5=85=89?= <9330@9ji.com> Date: Fri, 17 Dec 2021 14:47:59 +0800 Subject: [PATCH 3/4] Refine code acorrding to review comments --- config/config.go | 24 +++++++++++++----------- config/config_test.go | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/config/config.go b/config/config.go index e0f9eeb8f..f867c44df 100644 --- a/config/config.go +++ b/config/config.go @@ -127,23 +127,23 @@ type RecommendConfig struct { RefreshRecommendPeriod int `toml:"refresh_recommend_period"` FallbackRecommend []string `toml:"fallback_recommend"` ExploreRecommend map[string]float64 `toml:"explore_recommend"` - ExploreRecommendLock sync.Mutex - ItemNeighborType string `toml:"item_neighbor_type"` - UserNeighborType string `toml:"user_neighbor_type"` - EnableLatestRecommend bool `toml:"enable_latest_recommend"` - EnablePopularRecommend bool `toml:"enable_popular_recommend"` - EnableUserBasedRecommend bool `toml:"enable_user_based_recommend"` - EnableItemBasedRecommend bool `toml:"enable_item_based_recommend"` - EnableColRecommend bool `toml:"enable_collaborative_recommend"` - EnableClickThroughPrediction bool `toml:"enable_click_through_prediction"` + ItemNeighborType string `toml:"item_neighbor_type"` + UserNeighborType string `toml:"user_neighbor_type"` + EnableLatestRecommend bool `toml:"enable_latest_recommend"` + EnablePopularRecommend bool `toml:"enable_popular_recommend"` + EnableUserBasedRecommend bool `toml:"enable_user_based_recommend"` + EnableItemBasedRecommend bool `toml:"enable_item_based_recommend"` + EnableColRecommend bool `toml:"enable_collaborative_recommend"` + EnableClickThroughPrediction bool `toml:"enable_click_through_prediction"` + exploreRecommendLock *sync.Mutex } func (config *RecommendConfig) GetExploreRecommend(key string) (value float64, exist bool) { if config == nil { return 0.0, false } - config.ExploreRecommendLock.Lock() - defer config.ExploreRecommendLock.Unlock() + config.exploreRecommendLock.Lock() + defer config.exploreRecommendLock.Unlock() value, exist = config.ExploreRecommend[key] return } @@ -167,6 +167,7 @@ func (config *RecommendConfig) LoadDefaultIfNil() *RecommendConfig { EnableItemBasedRecommend: false, EnableColRecommend: true, EnableClickThroughPrediction: false, + exploreRecommendLock: &sync.Mutex{}, } } return config @@ -294,6 +295,7 @@ func (config *Config) FillDefault(meta toml.MetaData) { if !meta.IsDefined("recommend", "enable_click_through_prediction") { config.Recommend.EnableClickThroughPrediction = defaultRecommendConfig.EnableClickThroughPrediction } + config.Recommend.exploreRecommendLock = &sync.Mutex{} } // LoadConfig loads configuration from toml file. diff --git a/config/config_test.go b/config/config_test.go index 4dcd2cf04..1c1583868 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -62,7 +62,7 @@ func TestLoadConfig(t *testing.T) { value, exist = config.Recommend.GetExploreRecommend("latest") assert.Equal(t, true, exist) assert.Equal(t, 0.2, value) - value, exist = config.Recommend.GetExploreRecommend("unknown") + _, exist = config.Recommend.GetExploreRecommend("unknown") assert.Equal(t, false, exist) assert.Equal(t, "similar", config.Recommend.ItemNeighborType) assert.Equal(t, "similar", config.Recommend.UserNeighborType) From 31a70181a284c3b29a88faa128ca99da3b094314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=88=9A=E8=BE=85=E5=85=89?= <9330@9ji.com> Date: Fri, 17 Dec 2021 15:47:36 +0800 Subject: [PATCH 4/4] Refine code acorrding to review comments --- config/config.go | 6 +++--- config/config_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/config.go b/config/config.go index f867c44df..6ed4c8957 100644 --- a/config/config.go +++ b/config/config.go @@ -135,7 +135,7 @@ type RecommendConfig struct { EnableItemBasedRecommend bool `toml:"enable_item_based_recommend"` EnableColRecommend bool `toml:"enable_collaborative_recommend"` EnableClickThroughPrediction bool `toml:"enable_click_through_prediction"` - exploreRecommendLock *sync.Mutex + exploreRecommendLock sync.Mutex } func (config *RecommendConfig) GetExploreRecommend(key string) (value float64, exist bool) { @@ -167,7 +167,7 @@ func (config *RecommendConfig) LoadDefaultIfNil() *RecommendConfig { EnableItemBasedRecommend: false, EnableColRecommend: true, EnableClickThroughPrediction: false, - exploreRecommendLock: &sync.Mutex{}, + exploreRecommendLock: sync.Mutex{}, } } return config @@ -295,7 +295,7 @@ func (config *Config) FillDefault(meta toml.MetaData) { if !meta.IsDefined("recommend", "enable_click_through_prediction") { config.Recommend.EnableClickThroughPrediction = defaultRecommendConfig.EnableClickThroughPrediction } - config.Recommend.exploreRecommendLock = &sync.Mutex{} + config.Recommend.exploreRecommendLock = sync.Mutex{} } // LoadConfig loads configuration from toml file. diff --git a/config/config_test.go b/config/config_test.go index 1c1583868..0f5147028 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -75,9 +75,9 @@ func TestLoadConfig(t *testing.T) { } func TestConfig_FillDefault(t *testing.T) { - var config Config + var config *Config meta, err := toml.Decode("", &config) assert.NoError(t, err) config.FillDefault(meta) - assert.Equal(t, *(*Config)(nil).LoadDefaultIfNil(), config) + assert.Equal(t, (*Config)(nil).LoadDefaultIfNil(), config) }