[migrate.go#L740-L754] Migration logic can cause database errors

[lengpucheng]

Describe the Bug

migrate/migrate.go

Lines 740 to 754 in 511ae9f

	if err := m.databaseDrv.SetVersion(migr.TargetVersion, true); err != nil {
	return err
	}
	if migr.Body != nil {
	m.logVerbosePrintf("Read and execute %v\n", migr.LogString())
	if err := m.databaseDrv.Run(migr.BufferedBody); err != nil {
	return err
	}
	}
	// set clean state
	if err := m.databaseDrv.SetVersion(migr.TargetVersion, false); err != nil {
	return err
	}

The logic contained in the [migrate.go#L740-L754] code block may cause database exceptions. After the L740 is executed, if the database is interrupted due to network or other possible exceptions but the L746 has not executed SQL, an error will be reported and the database will be is set to dirty, which makes it impossible to continue the migration even after the subsequent network and connection return to normal
For example, in the case of containers or clusters, errors may occur due to network and container failures

Steps to Reproduce
Steps to reproduce the behavior:
This is a logical error. To reproduce, you need to manually control the database connection or debug:

1. The execution of 740L is completed, and the database has been changed
2. Disconnect the network, close the database connection or stop the database container
3. Continuously resume execution to simulate possible problems
4. The SQL has not yet executed 746L line err!=nil returns an error, and the database is marked as dirty
5. Restore the network or connect to re-execute the code, because the database has been marked as dirty and cannot be migrated normally

Expected Behavior
A retry mechanism should be added, or the 740L should be executed later, and the SQL should be executed first and then judge the execution result or error type to mark dirty

Migrate Version
The latest code (2022/08/01-03613f1) is still the same

Loaded Source Drivers
Logical errors are not involved

Loaded Database Drivers
Logical errors are not involved

Go Version
Logical errors are not involved

Stacktrace
Logical errors are not involved

Additional context
Logical errors are not involved

[dhui]

Setting the version and marking it as dirty before running migrations is a design choice which allows you to catch any errors. As the developer/operator, once the db migration is marked as dirty it is your job to figure out why the migration failed to apply and how best to move forward. There are many reasons why errors may occur. e.g. network failures, disk/db corruption, bad migrations
Simply retrying a migration may not be possible if the migration is not idempotent. Also, network failures may occur before the migration is attempted or after the migration has started running so it's not clear to migrate how best to proceed.

[lengpucheng]

Thank you, this is a problem when we use it as a database migration. After understanding your explanation, I understand this design model, and then I will improve our use plan. Thank you for your reply. I will close this issue