Страшной ненависти к быдлокодерам пост

[dil]

А сегодня ночью один скриптик перловый, запускаемый из крона, не отработал. Бывает. Ну я его перезапустил. А он чего-то думает долго. Минут через десять мне надоело ждать и я посмотрел в код.  А там ТАКОЕ… что прям хоть целиком в code_wtf отправляй.

Начало было уже хорошее:

if ( -e $filepath)
{
        unlink($filepath);
}
open (FILE, ">> $filepath");

На хрена, спрашивается, стирать файл, да ещё внешней программой, а потом открывать его на дописывание?

А дальше была реализована тривиальная, в общем-то, задача: выбираем из одной базы список пользователей, потом проверяем для них некоторые критерии в другой базе, и если совпало, то достаём регистрационные данные  и записываем в отдельную таблицу. Но КАК она реализована…

Во-первых, выборка пользователей почему-то работает очень долго, хотя всего по двум таблицам. Вскрытие показало, что в одной из таблиц много миллионов записей, а индекса по полю, по которому она связывается с другой таблицей, нет. Имеем многократный full scan в полный рост. Многократный – потому что там вложенный подзапрос с агрегирующей функцией плюс ещё во внешнем запросе группировка с использованием max(). И мне не очень понятно, зачем так сделано, но это я буду выяснять потом. Короче, работает он минут десять. После добавления простого индекса стал работать четыре, и DBA обещали подумать, как его можно ещё улучшить.

Но это фигня. Про binding параметров аффтар, видать, не подозревал. Не дочитал до этого места в документации. Поэтому дальше в цикле для каждого найденного пользователя формируется _строка_ с запросом на проверку критериев и выборку данных из второй базы. Имя пользователя вставляется непосредственно в эту строку,  далее на неё запускается prepare и execute. Запрос, естественно, никто не проверял на оптимальность, поэтому там тоже full scan, и работает он секунд пять. И  так для каждого из пары тысяч найденных пользователей. Поэтому скрипт работает в общей сложности больше двух часов, напрягая сервер почём зря.

Запись в таблицу реализована аналогично — каждый раз формируется строка с запросом, prepare, execute. Пару тысяч раз.

while(($username,$blah...) = $sth->fetchrow()) {
        ...
        $loadline ="\"$username\", \"$blah\",...";
        $loadline =~ s/,,/,NULL,/g;
        $loadline =~ s/,$/,NULL/g;
        $insertsql = "insert into foo values ($loadline); ";
        $sth2 = $dbh->prepare($insertsql);
        $sth2->execute();
}

Главное, совершенно непонятно,  нафига вытягивать данные в приложение и отправлять их во вторую базу через другое соединение. Базы хоть и разные, но на одном сервере. Обращаться к соседней базе можно через одно соединение  без всяких накладных расходов.

Когда я это осознал, громко выругался вслух. Потом написал письмо DBA. Которые высказались примерно в том же духе, мягко назвав это bad coding.  Потом я за 20 минут переписал этот ужас, сложив результаты первого запроса во временную таблицу и прицепив её ко второму. И попросив DBA добавить пару индексов.

Время работы скрипта сократилось с двух часов до четырёх минут,  которые, собственно,  занимает первый запрос. Второй работает меньше секунды.

Самое страшное, что этот скрипт там не один. Их многие десятки, и я сильно подозреваю, что в остальных ужас примерно такой же.

И ведь этих людей кто-то взял на работу. И они пишут код, который потом годами работает, потребляя ресурсы совершенно зазря. И в который никто не смотрит. Я туда заглянул совершенно случайно…

Upd: на unlink это уже тоже я поменял. В оригинале было так:

if ( -e $filepath)
{
        `/bin/rm $filepath`;
        `/usr/bin/touch $filepath`;
}
else
{
        `/usr/bin/touch $filepath`;
}
open (FILE, ">> $filepath");

Оригинал этой записи. Комментировать можно тут или там.

Любые материалы из этого блога запрещается использовать на сайте livejournal.ru в любой форме и любом объёме

Comments

[tejblum.livejournal.com]

Это, конечно, в данном случае скорее всего неважно, но:

1. unlink() в подобных случаях может оказаться нужен, поскольку потенциально меняет поведение программы. Стереть файл и создать его заново -- не то же самое, что обрезать его. Например, если указанное имя -- симлинк куда-то далеко, то unlink() и open() сотрет симлинк, и создаст файл в указанном месте, а open() без unlink()'а -- обрежет тот файл, куда указывает симлинк, и будет писать туда.


2. "open for appending", который делает >>, и "open for output", который делает > -- не одно и то же. Подробней об этом написано в man 2 open, в разеделе про O_APPEND.

[dil.livejournal.com]

в данном конкретном случае он не нужен, в этот файл каждый раз пишутся текущие данные, а потом он отправляется по почте. достаточно open на запись.

[alexkuklin.livejournal.com]

Да-да.
"Был на нашей деревне случай" (ц), когда отсутствие unlink привело к потере данных - т.к. копия делалась при помощи hardlink.