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

Statement-level granularity: Infinitely pending migration #2455

Open
ruslic19 opened this issue Jan 17, 2024 · 1 comment
Open

Statement-level granularity: Infinitely pending migration #2455

ruslic19 opened this issue Jan 17, 2024 · 1 comment

Comments

@ruslic19
Copy link

ruslic19 commented Jan 17, 2024

As described in atlas documentation here, migrations can partially fail and in provided there case in order to fix it, we add one more statement to our migration and then rerun it again.

Expected:
There is atlas migrate status which returns amount of pending migrations and when let's say our last migration partially fails, status command returns us pending: 1 which is supposed to become 0 when we will successfully fix and execute our partially failed migration.

Actual:
When our migration partially fails, we get pending: 1 as expected, but then when we fix it and execute it again, even if it executes successfully, pending remains to be 1, because of this, we can now execute our migrations multiple times and it never tells us that there is no migrations to execute anymore.

Steps to reproduce:
In general you can even use guide from statement level granularity itself or you can do it like this

  • Declaratively define users table schema with nullable name
  • Generate migration from defined schema and execute it
  • Insert couple of rows into users table with name being null
  • Change users table schema to have name NOT NULL
  • Generate migration from updated schema and try to execute it
  • Migration will be marked as partially failed, because table already contains null values
  • Manually edit migration to add sql statement to update all nulls with some default value before altering table
  • Recalculate hash of the migrations directory
  • Execute migration again, now it succeeds, but migration will still be marked as pending
  • Now you can try to get migration status or execute migrations again and it will always say that there is 1 pending migration
  • atlas_schema_revisions table now has total = 1 and applied = 2 and this is the cause of the issue

Reason:
As I understand it is related to the fact that we determine if migration is pending by comparing applied statements to the total statements in a given migration. Migration is considered to be complete only if amount of applied statements strictly equals to amount of total statements in the migration. For example, here in the code.

Whereas, in this case, when we update our migration to add new statement and execute it now successfully, row in the table atlas_schema_revisions responsible for this migration now contains total=value_before_we_added_new_statement and applied=all_executed_statements_including_new_one, so it results in something like this: total=1 applied=2.

Of course, we could just change strict equality to >= but this would cause issues in case when fixed migration partially fails again, so instead, when we execute migration, we should also update total to keep it up to date. Currently during apply migration process, when we find existing revision, we don't update it's total, but only executed at and operation version.

Suggested Fix: #2456
It is possible that this fix can cause some unexpected side effects, because it was tested only using use case described in this issue

ruslic19 added a commit to ruslic19/atlas that referenced this issue Jan 17, 2024
The reason for this is that users are required to edit existing migration files to resolve partial failure issues. Without keeping total in sync, such migration would be considered still pending, because total != applied.
Issue: ariga#2455
ruslic19 added a commit to ruslic19/atlas that referenced this issue Jan 17, 2024
The reason for this is that users are required to edit existing migration files to resolve partial failure issues. Without keeping total in sync, such migration would be considered still pending, because total != applied.
Issue: ariga#2455
@ruslic19
Copy link
Author

Suggested Fix: #2456
It is possible that this fix can cause some unexpected side effects, because it was tested only using use case described in this issue

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

No branches or pull requests

1 participant