Сложно ли изменить две строчки кода?
Герой этой истории — open source проект H2 database популярной реляционной базы данных для тестов, веб консоли для SQL и даже содержит внутри себя аналог LevelDB/Berkeley DB Java Edition/SQLite 3. Отличный проект, много раз использовал за свою практику и не было проблем. До тех пор пока не попытался использовать его совместно с redshift jdbc driver.
Есть такая база данных в AWS, Redshift — форк времен PostgreSQL 8.0.2. Где-то в том же десятилетии появился и его конкурент greenplum-db… Несмотря на то что эта БД имеет массово-параллельную архитектуру, и прочие «плюшки» column-oriented DBMS, при работе с ней не покидает ощущение что ты в музее компьютерной истории. Понял, что это ощущение было неспроста, когда обнаружил что в приложении конфликтуют драйвера современного PostgreSQL 9.6 и Redshift драйвер ископаемого wire протокола postgresql 8.x.
Обнаружил что используется PG wire protocol 8.x, когда подключался к PostgreSQL 9.6 в H2 web консоли. Результаты меня огорчили и я начал разбираться как же такое может происходить. Отладка привела в строчку получения соединения:
DriverManager.getConnection(url, prop);Вроде бы все выглядит по спецификации, так как это не JNDI и не javax.sql.DataSource.
Спускаемся глубже в DriverManager. Там все и так известно и ожидаемо. В его блоке статической инициализации используется ServiceLoader для загрузки реализующих java.sql.Driver и заявляющих об этом с помощью записи о реализации META-INF/services/java.sql.Driver в своем jar. Это достаточно давно отменило использование Class.forName(driver) — так все современные драйверы загружаются без этого ископаемого вызова. Ничего нового для меня тут нет.
Драйверы при запросе соединения перебираются в порядке, как они зарегистрировались в поле registeredDrivers. DriverManager для каждого из них по цепочке вызывает driver.connect(url, info). Если конкретный драйвер вернул объект соединения с базой данных, возвращаем его из функции. Первый кто обработал connection URL из цепочки драйверов побеждает!
Драйвер сам анализирует может ли он обработать подпротокол jdbc:subprotocol. На мою беду redshift jdbc драйвер обрабатывал кроме своего «redshift» еще и «postgresql», но с помощью древнего кода эпохи середины 2000х. Ясно, что connection url запрос никогда не дойдет до драйвера postgres 9.6.
Один плюс в карму разработчикам redshift jdbc — спасибо хоть классы древней реализации PG в отдельный пакет спрятали, а не оставили конфликтовать с org.postgresql.Driver в jar hell. Попробовал использовать более «свежий» их драйвер, но он не работал внутри spring boot executable jar, так как в нем зависимости упаковали «матрешкой» — jar'ы зависимостей внутри jar драйвера.
При этом пул соединений HikariCP правильно создает новый драйвер postgresql, в отличии от консоли H2. Раз уж пользователь указал driverClass, то он на нем и вызывает connect не полагаясь на DriverManager. Это работает в аду, учиненном redshift jdbc. Причина была найдена быстро и стало ясно как решать проблему.
Патч был создан в выходной и отправлен как pull request в репозитарий проекта, заодно создал заявку об ошибке. После этого началась переписка и аргументация изменения для контрибьютора, второго по активности в репозитарии h2database. Выполнил все его требования и замечания для этого pull request и изменения приняли в основной код проекта. Ушло много свободного времени из-за двух строчек изменений и драйвера redshift. Но тут уже был азарт и дело принципа — выжить в мире где ископаемый протокол перекрывает современный. Спасибо ему за уделенное время, за то что вник в эту проблему. Верю что дотошность при приеме pull request в популярном open source проекте идет на пользу качеству. Прошло почти два выходных дня, пока две строчки для исправления бага появились в проекте.
Другой pull request на новый функционал в schemaspy висит уже больше недели. Тут виноват сам, проблема что разрабатывал его на linux, а не работало на windows системе. Каюсь, что не дотестировал сразу.
Делитесь про то, как несколько строк кода поглощали время. Есть интригующие истории и рассказы детективного жанра?
Комментарии (43)
Areso
26.12.2017 06:29Считаю, что нужно отличать дебаг и последующий фикс и новую функциональность.
Для первого две строчки за пару дней — это отличный результат. Хуже, когда на это уходит месяц.
А вот новый функционал пишется гораздо проще и веселее, и быстрее (если уж считать в SLOC).
igor_suhorukov Автор
26.12.2017 09:05Согласен. Ещё рефакторинг — источник нового кода, почти как новая фича
TheShock
27.12.2017 01:36У нас был джун, он говорил на стендапах что-то вроде: «Я вчера рефакторил Редактор»
Это означало, что он запилил пару новых фич в Редактор и ничего не рефакторил.
igor_suhorukov Автор
26.12.2017 09:14Ещё гораздо меньше строк пишется/генерируется IDE, если подключить к проекту lombok
TheKnight
26.12.2017 12:49Или Kotlin…
Хотя в Kotlin (и Scala) пока нет удобных для Java аннотаций для логгирования@Log @Slf4j @Log4j @Log4j2
Borz
26.12.2017 15:57зачем аннотация, когда есть kotlin-logging
TheKnight
26.12.2017 16:10Потому что в Lombok используется аннотация? :) Искал решение похожее на то, что уже использовал.
Такой код:
companion object: KLogging()
выглядит длиннее чем одна коротка аннотация но лучше чем обычно.
val logger = LoggerFactory.getLogger("class name")
dougrinch
28.12.2017 00:18А чем плох такой вариант?
val Any.logger: Logger by object : ReadOnlyProperty<Any, Logger> { val loggers = ConcurrentHashMap<Class<*>, Logger>() override fun getValue(thisRef: Any, property: KProperty<*>): Logger { return loggers.computeIfAbsent(thisRef.javaClass, LoggerFactory::getLogger) } }
Да, не static final, но неужели не пофиг?
TheKnight
28.12.2017 14:37Вопрос в лаконичности.
Еще раз: в Lombok для создания объекта логгера используется одна короткая анннотация. В Kotlin это выглядит многословней. Вот вся претензия.dougrinch
28.12.2017 17:58Так наоборот же. Эту штуку надо написать всего один раз (и можно даже в библиотеке) и всё! Вам больше вообще ничего делать не надо. У всех классов уже есть логгер. В отличии от ломбока, в котором как раз надо еще целую аннотацию поставить.
TheKnight
28.12.2017 19:45Моя тупить. Можно пример минипроекта на plain Kotlin (не Android, spring, webservices, буквально три-четыре простеньких класса) который работает с этим?
dougrinch
28.12.2017 21:03Если честно, то лень. Но это и есть весь код. Суете в либу на топ уровне что я писал ранее, а используется просто как
import org.dou.lib.logger class Foo { init { logger.info("hello") //[main] INFO org.dou.Foo - hello } }
На самом деле, я уже обнаружил две большие проблемы у этого варианта:
1. Это публичное экстеншн проперти, так что я «logger» могу написать не только внутри класса, но и снаружи42.logger.info("msg") //[main] INFO java.lang.Integer - msg
2. Он вообще не работает с топ левел функциями (вне класса/объекта). Т.к. там нет this, у которого logger и отрастает.
В принципе, обе проблемы решаются если убрать «Any.» и сделать просто
(тоже на рутовом уровне), но здесь я пока не придумал как в теле геттера получать класс, откуда он вызывается. Каждый раз через исключение смотреть стектрейс — явно плохо.val logger: Logger get() { TODO() }dougrinch
28.12.2017 21:22Ну да, иначе никак не получается сейчас
val public_strange_name_do_not_use_it = ConcurrentHashMap<String, Logger>() inline val logger: Logger get() { return public_strange_name_do_not_use_it.computeIfAbsent(Exception().stackTrace[0].className) { cn -> LoggerFactory.getLogger(Class.forName(cn)) } }
По получаемому синтаксису вроде идеально получается, здесь только с перформансом уже проблемы скорее всего будут (по крайней мере, для библиотечной функции общего пользование такое явно недопустимо). В принципе, если дождаться JEP-303, то в паре с инлайном это будет идеальное решение. По идее, тогда даже мапа будет не нужна, т.к. можно будет повесить CallSite напрямую на нужный логгер, но в этом я пока не уверен, надо внимательнее посмотреть.dougrinch
28.12.2017 21:57Во!
public static CallSite bootstrapLoggerFactory(MethodHandles.Lookup lookup, String name, MethodType type) { Logger logger = LoggerFactory.getLogger(lookup.lookupClass()); MethodHandle mh = MethodHandles.constant(Logger.class, logger); return new ConstantCallSite(mh); }dougrinch
29.12.2017 04:34А, блин, лоханулся я. Это же фича джава компилятора будет, так что надо еще допиливать котлиновский чтобы он про Intrinsics.invokedynamic тоже узнал. А если в любом случае надо его пилить, то и JEP-303 уже можно не ждать и сделать свой велосипед.
norlin
26.12.2017 12:04Когда новую функциональность надо вкорячить в существующий код – это тоже не всегда тривиально бывает.
PapaBubaDiop
26.12.2017 11:47+1Была ошибка в 1 символ (другая кавычка в JSON файле на стороне сервера)
1) создать стори в JIRA
2) создать бранч в git server project
3) создать бранч в git front project
4) исправить кавычку в server
5) добавить тест в front
6) прогнать все тесты
7) pull request
8) merge 1
9) merge 2
10) close story
Неделя. А фактической работы — 0.3 секунды.
И да — ту кавычку вставил MAC OS текстпад роднойjustboris
27.12.2017 00:24Не очень понял, почему фикс делаем в "server project" а тест пишем во "front project". Было бы в одном месте, число действий сократилось бы вдвое.
PapaBubaDiop
27.12.2017 00:56UI Test на стороне телефона, который читает данные с сервера.
justboris
27.12.2017 01:16Все равно неясно.
Если сервер возвращает JSON, который ломает клиента, то нужно либо пофиксить сервер (и написать в сервере тест "не должен возвращать плохой json"), либо научить клиента обрабатывать такую ситуацию (и написать в клиенте тест "не падать когда получил такой-то json").
m08pvv
26.12.2017 12:11+1На тему обратной зависимости размера фикса от затраченного времени была отличная статья «The reverse correlation between size of change and length of investigation» в блоге Ayende Rahien (RavenDB).
igor_suhorukov Автор
26.12.2017 12:24Спасибо, интересно. Согласен с автором что race condition сложно воспроизвести
progchip666
26.12.2017 21:35+1Легко может. Особенно при программировании для встраиваемых систем, да ещё в случае многопроцессорных устройств! Когда аппаратные глюки накладываются на программные, причём в другом микроконтроллере.
igor_suhorukov Автор
26.12.2017 22:07Браво! Это вообще отдельная область разработки. Наводки, зависания из-за непропайки
progchip666
26.12.2017 22:26+3Чего только не случается. Непропайки, гонки фронтов. Последний раз провозился две недели потому что у микросхемы уважаемого производителя ADI почему то при понижении напряжения питания надо увеличивать время между записями в два регистра с миллисекунд буквально до 10 секунд. Без этого она не просто блокировала шину SPI и почему то начинала потреблять раз в 10 больше чем положено. В результате начинал греться стабилизатор и срабатывала тепловая защита. Попробуй разберись где там причина а где следствие.
Но на хабре почему то программистов микроконтроллеров не считают разработчиками вообще, нас даже изгоняли с этой площадки года на два или три в недалёком прошлом.
Наверно потому, что приравняли нас к ШАМАНАМ
Jef239
26.12.2017 23:18+1Угу, знакомо. Отладка кода осциллографом, отладка железа при помощи GDB… Правка бага в коде при помощи проводочка, правка ошибки в разводке кодом…
Причем даже на довольно больших компах замашки embeded работают. Как понять, на какой скорости и в каком формате программа гонит данные по COM-порту? Берем осциллограф и смотрим… Это вот вчера было, когда некий древний linux при воде русских символов переключил формат консоли из 8N1 в 7N1. :-) Первая мысль — а давайте осциллографом глянем.
igor_suhorukov Автор
26.12.2017 23:44Видимо разработка специфичнее и требует большего количества знаний, документации больше, многое ценное под NDA и комьюнити меньше.
Nick_mentat
27.12.2017 15:06+1Одна строчка
int x
Съела отпуск. Убила сорсник.
Когда-нибудь я допишу статью об этом…
Comdiv
27.12.2017 23:44Перекрытие имени в объемлющей области видимости? Бывает. Поэтому стоит включить соответствующую опцию в компиляторе.
Nick_mentat
28.12.2017 12:31Если быть точным — сознательное перекрытие областей, как хак решивший кучу проблем. Всё сломалось, когда гуру клинкода эту строчку удалил (она специально была написана в модуле где не используется).
Comdiv
28.12.2017 15:26+1Если это и решало кучу проблем, в конечном итоге такое хрупкое решение вряд ли можно признать хорошим. Поэтому не стоит винить «гуру клинкода», если только этот
int xне был помечен очевидным и подробным комментарием.Nick_mentat
29.12.2017 02:47//Do NOT touch! Very Important Code: int x
Позже пришлось добавить ещё и простыню текста снизу, поясняющую цель существования данного хака.
Sellec
27.12.2017 17:10+1Наверное, со своими пятью копейками про три дня ковыряния в коде DevExpress во время поиска причин непонятного поведения компонента WinForms после выхода новой версии с отсутствием документации по изменению этого поведения не стоит писать, потому что блог про OpenSource и Java? :)
igor_suhorukov Автор
27.12.2017 17:32Почему же… Боль-то одна — красноглазие. Не мне одному будет интересно.
alxt
Несколько дней.
Я хотел когда-то доработать что-то в hibernate, чтобы не писать лишнего при коннекте к firebird. Тогда у них был SVN а патчи надо было слать почтой. Отослал. 2 строки.
Тот проект мой заглох, больше я с firebird не работал.
Через 7 карт мне пришёл ответ, мол надо ж pull-request в github присылать. А я уже забыл, что и зачем делал…
alxt
7 лет, конечно…
igor_suhorukov Автор
Солидно! 7 лет