indexer: DB reset to handle incomplete migration history #19019
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@gegaowp is attempting to deploy a commit to the Mysten Labs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
I'm not a huge fan of this approach -- it seems to be baking in implementation details from diesel into our own code.
Am I right in thinking that this is just for the purposes of CI? If so, can we use diesel's own diesel migration etc to achieve the same effect?
not only for CI but for all DB reset env like staging and devnet too. Counting on migrations file is fragile while we are making changes in a breaking / non-incremental way, for example if we change a table name from table_a to table_b, then next time if we do db-reset by reverting all migrations, table_a will not be dropped as this is even worse when we have inconsistency between migration history in __diesel_schema_migrations and schema itself, which is what I saw today in CI and that caused failures. Not sure where the inconsistency was introduced but here a total reset instead of reverting all migrations is necessary afaict. |
gegaowp
force-pushed
the
db-reset-fix-again
branch
from
15e88ec
to
f701091
Compare
|
Is this not another example of what happens if you work against the intended usage of diesel's migration feature? I.e. if we did not modify migrations after we checked them in and only added new migrations in response to changes, could we then use the diesel CLI for this? IIUC we want to eventually have a clean starting point for the schema once it is settled, and that is why we are going back and modifying migration files, but can we get the best of both worlds by using migrations as intended until we are happy with the schema, then squashing everything down into the final schema files as a single breaking change? |
this is a fair point, but now the DB reset is solved by this pr, so we can squash the changes on each migration changes and that leaves the schema always compact and intuitive. I am open to this why-not-both idea tho if more relevant issues pop up and requires more baking. |
…19019) ## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
…19019) ## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
|
Yes, this is okay for us to unblock this release, but is not a long-term solution. We are only making things more brittle by relying on diesel's implementation details. I have added this as one of the topics for design discussions (although we have quite a few of these to get through, so maybe not for the coming Monday). |
…19019) ## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
…19019) ## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
## Description #19019 ## Test plan #19019 --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
…19019) ## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
## Description previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete. I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of ``` cannot drop the currently open database ``` thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL. ## Test plan 1. diesel reset on latest main 2. cherry-pick this pr to 1.31 and 1.30, run cmd below and make sure that DB are indeed reset ``` DB_POOL_SIZE=10 cargo run --bin sui-indexer -- --db-url =DB_URL --reset-db ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
Description
previous fix did not work well when migration run history in __diesel_schema_migrations table is not complete.
I also considered dropping the database as we usually do locally, but a connection cannot drop the database in its own connection and ended up having errors of
thus this pr drops all tables, functions and procedures explicitly for PG and tables for MySQL as we now only have tables in MySQL.
Test plan
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.