Продолжение прошлой статьи про ошибки на Go-собесах. В тот раз — про лайв-кодинг. Теперь — про code review: когда дают готовый сервис на ~150 строк и говорят “найди что не так, у тебя 30 минут”.
Разберём по косточкам реальный код с собеседования — микросервис трекинга рекламных кликов. Багов набралось 21, поэтому делю на две части. Первая — про самое страшное: concurrency, гонки, утечки памяти и горутин. Это то, что роняет сервис в проде. Часть 2 — про API design, ошибки и graceful shutdown — выйдет следом. Актуально для Go 1.26.
Из 21 бага на собесе я нашёл 18. Три самых тонких пропустил — потом, дома, перечитал спокойно и выписал. В этой части про concurrency пропустил один — TOCTOU race в дедупликации. Остальные семь — поймал. Расскажу как искал и какими красными флагами зацепился.
Контекст: что за код?
Дали типовой ad-tech сервис: HTTP endpoint принимает клик пользователя по баннеру, генерирует tracking link, кладёт в in-memory storage, паблишит в очередь. Дедупликация по (user_uuid, campaign_uuid) за последние 5 секунд (чтобы один пользователь не накручивал клики).
Stack: Gin, sync.RWMutex, чистый Go без БД. Вроде бы простенько. Вот основные структуры:
type CampaignClick struct { ClickUUID string UserUUID string CampaignUUID string Country string TrackingLink string CreatedAt time.Time}type InMemoryClickRepository struct { mu sync.RWMutex clicks []*CampaignClick}type Publisher struct { topic string out chan *CampaignClick}
Интервьюер мне такой: “Тут есть несколько проблем. Найди как можно больше за 30 минут”. Я думаю — ну сейчас покажу класс. И, к моему собственному удивлению, действительно показал: 18 из 21. Дальше — как именно я их искал, и какие три всё-таки пропустил.
1. “Mutex есть, но он не работает”
Открываю код, вижу:
type InMemoryClickRepository struct { mu sync.RWMutex clicks []*CampaignClick}func (r *InMemoryClickRepository) Save(c *CampaignClick) error { r.clicks = append(r.clicks, c) return nil}
Первое, на что я всегда смотрю в репозитории, — все ли write-методы берут Lock. Проверяю Save — там пусто. Никакого r.mu.Lock(). Mutex объявлен в структуре, в GetRecent используется, а в Save забыли. Классика жанра.
Озвучиваю интервьюеру: “Тут data race — Save пишет в slice без Lock, при этом GetRecent читает под RLock. Если одна горутина делает append (а он может перевыделить underlying array), а вторая в этот момент читает — получаем race на underlying array”. Интервьюер кивает, ставит галочку.
Дополнительно: даже без перевыделения массива у нас простая non-atomic запись в slice header (3 машинных слова: pointer, len, cap) одновременно с чтением. На amd64 это видно go test -race сразу, а в production — будет всплывать раз в сутки в виде “internal error” или невидимо рушить инварианты.
Правильно:
func (r *InMemoryClickRepository) Save(c *CampaignClick) error { r.mu.Lock() defer r.mu.Unlock() r.clicks = append(r.clicks, c) return nil}
Мораль: Если видишь sync.Mutex в структуре — проверь ВСЕ методы, где меняется состояние. Один забытый Lock() обнуляет всю защиту. И да, всегда гоняй go test -race в CI. Это не опционально.
2. Goroutine Leak — самая красивая засада
Дальше натыкаюсь на этот шедевр:
type Publisher struct { topic string out chan *CampaignClick}func NewPublisher(topic string, buffer int) *Publisher { return &Publisher{ topic: topic, out: make(chan *CampaignClick, buffer), }}func (p *Publisher) Publish(c *CampaignClick) { select { case p.out <- c: default: }}
И потом в handler’е:
go c.publisher.Publish(click)
Тут у меня сразу сработала привычка: вижу chan в структуре — ищу <- где-то ещё в коде, чтобы понять, кто consumer. Грепаю publisher.out по файлу — ни одного receive’а. Нет ни горутины-consumer’а, ни даже метода Run()/Start().
Говорю интервьюеру: “Канал есть, producer есть, а consumer’а нет. Сообщения накопятся до буфера на 100 элементов, дальше default сработает — и мы будем молча терять клики. Метрики на это нет, ошибки нет, в логах пусто. В ad-tech это деньги, которые улетают в /dev/null”. Это второй пойманный баг.
То есть: канал на 100 элементов, заполняется до упора, потом default срабатывает и сообщения просто молча теряются. Сервис трекает клики (это деньги в ad-tech!), но половину из них выбрасывает в /dev/null. И никто не узнает, потому что ошибки нет, метрики нет, ничего нет.
Плюс к этому — go c.publisher.Publish(click) запускает горутину, которая кладёт элемент в буфер. Это в 99% случаев лишнее: Publish неблокирующий, его можно вызвать синхронно. Лишняя горутина = лишний overhead на scheduler. А если к моменту запуска горутины процесс получит SIGTERM (см. graceful shutdown во второй части) — горутина просто умрёт на полпути.
Правильно:
type Publisher struct { topic string out chan *CampaignClick done chan struct{}}func NewPublisher(topic string, buffer int) *Publisher { p := &Publisher{ topic: topic, out: make(chan *CampaignClick, buffer), done: make(chan struct{}), } go p.run() // запускаем consumer! return p}func (p *Publisher) run() { for c := range p.out { if err := sendToBroker(p.topic, c); err != nil { log.Printf("publish failed: %v", err) // metric: dropped_messages++ } } close(p.done)}func (p *Publisher) Publish(c *CampaignClick) error { select { case p.out <- c: return nil default: return errors.New("publisher buffer full") // хотя бы знаем! }}func (p *Publisher) Close(ctx context.Context) error { close(p.out) select { case <-p.done: return nil case <-ctx.Done(): return ctx.Err() }}
В Go 1.26 завезли экспериментальный профайлер GOEXPERIMENT=goroutineleakprofile — он ловит ровно такие сценарии. Запускайте на staging и смотрите, что течёт. Собес проходил еще на Go 1.24
Мораль: Каждый chan должен иметь и producer’а, и consumer’а. Если канал создан, но никто из него не читает — это либо memory leak (если канал небуфкризированный — горутины висят на send), либо тихая потеря данных (если буферизованный + default). И никогда, НИКОГДА не глотай ошибку публикации без метрики.
3. Publisher по значению — копирование канала
Смотрю в main:
publisher := NewPublisher("clicks", 100) // возвращает *Publisherctrl := NewController(repo, *publisher) // разыменовываем!
И сигнатура:
func NewController(repo *InMemoryClickRepository, publisher Publisher) *ClickController { return &ClickController{ repo: repo, publisher: publisher, // копия! }}
Publisher передаётся по значению, без указателя. Цепляюсь сразу: внутри Publisher — chan *CampaignClick, при копировании мы копируем header канала. Сейчас работает, но это работает случайно — стоит добавить любое мутабельное поле (счётчик, mutex), и копия начнёт жить своей жизнью.
Каналы в Go — это reference type, при копировании структуры мы копируем сам header канала, а не содержимое. То есть формально оба Publisher’а указывают на один канал. Но это работает случайно. Стоит добавить в Publisher любое мутабельное поле (счётчик, mutex, slice статистики) — и копия начинает жить своей жизнью. Изменения в одной копии не видны в другой. Hello debugging from hell.
Плюс: если кто-то добавит метод, который меняет состояние:
func (p *Publisher) IncrementSent() { p.sent++ // в КОПИИ структуры!}
— это просто не будет работать. И ты будешь два дня гадать почему.
Правильно — всегда передавать по указателю, если в структуре есть mutex, channel, slice или map:
func NewController(repo *InMemoryClickRepository, publisher *Publisher) *ClickController { return &ClickController{ repo: repo, publisher: publisher, }}// в main:ctrl := NewController(repo, publisher) // без звёздочки
Ещё лучше — принимать interface, а не конкретный тип:
type ClickPublisher interface { Publish(c *CampaignClick) error}func NewController(repo *InMemoryClickRepository, publisher ClickPublisher) *ClickController { // тестировать в 10 раз проще, моки делаются за минуту}
Мораль: Структуры с каналами/мьютексами/слайсами — всегда по указателю. go vet поймает копирование структуры с sync.Mutex, но не с каналом — это блайнд-спот линтера. Если код удобно мокать — он удобно тестируется. Прячь конкретные типы за интерфейсами.
4. Memory Leak в “in-memory storage”
func (r *InMemoryClickRepository) PurgeOld() { r.mu.Lock() defer r.mu.Unlock() cutoff := time.Now().Add(-5 * time.Minute) for i := len(r.clicks) - 1; i >= 0; i-- { if r.clicks[i].CreatedAt.Before(cutoff) { r.clicks = append(r.clicks[:i], r.clicks[i+1:]...) } }}
Метод есть. Но фишка: никто его не вызывает. Перегрепал весь main.go — PurgeOld упомянут только в самом определении.
Что в итоге: при 1000 RPS за час сервис накопит 3.6 млн структур, за день — 86 млн. Через неделю pod падает с OOM. И это в production, в субботу ночью, когда никто не видит.
Правильно — запускать в отдельной горутине через ticker:
func (r *InMemoryClickRepository) StartPurger(ctx context.Context, interval time.Duration) { go func() { ticker := time.NewTicker(interval) defer ticker.Stop() for { select { case <-ctx.Done(): return case <-ticker.C: r.PurgeOld() } } }()}
И вызвать из main:
ctx, cancel := context.WithCancel(context.Background())defer cancel()repo.StartPurger(ctx, 1*time.Minute)
Мораль: Если ты написал cleanup-метод — ОБЯЗАТЕЛЬНО запусти его. И поставь алерт на размер in-memory структур (через expvar или Prometheus). “In-memory” не значит “infinity-memory”.
5. PurgeOld: держим Lock на O(N²) операции
К тому же PurgeOld написан неэффективно. Смотрите внимательнее:
for i := len(r.clicks) - 1; i >= 0; i-- { if r.clicks[i].CreatedAt.Before(cutoff) { r.clicks = append(r.clicks[:i], r.clicks[i+1:]...) }}
Каждый append(r.clicks[:i], r.clicks[i+1:]...) для удаления одного элемента — это O(N) копирование оставшегося хвоста. Если протухло K элементов из N — получаем O(K·N). На 1М кликов с 100K протухших — это 100 миллиардов операций. Под Lock.
И всё это время никто не может ни читать, ни писать. Сервис фактически зависает на длительности cleanup. RPS просаживается, latency взлетает, k8s начинает рестартить под из-за readiness-проб.
Правильно — две стадии: найти точку среза, сделать один срез:
func (r *InMemoryClickRepository) PurgeOld(maxAge time.Duration) { cutoff := time.Now().Add(-maxAge) r.mu.Lock() defer r.mu.Unlock() // clicks отсортированы по времени вставки (Save под Lock) // ищем первый "свежий" элемент через бинарный поиск idx := sort.Search(len(r.clicks), func(i int) bool { return r.clicks[i].CreatedAt.After(cutoff) }) if idx == 0 { return // нечего удалять } // один срез, O(N - idx) копирование r.clicks = append(r.clicks[:0], r.clicks[idx:]...)}
Бонус — для совсем большой нагрузки можно делать swap-and-truncate, чтобы освободить старый underlying array:
newClicks := make([]*CampaignClick, len(r.clicks)-idx)copy(newClicks, r.clicks[idx:])r.clicks = newClicks // GC заберёт старый массив
Мораль: Lock — это бутылочное горлышко. Внутри Lock держи только работу с защищённым стейтом, и делай её за O(log N) или O(N), не за O(N²). Если cleanup тяжёлый — выноси работу за Lock, под Lock делай только финальный swap.
6. TOCTOU race в дедупликации — тот самый, что я пропустил на интервью
А вот этот баг я проворонил. Прочитал handler, посмотрел: “ага, проверка в кеше, если нет — создаём”, всё логично. Тиканье таймера в голове — иду дальше. И только дома, перечитывая, понял, чего не увидел. Смотрите на handler:
if _, cached, ok := c.repo.GetRecent(req.UserUUID, req.CampaignUUID, c.cacheTTL); ok { ctx.JSON(http.StatusOK, ClickResponse{...cached...}) return}click := &CampaignClick{...}c.repo.Save(click)
Классический TOCTOU (Time Of Check vs Time Of Use). Между GetRecent и Save другая горутина может успеть сохранить дубликат. Получаем два клика для одного (user, campaign) за окно дедупликации. В ad-tech это значит — рекламодатель платит дважды за одного юзера.
Почему пропустил: глаз зацепился за RWMutex внутри репозитория — мозг успокоился (“там же synchronization”). А внутри handler’а между двумя вызовами этого репозитория Lock’а нет — и быть не может, потому что репозиторий не знает про composite-операции “сначала проверь, потом сохрани”.
Сценарий:
-
Goroutine A:
GetRecent(user1, camp1)→ не нашёл -
Goroutine B:
GetRecent(user1, camp1)→ не нашёл -
Goroutine A:
Save(click1) -
Goroutine B:
Save(click2)← дубликат!
Правильно — атомарная операция GetOrCreate на стороне репозитория:
func (r *InMemoryClickRepository) GetOrCreate( userUUID, campaignUUID string, maxAge time.Duration, factory func() *CampaignClick,) (click *CampaignClick, created bool) { r.mu.Lock() defer r.mu.Unlock() cutoff := time.Now().Add(-maxAge) for i := len(r.clicks) - 1; i >= 0; i-- { cc := r.clicks[i] if cc.UserUUID == userUUID && cc.CampaignUUID == campaignUUID && cc.CreatedAt.After(cutoff) { return cc, false } } newClick := factory() r.clicks = append(r.clicks, newClick) return newClick, true}
Один Lock на check + create. Дубликаты исключены by design.
Мораль: Если у тебя есть последовательность “проверь — потом сделай” — это всегда race condition между двумя горутинами. Объединяй в одну атомарную операцию, либо используй distributed lock (если несколько подов).
7. O(N) поиск — пока 100 кликов, потом 100 миллионов
Смотрю на GetRecent:
for i := len(r.clicks) - 1; i >= 0; i-- { cc := r.clicks[i] if cc.UserUUID == userUUID && cc.CampaignUUID == campaignUUID && cc.CreatedAt.After(cutoff) { return cc, true }}
Каждый запрос — линейный проход по всему слайсу. На 1000 кликов — норм. На 100M (см. п.4 про утечку) — каждый запрос становится секундой. Сервис ложится. Под Lock держим тоже долго.
Правильно — map по составному ключу:
type clickKey struct { userUUID, campaignUUID string}type InMemoryClickRepository struct { mu sync.RWMutex byKey map[clickKey]*CampaignClick // O(1) lookup ordered []*CampaignClick // для PurgeOld}func (r *InMemoryClickRepository) GetRecent(userUUID, campaignUUID string, maxAge time.Duration) (*CampaignClick, bool) { r.mu.RLock() defer r.mu.RUnlock() cc, ok := r.byKey[clickKey{userUUID, campaignUUID}] if !ok { return nil, false } if cc.CreatedAt.After(time.Now().Add(-maxAge)) { return cc, true } return nil, false}
PurgeOld теперь должен подчищать обе структуры синхронно. Чуть сложнее, но lookup O(1).
Мораль: “In-memory” — не значит “бесконечно быстро”. Структура данных решает. Линейный поиск по 100M элементов будет медленнее, чем запрос в индексированный PostgreSQL.
8. go publisher.Publish(click) — fire-and-forget без отмены
Возвращаемся к handler:
c.repo.Save(click)go c.publisher.Publish(click) // fire and forget
Помимо багов из пункта #2, тут есть отдельная проблема: горутина запускается без context’а и без какой-либо связи с lifecycle сервиса. Когда придёт SIGTERM:
-
http.Server.Shutdown(ctx)дожидается in-flight запросов -
Но эти запущенные
go publisher.Publish()горутины — никем не учитываются -
Процесс завершается, горутины умирают на полпути
В нашем случае Publish сейчас просто пишет в канал — это быстро. Но завтра кто-то заменит реализацию на синхронный вызов в Kafka — и тогда in-flight publish’и при shutdown будут теряться.
Правильно: использовать errgroup с контекстом сервиса либо принимать context.Context в Publish:
func (p *Publisher) Publish(ctx context.Context, c *CampaignClick) error { select { case p.out <- c: return nil case <-ctx.Done(): return ctx.Err() default: return ErrBufferFull }}
И в handler — синхронно:
if err := c.publisher.Publish(ctx.Request.Context(), click); err != nil { log.Printf("publish failed: %v", err) // не падаем — клик уже сохранён, publish можно отретраить из репозитория}
Мораль: Fire-and-forget горутины (go someFunc()) — это долговая расписка, которую кто-то рано или поздно предъявит. Каждая такая горутина должна либо иметь привязку к lifecycle (через context/wg/errgroup), либо быть явно задокументирована как “best-effort, may be lost”.
Итого Часть 1: 8 проблем concurrency & memory
|
# |
Проблема |
Тип |
|---|---|---|
|
1 |
Забытый |
Data race |
|
2 |
Канал без consumer’а |
Goroutine leak / data loss |
|
3 |
Publisher по значению |
Channel duplication |
|
4 |
PurgeOld не вызывается |
Memory leak |
|
5 |
PurgeOld O(N²) под Lock |
Latency spike |
|
6 |
TOCTOU race в дедупликации |
Race condition |
|
7 |
O(N) поиск в горячем пути |
Performance |
|
8 |
|
Fire-and-forget leak |
Из этих восьми багов на интервью я нашёл семь. Пропустил только #6 — TOCTOU race в дедупликации. Остальные семь поймал, потому что прошёлся по чеклисту: где Lock на write-методах? где consumer для каждого канала? где cleanup для in-memory storage? где O(N) под Lock? Это рутина — не “увидел и удивился”, а “проверил и сверил”.
TOCTOU же выпал, потому что я мысленно “доверился” RWMutex внутри репозитория — а композитная операция “check then save” живёт уровнем выше, в handler’е.
Во второй части — ещё 13 багов. Из них на интервью пропустил два (package main/compile error и отсутствие slog). Расскажу, какие зацепки помогли поймать остальные 11.
🆕 Что нового в Go 1.26 для борьбы с такими багами
-
GOEXPERIMENT=goroutineleakprofile— экспериментальный профайлер, ловит ровно проблемы из бага #2. Запускайте на staging. -
Green Tea GC by default — на 10-40% меньше overhead на GC. Особенно заметно на short-lived объектах (как наш ad-tech).
-
Стек-аллокация для slice backing stores в большем числе случаев — компилятор стал умнее с escape-анализом.
Напоминаю про свой telegram канал который я начал на прошлой неделе. Опять мне напихают что рекламой делюсь, но даю все равно ссылку t.me/go_interview_prep_ru. В нем собираюсь:
📝 Подробными разборами задач с реальных собесов
🧠 Еженедельными Go quiz с объяснениями
💡 Инсайдами про процессы топовых компаний
🔧 Практическими советами для Senior уровня
💰 Данными по зарплатам и market trends
П.С.: А как у вас с code review на собесах? Какие самые жёсткие куски кода давали? Делитесь в комментариях — соберу подборку для следующей статьи.
П.П.С.: Часть 2 уже в работе — там про API design, валидацию, graceful shutdown и тестовую горутину, которая не тестирует ничего. Stay tuned.
ссылка на оригинал статьи https://habr.com/ru/articles/1031010/