fix: improve cache synchronization mechanism #2312
Conversation
- Fix InitChannelCache to use abilities table for accurate cache building - Add cache invalidation functions (InvalidateChannelCache, InvalidateGroupModelCache, InvalidateUserCache) - Update UpdateChannelStatusById to immediately invalidate related caches - Update Channel.Update() to invalidate caches when configuration changes - Add comprehensive cache tests - Ensure real-time cache sync for channel status changes This fixes the issue where channel status changes (enable/disable) required service restart to take effect.
There was a problem hiding this comment.
Pull Request Overview
This PR improves the cache synchronization mechanism by fixing several cache consistency issues. The changes ensure that cache state remains accurate when channel configurations are modified and provide more granular cache invalidation capabilities.
- Fixed InitChannelCache() to build cache based on abilities table for accuracy
- Added cache invalidation functions for immediate synchronization after updates
- Enhanced channel status and configuration update methods with proper cache invalidation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| model/channel.go | Enhanced Channel.Update() and UpdateChannelStatusById() with cache invalidation logic |
| model/cache.go | Refactored cache initialization and added invalidation functions |
| model/cache_test.go | Added comprehensive test suite for cache functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Missing conditional check for UpdateAbilities() error. The error variable err is assigned but there's no if err != nil check before line 154.
Copilot uses AI. Check for mistakes.
| newChannelId2channel := make(map[int]*Channel) | ||
| var channels []*Channel | ||
| DB.Where("status = ?", ChannelStatusEnabled).Find(&channels) | ||
| for _, channel := range channels { | ||
| newChannelId2channel[channel.Id] = channel |
There was a problem hiding this comment.
The variable newChannelId2channel is created but never used in the updated InitChannelCache() function. This appears to be leftover code from the refactoring.
| newChannelId2channel := make(map[int]*Channel) | |
| var channels []*Channel | |
| DB.Where("status = ?", ChannelStatusEnabled).Find(&channels) | |
| for _, channel := range channels { | |
| newChannelId2channel[channel.Id] = channel | |
| var channels []*Channel | |
| DB.Where("status = ?", ChannelStatusEnabled).Find(&channels) | |
| for _, channel := range channels { | |
| // no longer populating newChannelId2channel |
Copilot uses AI. Check for mistakes.
| channels := []*Channel{ | ||
| { | ||
| Id: 1, | ||
| Status: ChannelStatusEnabled, | ||
| Group: "default", | ||
| Models: "gpt-3.5-turbo", | ||
| Priority: &[]int64{0}[0], |
There was a problem hiding this comment.
This inline slice creation and dereferencing pattern &[]int64{0}[0] is unnecessarily complex. Consider using a helper variable: priority := int64(0); Priority: &priority,
| channels := []*Channel{ | |
| { | |
| Id: 1, | |
| Status: ChannelStatusEnabled, | |
| Group: "default", | |
| Models: "gpt-3.5-turbo", | |
| Priority: &[]int64{0}[0], | |
| priority := int64(0) | |
| channels := []*Channel{ | |
| { | |
| Id: 1, | |
| Status: ChannelStatusEnabled, | |
| Group: "default", | |
| Models: "gpt-3.5-turbo", | |
| Priority: &priority, |
Copilot uses AI. Check for mistakes.
功能改进: