Skip to content
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

Оптимизация БД Сергеенков М.С. #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MihailSergeenkov
Copy link

Первым делом я написал тесты, чтобы не сломать существующую логику.
Далее с помощью PgHero я смог установить (точнее подтвердить), что выполняется очень много запросов в БД.
Далее я переписал импорт маршрутов, сопутствующие объекты я начал хранить в хэшах, в БД ходить только для создания, а создание маршрутов переделал на импорт, используя гем activerecord-import. Таким образом удалось уложиться в заданный бюджет и загрузить самый большой файл данных за 50 секунд.

Далее для оптимизации рендеринга страницы с маршрутами я начал дополнительно использовать гем rack-mini-profiler. В связке rack-mini-profiler и PgHero мне удалось уменьшить время рендеринга страницы с 16000мс до 2000мс.
С помощью PgHero я установил необходимость добавления нетерпеливой загрузки, что дало уменьшение времени загрузки до 6000мс.
А с помошью rack-mini-profiler я установил, что рендеринг паршиалов тоже нужно оптимизировать. Таким образом я смог загрузить страницу с более 1000 маршрутами за 2000мс.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Collaborator

@spajic spajic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve, но ещё на самом деле индексами можно сильно ускорить выборки

@@ -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.includes(bus: :services).where(from: @from, to: @to).order(:start_time)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -13,8 +13,13 @@ class Bus < ApplicationRecord
].freeze

has_many :trips
has_and_belongs_to_many :services, join_table: :buses_services
has_many :buses_services
has_many :services, through: :buses_services
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,78 @@
class ReloadJson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плюс за вынос логики в отдельный файл, но не очень понятное название.
Из названия непонятно что именно за json

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага, поспешил(

end

def call(file_name)
json = Oj.load(File.read(file_name))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так грузим сразу весь json в память, при большом файле потратим много оперативки, раздуем память процесса.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Точно...

@@ -2,14 +2,14 @@
<%= "Автобусы #{@from.name} – #{@to.name}" %>
</h1>
<h2>
<%= "В расписании #{@trips.count} рейсов" %>
<%= "В расписании #{@trips.size} рейсов" %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -2,4 +2,6 @@
# For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html
get "/" => "statistics#index"
get "автобусы/:from/:to" => "trips#index"

mount PgHero::Engine, at: "pghero"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,5 @@
class AddUniqIndexToCityName < ActiveRecord::Migration[5.2]
def change
add_index :cities, :name, unique: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В данном случае не важно, но напоминаю про опцию concurrently

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага, спасибо

given(:to_city) { FactoryBot.create(:city) }
given!(:trips) { FactoryBot.create_list(:trip, 3, from_id: from_city.id, to_id: to_city.id, bus_id: bus.id) }

scenario 'Пользователь открывает список маршрутов' do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants