Skip to content

Preventing duplication of game rating logs #1294

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.

Sign up for GitHub

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

Open
BElluu wants to merge 2 commits into
base: main
Choose a base branch
from
+26 −1
Open

Preventing duplication of game rating logs #1294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4479314
Preventing duplication of game rating logs
BElluu Jun 16, 2026
ab1efc1
fix credo
BElluu Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/teiserver/game/libs/match_rating_lib.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,10 @@ defmodule Teiserver.Game.MatchRatingLib do
|> Repo.transaction()
else
Multi.new()
|> Multi.insert_all(:insert_all, RatingLog, all_ratings)
|> Multi.insert_all(:insert_all, RatingLog, all_ratings,
on_conflict: :nothing,
conflict_target: [:match_id, :user_id, :rating_type_id]
)
|> Repo.transaction()
end
end
Expand Down
22 changes: 22 additions & 0 deletions priv/repo/migrations/20260616101633_unique_index_rating_logs.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
defmodule Teiserver.Repo.Migrations.UniqueIndexRatingLogs do
use Ecto.Migration

def up do
# Remove duplicate entries keeping the earliest
# (lowest id) per (match_id, user_id, rating_type_id)
execute("""
DELETE FROM teiserver_game_rating_logs
WHERE id NOT IN (
SELECT DISTINCT ON (match_id, user_id, rating_type_id) id
FROM teiserver_game_rating_logs
ORDER BY match_id, user_id, rating_type_id, id ASC
)
""")

create unique_index(:teiserver_game_rating_logs, [:match_id, :user_id, :rating_type_id])

@geekingfrog geekingfrog Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the SQL for this? I'm pretty sure this is going to acquire an exclusive lock on the entire table for the duration of the operation. This table has 13M rows at time of writing.
How long would the index creation take? If it's more than a couple of seconds, we need to change it and make it non blocking.

with postgres, you can create an index concurrently, and then create a unique constraint using the existing index.

@BElluu BElluu Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CREATE UNIQUE INDEX teiserver_game_rating_logs_match_id_user_id_rating_type_id_index
ON teiserver_game_rating_logs (match_id, user_id, rating_type_id);

Creating index for that many rows, can take some minutes.

So maybe we should firstly manualy delete duplicates

DO $$
DECLARE deleted integer;
BEGIN
  LOOP
    WITH dupes AS (
      SELECT id FROM (
        SELECT id, ROW_NUMBER() OVER (
          PARTITION BY match_id, user_id, rating_type_id
          ORDER BY id ASC
        ) rn
        FROM teiserver_game_rating_logs
      ) s WHERE s.rn > 1
      LIMIT 10000
    )
    DELETE FROM teiserver_game_rating_logs t USING dupes WHERE t.id = dupes.id;
    GET DIAGNOSTICS deleted = ROW_COUNT;
    EXIT WHEN deleted = 0;
    PERFORM pg_sleep(0.1);
  END LOOP;
END $$;

and then create index concurently out of transaction without lock

defmodule Teiserver.Repo.Migrations.UniqueIndexRatingLogs do
  use Ecto.Migration

  @disable_ddl_transaction true
  @disable_migration_lock true

  def up do
    create unique_index(
             :teiserver_game_rating_logs,
             [:match_id, :user_id, :rating_type_id],
             concurrently: true
           )
  end

  def down do
    drop unique_index(
           :teiserver_game_rating_logs,
           [:match_id, :user_id, :rating_type_id],
           concurrently: true
         )
  end
end

What do you think ?

end

def down do
drop unique_index(:teiserver_game_rating_logs, [:match_id, :user_id, :rating_type_id])
end
end
Loading