jkingster / QuickTicket

0 stars 0 forks source link

Bug: Database Backup Failure #62

Closed jkingster closed 1 month ago

jkingster commented 1 month ago

Version: ~<= 1.1.0.1, observed from 1.0.3 to 1.1.0.1

Description: When working on 1.1.0, I noticed that when a new migration is implemented for DB structural changes -- a backup is created and sent over to the ~\backup folder. However upon randomly analyzing the backup, I noticed the size is 0 KB when the original copy was around ~72 KB.

I found this odd that the backup was deemed "successful", but the actual internal contents of the DB were not being transferred over. I tested manually doing a backup in System > Configuration > Backup Database, and this successfully copied over all DB contents matching the ~72 KB size.

I can only assume it's a concurrency or I/O issue. But definitely a critical bug as backups are important pre-migration, incase the migration fails or something else internally goes wrong- you can uninstall and revert back to the old DB.

My current resolution when installing 1.1.0.1 is to beware, and to create a manual backup before starting the installation at all incase things inadvertently fail.

jkingster commented 1 month ago

The culprit seemed to be due to concurrency issues or I/O deadlocks. It would copy the file over but the source would be marked as 0 KB when copied. After copying I instilled additional size checking of each file source vs destination, and it would still fail because they weren't the same size.

Instead, I read up on Flyway and apparently you can do progmatic callbacks- so that's what I'm opting for instead to negate this backup measure being called before any migrations- Flyway will instead handle it because they have an event named BEFORE_EACH_MIGRATE which will call this backup process before the migration starts.

If the backup fails I will then prompt for the continue process, or exit to shutdown the application without migrating. Seems to be the most convenient workaround that actually involves Flyway/

jkingster commented 1 month ago
 private static class BackupCallback implements Callback {

        private final String databaseUrl;

        public BackupCallback(final String databaseUrl) {
            this.databaseUrl = databaseUrl;
        }

        @Override public boolean supports(Event event, Context context) {
            return event == Event.BEFORE_EACH_MIGRATE;
        }

        @Override public boolean canHandleInTransaction(Event event, Context context) {
            return true;
        }

        @Override public void handle(Event event, Context context) {
            if (event == Event.BEFORE_EACH_MIGRATE) {
                final DatabaseBackup databaseBackup = new DatabaseBackup(new File(databaseUrl));
                if (!databaseBackup.backup()) {
                    final ButtonType migrate = new ButtonType("Migrate", ButtonBar.ButtonData.OK_DONE);
                    final ButtonType shutdown = new ButtonType("Shutdown", ButtonBar.ButtonData.NO);
                    Alerts.get().showWarningConfirmation(
                            "WARNING!", "Database Backup Failed", WARNING_MESSAGE, migrate, shutdown
                    ).ifPresent(type -> {
                        if (type == shutdown) {
                            QuickTicket.getInstance().shutdown();
                        }
                    });
                }
            }
        }

        @Override public String getCallbackName() {
            return BackupCallback.class.getName();
        }
    }