Средний

11 мин

6.9K

Продолжение прошлой статьи про ошибки на 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)  // возвращает *Publisher
ctrl := 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-операции “сначала проверь, потом сохрани”.

Сценарий:

1. Goroutine A: GetRecent(user1, camp1) → не нашёл

2. Goroutine B: GetRecent(user1, camp1) → не нашёл

3. Goroutine A: Save(click1)

4. 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:

1. http.Server.Shutdown(ctx) дожидается in-flight запросов

2. Но эти запущенные go publisher.Publish() горутины — никем не учитываются

3. Процесс завершается, горутины умирают на полпути

В нашем случае 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

Из этих восьми багов на интервью я нашёл семь. Пропустил только #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.

П.П.П.С: Дописал так как увидел что кто то подумал - текст не сгенерированный, а реальный пример из компании где проходил собеседование. Но на Ваше усмотрение - так как обычно после этого еще больше напихают.