А сегодня ночью один скриптик перловый, запускаемый из крона, не отработал. Бывает. Ну я его перезапустил. А он чего-то думает долго. Минут через десять мне надоело ждать и я посмотрел в код. А там ТАКОЕ… что прям хоть целиком в
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");
Оригинал этой записи. Комментировать можно тут или там.
no subject
Повсеместно бытует и расширяется мнение, что факторизовать и оптимизировать код надо не раньше, чем наступят проблемы с производительностью или багами.
Что твой пример успешно подтверждает: вам приходилось ждать жалкие часы и раньше, да и на правку кода тебе понадобилось 20 минут. При том что подобного кода там однозначно осталось дофига, но он тебе (судя по всему) не мешает.
На всякий случай уточню, что мне такой подход нифига не нравится, но не я рулю рынком и не ты. :(