From 2c82cb48a619ae42d6ae556ea9aef91c1d3d5896 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 28 Dec 2025 17:04:00 +0100 Subject: [PATCH] Add new indicies to improve performance and remove unused ones to optimize database. --- app/controllers/exports_controller.rb | 2 +- app/controllers/imports_controller.rb | 1 + app/models/user.rb | 3 +- app/serializers/stats_serializer.rb | 3 +- app/services/countries_and_cities.rb | 11 +++-- .../reverse_geocoding/places/fetch_data.rb | 20 ++++++++- .../map/maplibre/_settings_panel.html.erb | 2 +- .../20251228000000_remove_unused_indexes.rb | 19 +++++++++ .../20251228100000_add_performance_indexes.rb | 30 ++++++++++++++ db/schema.rb | 41 +++++++++++++------ 10 files changed, 110 insertions(+), 22 deletions(-) create mode 100644 db/migrate/20251228000000_remove_unused_indexes.rb create mode 100644 db/migrate/20251228100000_add_performance_indexes.rb diff --git a/app/controllers/exports_controller.rb b/app/controllers/exports_controller.rb index 0c59e1bf..3b669425 100644 --- a/app/controllers/exports_controller.rb +++ b/app/controllers/exports_controller.rb @@ -7,7 +7,7 @@ class ExportsController < ApplicationController before_action :set_export, only: %i[destroy] def index - @exports = current_user.exports.order(created_at: :desc).page(params[:page]) + @exports = current_user.exports.with_attached_file.order(created_at: :desc).page(params[:page]) end def create diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 77b75251..29b84530 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -14,6 +14,7 @@ class ImportsController < ApplicationController def index @imports = policy_scope(Import) .select(:id, :name, :source, :created_at, :processed, :status) + .with_attached_file .order(created_at: :desc) .page(params[:page]) end diff --git a/app/models/user.rb b/app/models/user.rb index 6a591451..8c15c6be 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -73,7 +73,8 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def total_reverse_geocoded_points - points.where.not(reverse_geocoded_at: nil).count + # Use cached count from StatsQuery instead of direct COUNT query + StatsQuery.new(self).points_stats[:geocoded] end def total_reverse_geocoded_points_without_data diff --git a/app/serializers/stats_serializer.rb b/app/serializers/stats_serializer.rb index bd3939fb..cc7ff569 100644 --- a/app/serializers/stats_serializer.rb +++ b/app/serializers/stats_serializer.rb @@ -27,7 +27,8 @@ class StatsSerializer end def reverse_geocoded_points - user.points.reverse_geocoded.count + # Use cached count from StatsQuery instead of direct COUNT query + StatsQuery.new(user).points_stats[:geocoded] end def yearly_stats diff --git a/app/services/countries_and_cities.rb b/app/services/countries_and_cities.rb index 333cb7ac..3fa7669a 100644 --- a/app/services/countries_and_cities.rb +++ b/app/services/countries_and_cities.rb @@ -9,9 +9,12 @@ class CountriesAndCities end def call + # Use attribute access directly to avoid N+1 on country association + # when country_name column is nil and Point#country_name method + # falls back to country&.name points - .reject { |point| point.country_name.nil? || point.city.nil? } - .group_by(&:country_name) + .reject { |point| point[:country_name].nil? || point[:city].nil? } + .group_by { |point| point[:country_name] } .transform_values { |country_points| process_country_points(country_points) } .map { |country, cities| CountryData.new(country: country, cities: cities) } end @@ -22,7 +25,7 @@ class CountriesAndCities def process_country_points(country_points) country_points - .group_by(&:city) + .group_by { |point| point[:city] } .transform_values { |city_points| create_city_data_if_valid(city_points) } .values .compact @@ -31,7 +34,7 @@ class CountriesAndCities def create_city_data_if_valid(city_points) timestamps = city_points.pluck(:timestamp) duration = calculate_duration_in_minutes(timestamps) - city = city_points.first.city + city = city_points.first[:city] points_count = city_points.size build_city_data(city, points_count, timestamps, duration) diff --git a/app/services/reverse_geocoding/places/fetch_data.rb b/app/services/reverse_geocoding/places/fetch_data.rb index c297599e..54c30dd7 100644 --- a/app/services/reverse_geocoding/places/fetch_data.rb +++ b/app/services/reverse_geocoding/places/fetch_data.rb @@ -138,8 +138,24 @@ class ReverseGeocoding::Places::FetchData Place.insert_all(place_attributes) end - # Individual updates for existing places - places_to_update.each(&:save!) if places_to_update.any? + # Batch update existing places to avoid N+1 + if places_to_update.any? + update_attributes = places_to_update.map do |place| + { + id: place.id, + name: place.name, + latitude: place.latitude, + longitude: place.longitude, + lonlat: place.lonlat, + city: place.city, + country: place.country, + geodata: place.geodata, + source: place.source, + updated_at: Time.current + } + end + Place.upsert_all(update_attributes, unique_by: :id) + end end def build_point_coordinates(coordinates) diff --git a/app/views/map/maplibre/_settings_panel.html.erb b/app/views/map/maplibre/_settings_panel.html.erb index e5069c75..59d593b2 100644 --- a/app/views/map/maplibre/_settings_panel.html.erb +++ b/app/views/map/maplibre/_settings_panel.html.erb @@ -72,7 +72,7 @@ data-maps--maplibre-target="searchInput" autocomplete="off" /> - diff --git a/db/migrate/20251228000000_remove_unused_indexes.rb b/db/migrate/20251228000000_remove_unused_indexes.rb new file mode 100644 index 00000000..3c5f57e3 --- /dev/null +++ b/db/migrate/20251228000000_remove_unused_indexes.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class RemoveUnusedIndexes < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + remove_index :points, :geodata, algorithm: :concurrently, if_exists: true + remove_index :points, %i[latitude longitude], algorithm: :concurrently, if_exists: true + remove_index :points, :altitude, algorithm: :concurrently, if_exists: true + remove_index :points, :city, algorithm: :concurrently, if_exists: true + remove_index :points, :country_name, algorithm: :concurrently, if_exists: true + remove_index :points, :battery_status, algorithm: :concurrently, if_exists: true + remove_index :points, :connection, algorithm: :concurrently, if_exists: true + remove_index :points, :trigger, algorithm: :concurrently, if_exists: true + remove_index :points, :battery, algorithm: :concurrently, if_exists: true + remove_index :points, :country, algorithm: :concurrently, if_exists: true + remove_index :points, :external_track_id, algorithm: :concurrently, if_exists: true + end +end diff --git a/db/migrate/20251228100000_add_performance_indexes.rb b/db/migrate/20251228100000_add_performance_indexes.rb new file mode 100644 index 00000000..926463c1 --- /dev/null +++ b/db/migrate/20251228100000_add_performance_indexes.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class AddPerformanceIndexes < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + # Query: SELECT * FROM users WHERE api_key = $1 + add_index :users, :api_key, + algorithm: :concurrently, + if_not_exists: true + + # Query: SELECT id FROM users WHERE status = $1 + add_index :users, :status, + algorithm: :concurrently, + if_not_exists: true + + # Query: SELECT DISTINCT city FROM points WHERE user_id = $1 AND city IS NOT NULL + add_index :points, %i[user_id city], + name: 'idx_points_user_city', + algorithm: :concurrently, + if_not_exists: true + + # Query: SELECT 1 FROM points WHERE user_id = $1 AND visit_id IS NULL AND timestamp BETWEEN... + add_index :points, %i[user_id timestamp], + name: 'idx_points_user_visit_null_timestamp', + where: 'visit_id IS NULL', + algorithm: :concurrently, + if_not_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 089b01c7..e7c2c4b1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do +ActiveRecord::Schema[8.0].define(version: 2025_12_28_100000) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -80,6 +80,29 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do create_table "data_migrations", primary_key: "version", id: :string, force: :cascade do |t| end + create_table "digests", force: :cascade do |t| + t.bigint "user_id", null: false + t.integer "year", null: false + t.integer "period_type", default: 0, null: false + t.bigint "distance", default: 0, null: false + t.jsonb "toponyms", default: {} + t.jsonb "monthly_distances", default: {} + t.jsonb "time_spent_by_location", default: {} + t.jsonb "first_time_visits", default: {} + t.jsonb "year_over_year", default: {} + t.jsonb "all_time_stats", default: {} + t.jsonb "sharing_settings", default: {} + t.uuid "sharing_uuid" + t.datetime "sent_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["period_type"], name: "index_digests_on_period_type" + t.index ["sharing_uuid"], name: "index_digests_on_sharing_uuid", unique: true + t.index ["user_id", "year", "period_type"], name: "index_digests_on_user_id_and_year_and_period_type", unique: true + t.index ["user_id"], name: "index_digests_on_user_id" + t.index ["year"], name: "index_digests_on_year" + end + create_table "exports", force: :cascade do |t| t.string "name", null: false t.string "url" @@ -226,18 +249,8 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do t.string "country_name" t.boolean "raw_data_archived", default: false, null: false t.bigint "raw_data_archive_id" - t.index ["altitude"], name: "index_points_on_altitude" - t.index ["battery"], name: "index_points_on_battery" - t.index ["battery_status"], name: "index_points_on_battery_status" - t.index ["city"], name: "index_points_on_city" - t.index ["connection"], name: "index_points_on_connection" - t.index ["country"], name: "index_points_on_country" t.index ["country_id"], name: "index_points_on_country_id" - t.index ["country_name"], name: "index_points_on_country_name" - t.index ["external_track_id"], name: "index_points_on_external_track_id" - t.index ["geodata"], name: "index_points_on_geodata", using: :gin t.index ["import_id"], name: "index_points_on_import_id" - t.index ["latitude", "longitude"], name: "index_points_on_latitude_and_longitude" t.index ["lonlat", "timestamp", "user_id"], name: "index_points_on_lonlat_timestamp_user_id", unique: true t.index ["lonlat"], name: "index_points_on_lonlat", using: :gist t.index ["raw_data_archive_id"], name: "index_points_on_raw_data_archive_id" @@ -245,10 +258,11 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do t.index ["reverse_geocoded_at"], name: "index_points_on_reverse_geocoded_at" t.index ["timestamp"], name: "index_points_on_timestamp" t.index ["track_id"], name: "index_points_on_track_id" - t.index ["trigger"], name: "index_points_on_trigger" + t.index ["user_id", "city"], name: "idx_points_user_city" t.index ["user_id", "country_name"], name: "idx_points_user_country_name" t.index ["user_id", "reverse_geocoded_at"], name: "index_points_on_user_id_and_reverse_geocoded_at", where: "(reverse_geocoded_at IS NOT NULL)" t.index ["user_id", "timestamp", "track_id"], name: "idx_points_track_generation" + t.index ["user_id", "timestamp"], name: "idx_points_user_visit_null_timestamp", where: "(visit_id IS NULL)" t.index ["user_id", "timestamp"], name: "index_points_on_user_id_and_timestamp", order: { timestamp: :desc } t.index ["user_id"], name: "index_points_on_user_id" t.index ["visit_id"], name: "index_points_on_visit_id" @@ -373,9 +387,11 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do t.string "utm_campaign" t.string "utm_term" t.string "utm_content" + t.index ["api_key"], name: "index_users_on_api_key" t.index ["email"], name: "index_users_on_email", unique: true t.index ["provider", "uid"], name: "index_users_on_provider_and_uid", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true + t.index ["status"], name: "index_users_on_status" end add_check_constraint "users", "admin IS NOT NULL", name: "users_admin_null", validate: false @@ -400,6 +416,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_26_170919) do add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "areas", "users" + add_foreign_key "digests", "users" add_foreign_key "families", "users", column: "creator_id" add_foreign_key "family_invitations", "families" add_foreign_key "family_invitations", "users", column: "invited_by_id"