-
Notifications
You must be signed in to change notification settings - Fork 12
optimization #2
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?
optimization #2
Conversation
spajic
left a comment
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.
Хорошая работа, респект за тесты!
| @@ -0,0 +1,10 @@ | |||
| class AddIndexesToTrip < ActiveRecord::Migration[5.2] | |||
| disable_ddl_transaction! | |||
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.
👍
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index :trips, [:from_id, :to_id], algorithm: :concurrently |
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.
👍 Плюс за concurrently
| end | ||
|
|
||
| it "doesn't send unnecessary requests to db" do | ||
| expect { get :index, params: { from: trip.from.name , to: trip.to.name } }.not_to exceed_query_limit(2) |
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.
👍 за 'rspec-sqlimit'
|
|
||
| puts "Controller time: #{time}" | ||
|
|
||
| expect(time).to be < 0.006 |
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.
👍
| end | ||
| let!(:trip) { Trip.preload(:from, :to).take } | ||
|
|
||
| it 'correct trips count' do |
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.
Хорошо читается, когда получается согласованное предложение, например it 'has correct trips count'
| expect(assigns(:trips).size).to eq(9) | ||
| expect(assigns(:from).name).to eq(trip.from.name) | ||
| expect(assigns(:to).name).to eq(trip.to.name) | ||
| 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.
Лишний отступ
| @@ -0,0 +1,44 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| require 'rails_helper' | |||
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.
requireы было бы лучше вынести в rails_helper и реквайрить уже его
Проблема импорта данных
импорт из файла
large.jsonбыл не возможен т.е. занимал огромное количество временидля feedback loop был выбран файл
small.json, который исполнялся 7.5 сек.Анализ логов выявил, что операции insert быстрые, но их слишком много.
Для вставки batches, использовал gem
activerecord-import, позволяющий вставлять пачками, используя при это active record валидации, время уменьшилось до 3 сек, но еще оставлось много запросов к базе типаfind_by_name, для уменшения кол-ва запросов, было принято решение кешировать объекты в памяти, что позволило скротить время до 0.5 сек.Файл
large.jsonстал загружаться за приемлемое - 30 сек времяПроблема отображения расписаний
После загрузки
large.jsonв базу, оказалось что страницаавтобусы/Самара/Москвазагружается очень долго, результат загрузки(для тестирования использовалk6)Было выявлено 2 основные проблемы:
tripsвызывалисьbusиservices, добавленeager_loadРезультат:
Доп оптимзации:
Кешировние partial
_serviceУдаление partial
_delimetr