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

Drop invalid index in safe_add_index #241

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

Conversation

dgsuarez
Copy link

@dgsuarez dgsuarez commented Sep 7, 2023

Hi! Saw this TODO in the code, we've been running something similar in our codebase, so better contribute it back :)

Allow safe_add_index to be seamlessly retried.

When the add_index...concurrently fails, the index is left in Postgres with INVALID status.

The new code drops the index if it's found and invalid. With this, if the migration fails because of a lock timeout for example, on retry it can work. Without it the retries always fail until you manually drop the index.

@dgsuarez dgsuarez marked this pull request as ready for review September 7, 2023 02:42
@dgsuarez
Copy link
Author

dgsuarez commented Sep 8, 2023

Small update to fix the errors in pre-6.1 rails versions

@ermolaev
Copy link

@ankane please show tis PR

when enable Lock Timeout Retries, possible situation:

-- add_index(:users, :email_last_open_at, {:algorithm=>:concurrently, :using=>:brin})
-- Lock timeout. Retrying in 10 seconds...
-- add_index(:users, :email_last_open_at, {:algorithm=>:concurrently, :using=>:brin})
rake aborted!
StandardError: An error has occurred, all later migrations canceled:
 
PG::DuplicateTable: ERROR:  relation "index_users_on_email_last_open_at" already exists

SELECT * FROM pg_class, pg_index WHERE pg_index.indisvalid = false AND pg_index.indexrelid = pg_class.oid;

oid 7411974
relname index_users_on_email_last_open_at

We must run index deletion before re-creation

remove_index :users, :email_last_open_at, if_exists: true

@chaadow
Copy link
Contributor

chaadow commented Aug 30, 2024

@ankane For my case, this pull request is helpful because when indices are INVALID rails does not add the CREATE UNIQUE INDEX inside db/structure.sql

which in CI would not generate a proper database and make the tests fail ( and in a team of many developers, this creates issues where structure.sql differ a lot from one developer ( who does not have an invaild index ) to another who has

the solution is to manually remove the index from psql or in some other way, but it would be great to have this PR merged to automatically handle the case

Thanks!

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.

3 participants