Skip to content

db: drop duplicate access_tokens hash index - #3408

Merged
ValentaTomas merged 1 commit into
mainfrom
drop-dup-access-tokens-hash-index
Jul 27, 2026
Merged

ValentaTomas merged 1 commit into
mainfrom
drop-dup-access-tokens-hash-index

Conversation

@ValentaTomas

@ValentaTomas ValentaTomas commented Jul 27, 2026

Copy link
Copy Markdown
Member

access_tokens.access_token_hash carries two identical unique indexes: the UNIQUE constraint's own and a standalone copy. Identical indexes are interchangeable to the planner, so lookups continue on the constraint's index and uniqueness is untouched; every token write stops maintaining the second copy. The standalone twin is the droppable one — the constraint-backed index refuses DROP INDEX, making a mixed-up drop fail loud instead of silently.

Schema proof, both definitions in this repo: the standalone unique index (20250825102440 L9-L10) and the column's UNIQUE constraint that already carries its own index (20250211160814 L12) — identical key (access_token_hash). Background: PostgreSQL wiki — duplicate indexes.

@cla-bot cla-bot Bot added the cla-signed label Jul 27, 2026
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Online index drop on an auth-related table; uniqueness and lookups remain on the constraint index, with low functional risk if the correct standalone index is dropped.

Overview
Removes the redundant unique index idx_access_tokens_access_token_hash on access_tokens.access_token_hash because the column’s UNIQUE constraint already provides an equivalent index, so token writes no longer maintain a duplicate index while lookups and uniqueness stay the same. The migration uses DROP INDEX CONCURRENTLY with a bounded statement_timeout and restores the migrator’s 3h timeout afterward; rollback drops any invalid leftover index before recreating the standalone index concurrently.

Reviewed by Cursor Bugbot for commit 93e7824. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
3514 5 3509 7
View the top 3 failed test(s) by shortest run time
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_1_lists_root_directory
Stack Traces | 0.01s run time
=== RUN   TestListDir/depth_1_lists_root_directory
=== PAUSE TestListDir/depth_1_lists_root_directory
=== CONT  TestListDir/depth_1_lists_root_directory
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_1_lists_root_directory
--- FAIL: TestListDir/depth_1_lists_root_directory (0.01s)
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
Stack Traces | 0.01s run time
=== RUN   TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
=== PAUSE TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
=== CONT  TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
--- FAIL: TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory) (0.01s)
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_0_lists_only_root_directory
Stack Traces | 0.02s run time
=== RUN   TestListDir/depth_0_lists_only_root_directory
=== PAUSE TestListDir/depth_0_lists_only_root_directory
=== CONT  TestListDir/depth_0_lists_only_root_directory
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_0_lists_only_root_directory
--- FAIL: TestListDir/depth_0_lists_only_root_directory (0.02s)
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir
Stack Traces | 0.41s run time
=== RUN   TestListDir
=== PAUSE TestListDir
=== CONT  TestListDir
--- FAIL: TestListDir (0.41s)
View the full list of 1 ❄️ flaky test(s)
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_3_lists_all_directories_and_files

Flake rate in main: 30.77% (Passed 9 times, Failed 4 times)

Stack Traces | 0.01s run time
=== RUN   TestListDir/depth_3_lists_all_directories_and_files
=== PAUSE TestListDir/depth_3_lists_all_directories_and_files
=== CONT  TestListDir/depth_3_lists_all_directories_and_files
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_3_lists_all_directories_and_files
--- FAIL: TestListDir/depth_3_lists_all_directories_and_files (0.01s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@ValentaTomas
ValentaTomas marked this pull request as ready for review July 27, 2026 04:13

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — this correctly identifies and drops the redundant standalone index, keeping the UNIQUE constraint's own index intact.

Extended reasoning...

Overview

Single new migration file that drops idx_access_tokens_access_token_hash, a standalone unique index on access_tokens.access_token_hash. I verified in the migration history that this index is genuinely redundant: 20250211160814_add_token_hashes.sql added the column as TEXT UNIQUE, which creates an implicit unique index (access_tokens_access_token_hash_key), and a later migration (20250825102440_add_hash_indexes.sql) added the standalone idx_access_tokens_access_token_hash unique index on the same column — an unintentional duplicate. This PR removes the later, non-constraint-backed duplicate, which is the correct one to drop (the constraint-backed index cannot be removed via DROP INDEX at all, so a mixed-up target would simply fail rather than silently corrupt anything).

Security risks

None. This is a pure schema/index change with no application logic, query, or auth-path modifications. Uniqueness enforcement and hash lookups continue via the constraint's index.

Level of scrutiny

Low. This is a mechanical, well-reasoned online index drop (DROP INDEX CONCURRENTLY, no transaction wrapper, bounded statement_timeout) with a symmetric and correct down migration (clears any leftover INVALID index before recreating). The change is self-contained to one file and matches established patterns in this repo's migration history.

Other factors

Codecov reports full coverage on the change; Cursor Bugbot's own summary corroborates the redundant-index analysis. No outstanding review comments to address.

@ValentaTomas
ValentaTomas merged commit 9e91fba into main Jul 27, 2026
43 checks passed
@ValentaTomas
ValentaTomas deleted the drop-dup-access-tokens-hash-index branch July 27, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants