hangfire-postgres / Hangfire.PostgreSql

PostgreSql Storage Provider for Hangfire
Other
358 stars 132 forks source link

Issue with PostgreSqlObjectsInstaller not updating schema version on failed script execution #337

Closed yurii-hunter closed 9 months ago

yurii-hunter commented 9 months ago

Description: The PostgreSqlObjectsInstaller currently encounters an issue when executing the Install script. Although it successfully runs the script, it fails to update the schema version due to the WHERE ""version"" = @PreviousVersion condition.

Problem Scenario: Consider a situation where the current schema version is 10, and we are attempting to install version 11 using Install.v11.sql. If an exception occurs during the execution of this script, the schema.version is not set to 11.

Simultaneously, all subsequent scripts are executed, but their versions are not set because of the WHERE ""version"" = @PreviousVersion condition within the script:

UPDATE ""{schemaName}"".""schema"" SET ""version"" = @Version WHERE ""version"" = @PreviousVersion

As a result, even though we executed script 12, the database version remains at 10. Consequently, indexes, columns, and other elements are migrated without the corresponding version update.

Issue Impact: This creates a problem where, even if script 11 is successfully executed after a restart, the subsequent execution of script 12 encounters issues because the indexes from script 12 have already been applied.

Proposed Solution: To address this issue, we may need to revise the update condition within the script to ensure that even if a previous script fails, subsequent scripts update the version correctly.

azygis commented 9 months ago

The fact that it still continues migrations is straight bug. Are you sure it's still doing the subsequent upgrades after one fails?

yurii-hunter commented 9 months ago

@azygis well, Here we re-throw exception if it is not "version-already-applied" https://github.com/hangfire-postgres/Hangfire.PostgreSql/blob/master/src/Hangfire.PostgreSql/PostgreSqlObjectsInstaller.cs#L86C23-L86C23

And here we catch this exception, log and continue to the next iteration https://github.com/hangfire-postgres/Hangfire.PostgreSql/blob/master/src/Hangfire.PostgreSql/PostgreSqlObjectsInstaller.cs#L95C16-L95C16

azygis commented 9 months ago

Sorry, wasn't able to dig previously. Indeed it seems we do have a bug. Not super serious, but definitely something to fix soon.

Would you be able to open a PR? We certainly do not want to continue with any of the later migrations if anything prior failed. The only silent exception should be the missing resource (as is already), as then we reached the last version.

azygis commented 9 months ago

Thank you! I will try to remember to publish it tomorrow.

azygis commented 9 months ago

As expected, forgot about it during the day and then noticed the build was failing.

Fixed now, published moments ago. Thank you again!