Ревью кода: высоконагруженный in-memory кэш на sync.Mutex — найдите ошибки конкуренции
Проведите ревью in-memory кэша, рассчитанного на высокую нагрузку в проде, с соотношением чтение/запись примерно 80/20. GetOrCreate должна вернуть существующее значение по ключу либо сохранить и вернуть новое; Get должна вернуть значение из кэша. Найдите все ошибки конкуренции и корректности и скажите, как их исправить.
var cache = make(map[string]string)
// GetOrCreate проверяет существование ключа; если его нет — создаёт значение.
func GetOrCreate(key, value string) string {
var m sync.Mutex
m.Lock()
value = cache[key]
m.Unlock()
if value != "" {
return value
}
m.Lock()
cache[key] = value
m.Unlock()
return value
}
func Get(key string) string {
var m sync.Mutex
m.Lock()
v := cache[key]
m.Unlock()
return v
}Найдите и исправьте ошибки.
sync.Mutex — локальная переменная, поэтому каждый вызов блокирует свою копию, а общий cache ничем не защищён — конкурентные вызовы гоняются и падают с fatal error: concurrent map writes. Ещё value = cache[key] перезаписывает аргумент, поэтому путь создания сохраняет "", а снятие блокировки между чтением и записью делает get-or-create неатомарным. Исправление: один общий замок — RWMutex, раз чтений больше, — удерживаемый на всём check-then-set, и не затирать value.
- ✗Не заметить, что мьютекс — локальная переменная, поэтому ничего не синхронизирует между горутинами
- ✗Упустить, что
value = cache[key]перезаписывает аргумент, и путь создания сохраняет пустую строку - ✗Считать get-or-create атомарным, хотя чтение и запись под разными блокировками
- →Почему взятие
RLockна чтение и затемLockна запись всё равно позволяет двум вызывающим создать значение? - →Как
sync.Mapили группаsingleflightизменили бы этот дизайн?
Найдите ошибки
var cache = make(map[string]string)
func GetOrCreate(key, value string) string {
var m sync.Mutex // ❌ локальный мьютекс — у каждого вызова свой
m.Lock()
value = cache[key] // ❌ затирает аргумент value
m.Unlock()
if value != "" {
return value
}
m.Lock()
cache[key] = value // ❌ value уже "" — сохраняем пустую строку
m.Unlock() // ❌ блокировка снята между чтением и записью
return value
}Почему это сломано
- Мьютекс локален.
var m sync.Mutexобъявлен внутри функции, поэтому каждый вызов берёт свой мьютекс. Общийcacheне защищён ничем — конкурентные вызовы пишут в map одновременно и аварийно падают:
fatal error: concurrent map writes- Аргумент затирается.
value = cache[key]переписывает переданное значение результатом поиска. Если ключа нет,valueстановится"", и ниже в кэш сохраняется пустая строка, а не то, что просил вызывающий.
- Get-or-create неатомарен. Между чтением и записью блокировка снимается, поэтому два вызова могут оба промахнуться и оба создать значение — «проверка» и «установка» не одно целое.
✅ Исправление — один общий RWMutex (чтений 80%), удерживаемый на всём check-then-set, с , ok чтобы отличить отсутствие от пустого значения:
var (
mu sync.RWMutex
cache = make(map[string]string)
)
func GetOrCreate(key, value string) string {
mu.Lock()
defer mu.Unlock()
if existing, ok := cache[key]; ok {
return existing
}
cache[key] = value
return value
}
func Get(key string) string {
mu.RLock()
defer mu.RUnlock()
return cache[key]
}
