-
Notifications
You must be signed in to change notification settings - Fork 114
Оптимизация загрузки данных в бд из json и загрузки страницы #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
1d11f37
81c9eb3
d78d831
5567afd
12be987
6bbf6bd
f1cb57f
82fc9a1
864a979
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,7 @@ | |
| /tmp | ||
| /log | ||
| /public | ||
| fixtures/1M.json | ||
| fixtures/10M.json | ||
| fixtures/*.gz | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| --require spec_helper |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,6 @@ class TripsController < ApplicationController | |
| def index | ||
| @from = City.find_by_name!(params[:from]) | ||
| @to = City.find_by_name!(params[:to]) | ||
| @trips = Trip.where(from: @from, to: @to).order(:start_time) | ||
| @trips = Trip.where(from: @from, to: @to).includes(bus: :services).order(:start_time) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💪
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Не специально. Вообще ar соберёт.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. по идее не помешало бы ещё индекс на trips [from_id, to_id], можно составной именно на время рендеринга это сильно не влияет (так как процент на выполнение этого SQL маленький), но если бы мы заходили со стороны оптимизации БД - там бы это очень помогло; |
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| class TripsImporter | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. лайк за отдельный класс |
||
| def initialize(file = 'fixtures/small.json') | ||
| @file = file | ||
| end | ||
|
|
||
| def self.call(...) | ||
| new(...).call | ||
| end | ||
|
|
||
| def call | ||
| json = JSON.parse(File.read(file)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Можно поробовать разбирать JSON потоков И ваще курутотень будет) |
||
|
|
||
| clean_database | ||
|
|
||
| ActiveRecord::Base.transaction do | ||
| city_names = Set.new | ||
| service_names = Set.new | ||
| buses = {} | ||
|
|
||
| # первый проход - собираем "справочные" данные - города, услуги, автобусы | ||
| # собираем в Set или хэш, так чтобы удобно было вставлять в бд | ||
| json.each do |trip| | ||
| city_names.add trip['from'] | ||
| city_names.add trip['to'] | ||
|
|
||
| service_names.merge trip['bus']['services'] | ||
| buses[trip['bus']['number']] = trip['bus']['model'] | ||
| end | ||
|
|
||
| # вставляем справочное | ||
| City.insert_all city_names.map { |name| { name: name } } | ||
| Service.insert_all service_names.map { |name| { name: name } } | ||
| Bus.insert_all buses.map { |number, model| { number: number, model: model }} | ||
|
|
||
| # формируем хэши, чтобы удобно получить доступ к id при втором проходе | ||
| cities = City.all.each_with_object({}) { |city, hash| hash[city.name] = city.id } | ||
| services = Service.all.each_with_object({}) { |service, hash| hash[service.name] = service.id } | ||
| buses = Bus.all.each_with_object({}) { |bus, hash| hash[bus.number] = bus.id } | ||
|
|
||
| # тут соберём пары автобус-услуга | ||
| buses_services = Set.new | ||
|
|
||
| # тут данные по поездкам для вставки | ||
| trips = [] | ||
|
|
||
| json.each do |trip| | ||
| from_id = cities[trip['from']] | ||
| to_id = cities[trip['to']] | ||
| bus_id = buses[trip['bus']['number']] | ||
|
|
||
| # заполняем пары автобус-услуга | ||
| service_ids = services.values_at(*trip['bus']['services']) | ||
| service_ids.each do |service_id| | ||
| buses_services.add([bus_id, service_id]) | ||
| end | ||
|
|
||
| trips.push({ | ||
| from_id: from_id, | ||
| to_id: to_id, | ||
| bus_id: bus_id, | ||
| start_time: trip['start_time'], | ||
| duration_minutes: trip['duration_minutes'], | ||
| price_cents: trip['price_cents'], | ||
| }) | ||
| end | ||
|
|
||
| # вставка данных о поездках | ||
| # пока не стала батчить, т.к. и так довольно быстро происходит | ||
| Trip.insert_all trips | ||
|
|
||
| # вот такой insert into buses_services | ||
| # вообще чаще используем has_and_belongs_to_many , потому что часто в связке потом нужны доп. данные и таймстемпы, и свой id | ||
| # тогда можно было бы insert_all использовать | ||
| if buses_services.present? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ухты, не подумал что так можно было buses_services одним запросом вставить |
||
| # что-то решила на всякий случай подготовить строки ) | ||
| values = buses_services.map { |arr| sprintf("(%s, %s)", arr[0], arr[1]) }.join(", ") | ||
| sql = "INSERT INTO buses_services (bus_id, service_id) VALUES #{values};" | ||
| ActiveRecord::Base.connection.execute(sql) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :file | ||
|
|
||
| def clean_database | ||
| City.delete_all | ||
| Bus.delete_all | ||
| Service.delete_all | ||
| Trip.delete_all | ||
| ActiveRecord::Base.connection.execute('delete from buses_services;') | ||
| end | ||
| end | ||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| <li>Сервисы в автобусе:</li> | ||
| <ul> | ||
| <% services.each do |service| %> | ||
| <%= render "service", service: service %> | ||
| <li><%= "#{service.name}" %></li> | ||
| <% end %> | ||
| </ul> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,5 +12,5 @@ | |
| <%= render "services", services: trip.bus.services %> | ||
| <% end %> | ||
| </ul> | ||
| <%= render "delimiter" %> | ||
| ==================================================== | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. можно оставить паршлы, применить рендеринг коллекций и там даже можно delimiter (spacer) задать параметром https://guides.rubyonrails.org/layouts_and_rendering.html#spacer-templates так будет не настолько тормозить, и при этом сохраняется удобство декомпозиции на паршлы |
||
| <% end %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| ## A. Импорт данных | ||
|
|
||
| Метрика: | ||
| - изначально замерила время на small.json - было около 15 секунд | ||
| - далее меряла small.json -> medium.json -> large.json | ||
|
|
||
| ### Подготовка | ||
|
|
||
| Сначала вынесла код в сервис-обжект (PORO) `TripsImporter` для удобства + написала базовый тест. | ||
| Далее стала смотреть, как оптимизировать. | ||
|
|
||
| ### Оптимизация - исследование | ||
|
|
||
| Сначала попробовала поэтапно идти: | ||
|
|
||
| - переписала на один insert trips, не трогая другие части; стало ещё медленнее - откатила пока | ||
| - добавляла уникальные индексы на справочные данные + использовала upsert, но во-первых не так уж сильно оптимизировало, во-вторых оказалось, что `upsert` не возвращает id, если нет вставки. Тест не отловил, т.к. там одна поездка. Дописывать тест не стала (поленилась), но в реальном приложении нужно. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. при вставке какого-то большого объёма данных может иметь смысл наоборот удалить индексы, вставить все данные, а потом уже пересоздать индексы заново |
||
| - попробовала рубипрофом попрофилировать, но такое себе - всё размазано, и так видно, что 100500 запросов идёт | ||
| - попробовала проверить, какая часть занимает много времени, комментируя куски кода и запуская, в принципе довольно наглядно. Понятно, что insert trips и sessions долго работает (ожидаемо) | ||
| - также смотрела рельсовые логи, видно, что также идёт 100500 мелких запросов | ||
| - также померяла, что сама загрузка (parse и обход) json занимает не так много времени, поэтому уж пару раз можно пройтись | ||
|
|
||
| ### Оптимизация | ||
|
|
||
| Решила сначала собрать справочные данные по городам, автобусам и тд, потом вставить справочные данные и запросить получившиеся id из бд и сформировать подходящие структуры данных для дальнейшего поиска при подготовке данных для trips. Попутно добавила уникальные индексы на `name` и тд. | ||
|
|
||
| Далее пройтись ещё раз, подготовить данные по поездкам и услугам автобусов, и уже вставить их отдельно. | ||
|
|
||
| Эта оптимизация была эффективной - файл large стал обрабатываться за ~ 3-3,35 секунды. | ||
|
|
||
| ``` | ||
| anna@vivosaurus:~/apps/rails-optimization-task3$ be rake reload_json[fixtures/large.json] | ||
| 3.356575947 | ||
| ``` | ||
|
|
||
| ```ruby | ||
| task :reload_json, [:file_name] => :environment do |_task, args| | ||
| start_time = Time.current | ||
|
|
||
| TripsImporter.call(args.file_name) | ||
|
|
||
| end_time = Time.current | ||
|
|
||
| p end_time - start_time | ||
| end | ||
| ``` | ||
|
|
||
| Решила рискнуть и попробовать на `1M.json`, получилось за 30 секунд. Как так? Все записи на месте в бд. | ||
|
|
||
| ## Б. Отображение расписаний | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Не помешало бы реквест-тест добавить, моя недоработка ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. в ПР видел ещё тесты на view, вот это крутотень конечно
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ну реквест- типа интеграционные, покроют и вью (в той мере, в к-й напишем :) |
||
|
|
||
| Изначально время: 8329сек | ||
|
|
||
| Сразу видим в rack-mini-profiler 1437 sql-запросов, делаем includes: | ||
| ```ruby | ||
| @trips = Trip.where(from: @from, to: @to).includes(bus: :services).order(:start_time) | ||
| ``` | ||
|
|
||
| Время: 3930 | ||
|
|
||
| Дальше видно, что очень много partials загружается: | ||
|
|
||
| - убрала `partial` `service` (незачем в отдельном файле рендерить одну строчку, это не бесплатно) | ||
|
|
||
| Время: 2336 | ||
|
|
||
| - убрала аналогично `partial` `delimiter` | ||
|
|
||
| Время: 632 | ||
|
|
||
| Решила, что этого хватит. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Аналогичное решение)) |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class AddUniqueIndexToServices < ActiveRecord::Migration[8.0] | ||
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index :services, :name, unique: true, algorithm: :concurrently | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class AddUniqueIndexToBusNumbers < ActiveRecord::Migration[8.0] | ||
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index :buses, :number, unique: true, algorithm: :concurrently | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class AddUniqueIndexToCities < ActiveRecord::Migration[8.0] | ||
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index :cities, :name, unique: true, algorithm: :concurrently | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class AddUniqueIndexToBusesServices < ActiveRecord::Migration[8.0] | ||
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index :buses_services, [:bus_id, :service_id], unique: true, algorithm: :concurrently | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3