Вчера произошла эпическая история. После планового деплоя в субботу вечером (так было нужно), мне прилетело сообщение “кирилл, у нас почему-то не показываются заявки”. Наверное фильтры слетели, подумал я и пошел проверять. Фильтры не слетели. Я слегка напрягся и пошел в яндекс клауд посмотреть что там в базе. Как я и боялся, таблицы были пустыми. Причем не все, но многие. Самое интересное, что они были не просто пустыми, но у них сбросились счетчики.
Увидел я это не сразу после деплоя, поэтому было не до конца понятно, это деплой привел к удалению данных или что-то другое. Я быстро восстановил снепшот на новом кластере, благо это делается одним кликом и выполнил туда деплой заново. Какого было мое удивление, когда после деплоя база очистилась. Какого хрена подумал я, прикидывая, что могло быть причиной. В этот момент ко мне присоединился второй разработчик проекта, с которым мы весело провели 3 часа за дебагом.
Сам деплой был необычным, потому что мы выкатывали большое изменение для обработки заявок основного договора (до этого работало только раннее бронирование). Туда входило и много кода и около 40 миграций и обновления зависимостей и новая конфигурация. Но мы точно не добавляли код, который бы грохал половину базы (как нам тогда казалось, хаха).
Дальше мы полезли изучать код на предмет подозрительных вещей:
Логи
Изменения в конфигурации
Ишьюсы в Laravel (основной фреймворк)
Миграции
Ничего подозрительного не нашли, за исключением пары транкейтов в паре миграций. Но эти транкейты были на новые таблицы, в которых точно не было данных, их скорее делали даже для локального сброса, чтобы не фиксить данные в девелоперских базах после изменения структуры таблицы.
В любом случае мы решили проверить миграции, поэтому сделали следующее. Настроили систему так, чтобы за один деплой выполнялась ровно одна миграция. Дальше мы начали выполнять последовательно деплои, попутно проверяя состояние базы в конце каждого деплоя. И, вдруг, где-то на двадцатой миграции база очистилась.
Открываем миграцию, а там тот самый пресловутый транкейт, который хоть и выглядел подозрительно, но не касался других таблиц. Смотрю в логи и вижу что транкейт выполняется так: TRUNCATE marketing_discounts CASCADE. И тут я как понял.
Не знаю как так получилось, но я даже не был в курсе, что у транкейта есть такая опция. CASCADE приводит к тому, что дропаются все связанные таблицы (рекурсивно) независимо от того, есть ли там данные или нет. Сказать что я был в шоке, ничего не сказать. Тут же нашелся issues в ларавеле, где выяснилось несколько интересных деталей. Мы не единственные кто грохнул базу таким образом. Собственно сам issue появился с целью того, чтобы защитить всех остальных, на что разработчики сказали сорян, обратная совместимость. Самое смешное, что подобное поведение реализовано только для драйвера постгреса, у остальных такого нет.
> When you truncate tables using the laravel illuminate db builder it truncates the table as expected. However, postgresql is different because it changes the DEFAULT behavior of truncate from RESTRICT to CASCADE. This means that you can loose all your data in other "related" tables (something that doesn't happen with the other sql drivers)
И ниже смешные комментарии в духе:
> 3 years passed, Laravel users still truncates their entire databases...
> We were also a victim of this behavior this morning, fortunately we were on a test database. Very dangerous!
Ишью кстати закрыли с поинтами что мы не будем ломать обратную совместимость и транкейт должен сбрасывать данные, а не падать с ошибкой.
В итоге мы все поправили и восстановили данные, но открыли в себе новый страх. Давно в моей жизни не было таких приключений)
О разработке я пишу в своем телеграм канале "организованное программирование" https://t.me/orgprog/137
Сам ишью: : https://github.com/laravel/framework/issues/35157
Комментарии (36)
vtal007
01.07.2024 14:58+2Простите, полез в доку
https://www.postgresql.org/docs/current/sql-truncate.html
Пишут, что по дефолту - рестрикт
Или это ребята в Laravel напортачили?
toxicmt Автор
01.07.2024 14:58+1Об этом и речь. Я привел цитату в посте, плюс можно по ишьюсу понять что они дичь сделали: https://github.com/laravel/framework/issues/35157
artptr86
01.07.2024 14:58+3truncate здорового человека:
https://github.com/illuminate/database/blob/master/Query/Grammars/Grammar.php#L1397
public function compileTruncate(Builder $query) { return ['truncate table '.$this->wrapTable($query->from) => []]; }truncate курильщика:
https://github.com/illuminate/database/blob/master/Query/Grammars/PostgresGrammar.php#L631
public function compileTruncate(Builder $query) { return ['truncate '.$this->wrapTable($query->from).' restart identity cascade' => []]; }Зато всё по науке — переопределение базового метода в наследнике :)
HardWrMan
01.07.2024 14:58Когда рад, что тебя с самого начала учили, что всегда всё переопределяй сам и никогда не полагайся на дефолты. Правда, это относилось к программам, особенно на МК, но, похоже, оно актуально даже с БД.
Drucocu
01.07.2024 14:58+2Спасибо, что поделились. Ни в коем случае не хочу набрасывать, просто личные эмиоции от прочитанного:
много кода и около 40 миграций и обновления зависимостей и новая конфигурация
Жесть. Гигантские релизы - зло. Вероятность того, что что-то пойдёт не так, крайне высока. Если есть возможность, всегда нужно стараться выкатывать код постепенно, пусть часть и не будет задействована в данный момент.
CASCADE приводит к тому, что
дропаются{очищаются} все связанные таблицыЭто более или менее интуитивно. Связь же строится на основе ключей. С чем связываться, если в родительской таблице все данные очищены?
toxicmt Автор
01.07.2024 14:58+3Жесть. Гигантские релизы - зло. Вероятность того, что что-то пойдёт не так, крайне высока.
Вы абсолютно правы, но тут важен контекст, этот релиз делался всего три недели и его нужно было выкатывать отдельно. Проект почти что внутренний и носит некий технический характер.
> Это более или менее интуитивно. Связь же строится на основе ключей. С чем связываться, если в родительской таблице все данные очищены?
Это не дефолтное поведение. По дефолту транкейт не удаляет ничего в связанных таблицах. Ужас не только в том что они поменяли дефолтное поведение, но и еще и в том, что сделали это только для одного драйвера. В итоге получился не принцип наименьшего удивления, а принцип наибольшего удивления.artptr86
01.07.2024 14:58+3Это не дефолтное поведение. По дефолту транкейт не удаляет ничего в связанных таблицах.
Ирония ещё и в том, что и PostgreSQL, и MySQL, и MariaDB, и MS SQL Server, и (вероятно) даже SQLite не могут транкейтить таблицы, на которые через внешние ключи ссылаются другие таблицы. При этом, похоже, только у постгреса есть опция каскадного транкейта. Ей-то они и воспользовались.
Судя по всему, они столкнулись с багом транкейта для постгреса, но не проанализировали, что аналогичное может случиться и с другими СУБД. По-хорошему, им нужно было бы сделать дополнительную опцию
truncate, а при отсутствии поддержки или эмулировать каскадный транкейт, или выбрасывать ошибку. То решение, которое они приняли — самое опасное и архитектурно непродуманное, но и самое лёгкое.toxicmt Автор
01.07.2024 14:58Я не первый раз наталкиваюсь на то, что главный в ларавеле принимает решения, которые вызывают удивление. Там много где в ишьюсах видно что люди годами с ним спорят про проверенные решения в других фреймворках, которые он отказывается добавлять. Иногда бывает что он сопротивляется по пять лет, потом понимает что надо и добавляет (как было со схемой базы)
zubrbonasus
01.07.2024 14:58+1Лучшее время для деплоя, по best practice, это вторник-среда.
Также хорошей практикой есть использование тестов во время деплоя. Такие тесты нужны как раз для того чтобы не удалить случайно данные из бд или чтобы не накатить нерабочий код.
toxicmt Автор
01.07.2024 14:58+2Так как я работаю над проектом в выходные (я фаундер) и в это время пользователей нет, то для меня и безопасности это было лучшее время для деплоя
toxicmt Автор
01.07.2024 14:58+2или чтобы не накатить нерабочий код.
покрытие кода тестами почти 90%. Только ситуация же к коду не имеет отношения, а на миграции тесты не пишут, подобные вещи видят визуально. В данном случае проект микроскопический и над ним никто фултайм не работает (это не проект даже, а так, сайд история к большому проекту), поэтому не заметили.
solidius
01.07.2024 14:58Пусть часть и не будет задействована в данный момент.
Релиз, в котором часть нового кода не используется и оставлена на будущее - очевидно злое зло. Если эта часть окажется багованной, то вместо проверки только изменений последнего патча, придется еще собирать подозреваемых по кускам из старых патчей.
2medic
01.07.2024 14:58+2О как! А не подскажете, каскадное удаление использует метод класса модели (Model::truncate) или DB::table()->truncate() тоже?
Inskin
01.07.2024 14:58+4А что, тестового/предрелизного стенда у вас нет? На нём бы сразу же проблема всплыла, не дошла бы до продакшена.
toxicmt Автор
01.07.2024 14:58Всплыла бы да, но даже такая ситуация для данного проекта не требует введения стейджинга. Подобные ситуации случаются крайне редко, а если все остальное нормально, то они еще и не критичны. В нашем случае вообще ничего страшного не случилось, мы были готовы.
Enjection
01.07.2024 14:58+3А для себя-то какие-то выводы сделали? Стали читать sql код генерируемый фреймворком? Подняли препрод и стали репетировать миграции на нём?
toxicmt Автор
01.07.2024 14:58За мою довольно длинную карьеру, что-то подобное в миграциях произошло первый раз. Одна такая ситуация не причина отказываться от инструментария фреймворка, поэтому мы продолжим как действовали, но транкейта в миграциях больше не будет) Препрод нам не нужен, потому что 1) не тот контекст у проекта 2) мы учитывали этот риск, в выходные проект и так практически не работал и не будет работать (более того, это сезонный проект), поэтому все было по плану, просто чуть дольше по времени.
artptr86
01.07.2024 14:58+32Making developers happy
Sleuthhound
01.07.2024 14:58+2Интересно, этому чуваку на газон никто тонну навоза с надписью "спасибо за этот комит" не подвозил за эти 6 лет
sslab
01.07.2024 14:58+4Много кода... 40 миграций...Деплой в субботу вечером...
Столько намеков было )))
toxicmt Автор
01.07.2024 14:58+2Ну согласитесь что если я работаю над проектом в выходные, то логично его деплоить в выходные. Тем более это еще и совпадало с моментом, когда на проекте не было пользователей. А количество кода тут не важно, сама ситуация не подразумевала каких-то глобальных изменений, ну добавилось сущностей и крудов.
artptr86
И где ссылка на issue?
toxicmt Автор
Блин думал что добавил. Вот : https://github.com/laravel/framework/issues/351
artptr86
Это что-то не то
toxicmt Автор
ой, щас: https://github.com/laravel/framework/issues/35157