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

FIX: Split backfill into separate migrations to use independent transactions #1063

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

romanrizzi
Copy link
Member

No description provided.

@romanrizzi romanrizzi force-pushed the split_embedding_table_backfill branch from 99279cd to 0bb8f71 Compare January 14, 2025 16:17
@romanrizzi
Copy link
Member Author

@davidtaylorhq - CI failed because the post migration that drops the table ran before the new ones. I modified the timestamps to just after the new tables creation. I think this won't be an issue because of how migrations are applied during deploy, right?

@davidtaylorhq
Copy link
Member

davidtaylorhq commented Jan 14, 2025

It depends on the deployment. Some environments (i.e. most self-hosters, and all development environments) will run all migrations in numerical order, regardless of pre/post. Other environments will run pre-migrations first, then make the app live, then run post-migrations.

So, best strategy is to make the current post-migration a no-op, and introduce a new one with a higher number

@romanrizzi romanrizzi force-pushed the split_embedding_table_backfill branch from 0bb8f71 to b370de5 Compare January 14, 2025 16:24
@romanrizzi romanrizzi merged commit 356ea77 into main Jan 14, 2025
6 checks passed
@romanrizzi romanrizzi deleted the split_embedding_table_backfill branch January 14, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants