Pull to refresh

Как написать красивый код и завалить проект

Reading time 6 min
Views 40K
— Мы забрели в зону с сильным магическим индексом-объяснил он, — Когда-то давно здесь образовалось мощное магическое поле.
– Вот именно, — ответил проходящий мимо куст.
Терри Пратчетт, Цвет волшебства


Поддерживать некрасивый код неприятно. В некрасивом коде сложнее разобраться, он чаще бывает устаревшим и зачастую содержит ошибки. Однако это честная неприятность — ты сразу знаешь, что с кодом не всё впорядке и пишешь дополнительные тесты перед изменением, несколько раз проверяешь, закладываешь в оценках время на то, чтобы всё починить.

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



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


Guava: как написать меньше кода и потратить больше ресурсов


Guava — это библиотека базовых методов и объектов, созданная Гуглом в качестве альтернативы стандартным библиотекам и обладающая множеством полезных функций. В частности в Guava реализованы библиотеки для работы с коллекциями, которые позволяют упростить манипуляцию с данными. Эта история покажет что эту простоту очень легко использовать во зло.

Пусть нам надо реализовать сервис, который занимается в основном преобразованиями данных: запрашивает данные у внешней системы, оповещает другую внешнюю систему и отдаёт данные тому, кто их запрашивал. Реализация может выглядеть примерно так:

	private final static Predicate<Entity> checkPredicate = entity -> entity.performCheck();
	private final static Function<Entity, Integer> mapToIdFunction = entity -> entity.getId();
	
	private final Function<Integer, Entity> lookupFunction = id -> storageService.lookupEntity(id);
	
	@Override
	public ImmutableList<Entity> getCheckedEntityByIds(List<Integer> ids) {
		logger.info("Received " + ids.size() + " ids.");
		
		List<Entity> uncheckedEntities = Lists.transform(ids, lookupFunction);
		Collection<Entity> filteredEntities = Collections2.filter(uncheckedEntities, checkPredicate);
		
		notificationService.sendUpdate(Collections2.transform(filteredEntities, mapToIdFunction));
		
		logger.info("Got " + filteredEntities.size() + " entities.");
		return ImmutableList.copyOf(filteredEntities);
	}


Согласитесь, написано достаточно лаконично и можно легко понять, что происходит, просто читая исходный код. Есть только одна проблема — этот красивый и лаконичный код содержит ошибку, которая приводит к бессмысленной трате ресурсов и может привести к внутренним ошибкам в данных. Ошибку легко продемонстрировать таким тестом:

	@Before
	public void setup() {
		storageService = new StorageService() {
			@Override
			public Entity lookupEntity(int id) {
				lookupCounter++;
				return new Entity(id);
			}
		};
		notificationService = new NotificationService() {
			@Override
			public void sendUpdate(Collection<Integer> ids) {
				System.out.println(ids);
			}
		};
		
		guavaServiceImpl = new GuavaServiceImpl(storageService, notificationService);
	}

	@Test
	public void testGuava() {
		List<Integer> ids = new ArrayList<>();
		for (int i = 0; i < 100; ++i) {
			ids.add(i);
		}
		guavaServiceImpl.getCheckedEntityByIds(ids);
		Assert.assertEquals(100, lookupCounter);
	}

java.lang.AssertionError: expected:<100> but was:<300>
at org.junit.Assert.fail(Assert.java:88)

Как несложно видеть, результат теста неутешительный — мы делаем в три раза больше запросов, чем разумно. Это вызвано тем, что соответсвующие методы на самом деле не трансформируют\фильтруют\etc, а создают соответсвующую обёртку. Т.е. для получения информации, к примеру, о длине коллекции нам прийдётся перепроверять все элементы из более верхней коллекции (трансформирующей), которая в свою очередь будет трансформировать элементы каждый раз при запросе.

Так что в лучшем случае мы получим потерю производительности из-за скрытой стоимости простых операций ~O(n * m), где n — число элементов, а m — вложенность коллекций, а в худшем это приведёт у утечкам памяти и неконтролируемым запросам к другим сервисами т.к. даже отфильтрованная коллекция с несколькими элементами будет хранить ссылки на все изначальные элементы.

Совет по использованию Guava: даже если результаты трансформации и фильтрации выглядят как коллекции и крякают как коллекции, считать коллекциями их не стоит, они являются одноразовыми и перед практически любым использованием стоит скопировать их содержимое в другую коллекцию. К примеру хорошая практика использовать ImmutableList.copyOf() — это сможет защитить и от других проблем.

Google App Engine: почему полезно не верить документации

Все лгут.
Доктор Хаус


Google App Engine — достаточно удобная платформа для разработки и развёртывания облачных приложений, с возможностями для автоматического выделения и распределения ресурсов при высокой нагрузке и быстрой работы с различными сервисами, как то база данных и файловое хранилище. Эта история покажет, какие подводные камни есть у простоты интеграции между различными компонентами.

Главный герой этой истории — средний по размерам проект на базе App Engine с одной базой данных Cloud SQL (Maria DB), которую он мапил в бизнес сущности с помощью Hibernate.

Проект развивался, база росла и в один прекрасный момент наш будущий пациент просто упал. Падение сопровождались пустотой в логах и огромным числом коннектов к базе данных (при всего нескольких экземплярах приложения). Также в логах базы данных обнаружилось несколько неудачных бейкапов. После рестарта базы данных приложение стало работать как ни в чём не бывало.

Самый напрашивающийся диагноз — это волчанка кто-то блокирует базу данных. Так как никаких долгих транзакций по записи в логах обнаружено не было, то виновником посчитали бейкап базы данных, который переконфигурировали на работу без блокировок. Так же были включены бинарные логи, чтобы в случае повторения можно было найти транзакции.

Лирическое отступление об mysql binlog
MySQL поддерживает несколько видов логов. В бинарные логи попадают только транзакции, которые изменяют базу данных, благодаря чему он работает достаточно быстро даже для использования на продакшн сервере.
Для включения его необходимо запустить инстанс с атрибутом --log-bin[=base_name].
Проверить что бинарные логи пишутся можно таким запросом:
SHOW VARIABLES LIKE 'log_bin';
SHOW BINARY LOGS;

Прочитать полученые результаты можно с помощью утилиты mysqlbinlog


Однако ни лечение, ни анализы не помогли — падение повторилось, а бинарные логи не показали никаких подозрительных транзакций.

После тщательной перепроверки всего, связанного с базой данных, причина падений таки была найдена и она оказалась вызвана следованием рекомендациям из документации App Engine. В частности, в случае приложения и базы данных на App Engine, документация советует открывать новый коннект к базе данных каждый раз при получении нового запроса, т.к. открытие нового коннекта дешевле чем поддержание старого:
How to best manage your database connections depends on your use case. For example, if the time to create a new database connection is greater than checking and reusing an existing connection, then we recommend you use connection pooling. Conversely, if the time to create a new connection is about the same as testing if an existing connection is alive and reusing it, then we recommend that you create a new connection to service each HTTP request, and reuse it for the duration of the request. In particular, the latter case may apply when you connect from Google App Engine to Google Cloud SQL.
Google Cloud SQL FAQ

Однако в связке с Hibernate такой подход привёл к достаточно печальным результатам: в этом приложении есть несколько достаточно тяжёлых запросов на выборку данных из таблицы, которые обычно прорабатывают нормально. Но в случае нескольких параллельных запросов они могут выполнятся дольше минуты, что приводит к автоматическому обрыву запроса средствами App Engine. Код, который ждёт результатов запроса на Java стороне перестаёт выпоняться, но сессия на стороне базы данных продолжает обработку результата тормозя следующие запросы, в результате чего и получается своеобразный «снежный ком».

Для полноты истории стоит рассказать, что всё закончилось хорошо благодаря настройке пула коннектов. В случае Hibernate это можно сделать при помощи замены свойства
<connection.pool_size>0</connection.pool_size>
на настройки конкретной реализации пула коннектов. В случае App Engine предпочтительные варианты — это DBCP и HikariCP.
Предупреждение: встроенный в Hibernate пул коннектов использовать не стоит!
В документации Hibernate прямо говорится, что его можно использовать только для тестовых целей:
Hibernate's own connection pooling algorithm is, however, quite rudimentary. It is intended to help you get started and is not intended for use in a production system, or even for performance testing. You should use a third party pool for best performance and stability. Just replace the hibernate.connection.pool_size property with connection pool specific settings. This will turn off Hibernate's internal pool. For example, you might like to use c3p0.
Hibernate API documentation


Небольшое заключение


Эти две истории символизируют, что никакая простота и красота кода не даётся бесплатно. Помните — все лгут и для того, чтобы ваше приложение работало так хорошо как вы хотите, вы должны точно понимать что на самом деле делает ваш красивый код и та магия, которую он использует.
Tags:
Hubs:
+12
Comments 28
Comments Comments 28

Articles