Skip to content

Conversation

@smudge
Copy link
Member

@smudge smudge commented Dec 10, 2025

This allows for proactively adding indexes out of band, so that the migrator will no-op and complete quickly. For very large tables this may be necessary to stay within constraints of synchronous deployment steps (e.g. a timeout on migration processes).

I also adjusted test logger setup so that it relies entirely on log levels, and will log out migration warnings even if debug logging is turned off for the rest of the test suite.

/no-platform

@smudge smudge requested a review from effron December 10, 2025 19:09
execute("SET statement_timeout TO #{pg_seconds(statement_timeout) || 'DEFAULT'};")
execute("SET lock_timeout TO #{pg_seconds(lock_timeout) || 'DEFAULT'};")
when 'MySQL', 'MariaDB'
when "MySQL", "Mysql2"
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this change about?

Copy link
Member Author

Choose a reason for hiding this comment

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

Until I enabled "warn" logs for the migration phase of testing, I didn't notice the fact that we weren't applying statement timeouts to mysql, because "Mysql2" is the actual adapter name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could probably use a harder testing constriant/guarantee there, but there's so much of this new migration/DB-testing layer that I want to extract & more directly test as its own thing 🤷

Copy link
Contributor

@effron effron left a comment

Choose a reason for hiding this comment

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

domainLGTM

@smudge smudge merged commit 07adf00 into Betterment:main Dec 10, 2025
25 checks passed
@smudge smudge deleted the smudge/upsert-optimization branch December 10, 2025 20:52
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.

2 participants