From faede65d8d66347fee0952d71c27ebdf05867752 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Mon, 14 Apr 2025 12:04:00 +0200 Subject: [PATCH] Changed transactions to use a wrapper that ensures a transaction is allocated. --- .../Library/Main/Database/ExtensionMethods.cs | 17 ++++++++++++ .../Main/Database/LocalBugReportDatabase.cs | 2 +- .../Library/Main/Database/LocalDatabase.cs | 10 +++---- .../Main/Database/LocalListChangesDatabase.cs | 2 +- .../Main/Database/LocalRepairDatabase.cs | 10 +++---- .../Main/Database/LocalRestoreDatabase.cs | 6 ++--- .../Main/Database/ReusableTransaction.cs | 4 +-- .../Library/Main/Operation/CompactHandler.cs | 4 +-- .../Library/Main/Operation/DeleteHandler.cs | 2 +- .../Main/Operation/PurgeFilesHandler.cs | 2 +- .../Library/Main/Operation/RepairHandler.cs | 2 +- .../Library/RestAPI/Database/Connection.cs | 26 +++++++++---------- 12 files changed, 52 insertions(+), 35 deletions(-) diff --git a/Duplicati/Library/Main/Database/ExtensionMethods.cs b/Duplicati/Library/Main/Database/ExtensionMethods.cs index aa6364103..df735491d 100644 --- a/Duplicati/Library/Main/Database/ExtensionMethods.cs +++ b/Duplicati/Library/Main/Database/ExtensionMethods.cs @@ -493,6 +493,23 @@ public static class ExtensionMethods return cmd; } + /// + /// Creates the transaction for the given connection and executes a "BEGIN IMMEDIATE" command to ensure that the transaction can be comitted or rolled back. + /// This works around a quirk in SQLite where the transaction is not initialized until the first command is executed. + /// This means that if the transaction is rolled back or committed with no commands executed, it will fail with an exception. + /// + /// The connection to create the transaction on + /// The transaction + public static IDbTransaction BeginTransactionSafe(this IDbConnection self) + { + var transaction = self.BeginTransaction(); + using (var cmd = self.CreateCommand(transaction, "BEGIN IMMEDIATE;")) + cmd.ExecuteNonQuery(); + + return transaction; + } + + /// /// Creates a command with the given command string, and adds parameters to fit the input /// diff --git a/Duplicati/Library/Main/Database/LocalBugReportDatabase.cs b/Duplicati/Library/Main/Database/LocalBugReportDatabase.cs index ff39492c7..778bed90b 100644 --- a/Duplicati/Library/Main/Database/LocalBugReportDatabase.cs +++ b/Duplicati/Library/Main/Database/LocalBugReportDatabase.cs @@ -41,7 +41,7 @@ namespace Duplicati.Library.Main.Database public void Fix() { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand()) { cmd.Transaction = tr; diff --git a/Duplicati/Library/Main/Database/LocalDatabase.cs b/Duplicati/Library/Main/Database/LocalDatabase.cs index 17c5c2054..04c99b27f 100644 --- a/Duplicati/Library/Main/Database/LocalDatabase.cs +++ b/Duplicati/Library/Main/Database/LocalDatabase.cs @@ -654,7 +654,7 @@ AND Fileset.ID NOT IN // TODO: Remove this public IDbTransaction BeginTransaction() { - return m_connection.BeginTransaction(); + return m_connection.BeginTransactionSafe(); } protected class TemporaryTransactionWrapper : IDisposable @@ -671,7 +671,7 @@ AND Fileset.ID NOT IN } else { - m_parent = connection.BeginTransaction(); + m_parent = connection.BeginTransactionSafe(); m_isTemporary = true; } } @@ -1626,7 +1626,7 @@ AND oldVersion.FilesetID = (SELECT ID FROM Fileset WHERE ID != @FilesetId ORDER public void PurgeLogData(DateTime threshold) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { var t = Library.Utility.Utility.NormalizeDateTimeToEpochSeconds(threshold); @@ -1643,7 +1643,7 @@ AND oldVersion.FilesetID = (SELECT ID FROM Fileset WHERE ID != @FilesetId ORDER public void PurgeDeletedVolumes(DateTime threshold) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { m_removedeletedremotevolumeCommand.SetParameterValue("@Now", Library.Utility.Utility.NormalizeDateTimeToEpochSeconds(threshold)) @@ -1663,7 +1663,7 @@ AND oldVersion.FilesetID = (SELECT ID FROM Fileset WHERE ID != @FilesetId ORDER { if (m_connection.State == ConnectionState.Open && !m_hasExecutedVacuum) { - using (var transaction = m_connection.BeginTransaction()) + using (var transaction = m_connection.BeginTransactionSafe()) using (var command = m_connection.CreateCommand(transaction)) { // SQLite recommends that PRAGMA optimize is run just before closing each database connection. diff --git a/Duplicati/Library/Main/Database/LocalListChangesDatabase.cs b/Duplicati/Library/Main/Database/LocalListChangesDatabase.cs index 62370c585..694e55872 100644 --- a/Duplicati/Library/Main/Database/LocalListChangesDatabase.cs +++ b/Duplicati/Library/Main/Database/LocalListChangesDatabase.cs @@ -112,7 +112,7 @@ namespace Duplicati.Library.Main.Database m_previousTable = "Previous-" + Library.Utility.Utility.ByteArrayAsHexString(Guid.NewGuid().ToByteArray()); m_currentTable = "Current-" + Library.Utility.Utility.ByteArrayAsHexString(Guid.NewGuid().ToByteArray()); - m_transaction = m_connection.BeginTransaction(); + m_transaction = m_connection.BeginTransactionSafe(); using (var cmd = m_connection.CreateCommand(m_transaction)) { diff --git a/Duplicati/Library/Main/Database/LocalRepairDatabase.cs b/Duplicati/Library/Main/Database/LocalRepairDatabase.cs index 4186052a3..75e240118 100644 --- a/Duplicati/Library/Main/Database/LocalRepairDatabase.cs +++ b/Duplicati/Library/Main/Database/LocalRepairDatabase.cs @@ -289,7 +289,7 @@ namespace Duplicati.Library.Main.Database public void FixDuplicateMetahash() { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { var sql_count = @@ -355,7 +355,7 @@ namespace Duplicati.Library.Main.Database public void FixDuplicateFileentries() { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { var sql_count = @"SELECT COUNT(*) FROM (SELECT ""PrefixID"", ""Path"", ""BlocksetID"", ""MetadataID"", COUNT(*) as ""Duplicates"" FROM ""FileLookup"" GROUP BY ""PrefixID"", ""Path"", ""BlocksetID"", ""MetadataID"") WHERE ""Duplicates"" > 1"; @@ -398,7 +398,7 @@ namespace Duplicati.Library.Main.Database { var blocklistbuffer = new byte[blocksize]; - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) using (var blockhasher = HashFactory.CreateHasher(blockhashalgorithm)) { @@ -535,7 +535,7 @@ namespace Duplicati.Library.Main.Database public void FixDuplicateBlocklistHashes(long blocksize, long hashsize) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { var dup_sql = @"SELECT * FROM (SELECT ""BlocksetID"", ""Index"", COUNT(*) AS ""EC"" FROM ""BlocklistHash"" GROUP BY ""BlocksetID"", ""Index"") WHERE ""EC"" > 1"; @@ -591,7 +591,7 @@ namespace Duplicati.Library.Main.Database public void CheckAllBlocksAreInVolume(string filename, IEnumerable> blocks) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { var tablename = "ProbeBlocks-" + Library.Utility.Utility.ByteArrayAsHexString(Guid.NewGuid().ToByteArray()); diff --git a/Duplicati/Library/Main/Database/LocalRestoreDatabase.cs b/Duplicati/Library/Main/Database/LocalRestoreDatabase.cs index f54ec37fd..545e4d2f4 100644 --- a/Duplicati/Library/Main/Database/LocalRestoreDatabase.cs +++ b/Duplicati/Library/Main/Database/LocalRestoreDatabase.cs @@ -246,7 +246,7 @@ END "); // If we get a list of filenames, the lookup table is faster // unfortunately we cannot do this if the filesystem is case sensitive as // SQLite only supports ASCII compares - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { var p = expression.GetSimpleList(); var m_filenamestable = "Filenames-" + m_temptabsetguid; @@ -1173,7 +1173,7 @@ ORDER BY ""A"".""TargetPath"", ""BB"".""Index""")); public DirectBlockMarker(IDbConnection connection, string blocktablename, string filetablename, string statstablename) { - m_transaction = connection.BeginTransaction(); + m_transaction = connection.BeginTransactionSafe(); m_blocktablename = blocktablename; m_filetablename = filetablename; @@ -1362,7 +1362,7 @@ WHERE ""FileID"" = @TargetFileId AND ""Index"" = @Index AND ""Hash"" = @Hash AND public IEnumerable GetFilesAndSourceBlocksFast(long blocksize) { - using (var transaction = m_connection.BeginTransaction()) + using (var transaction = m_connection.BeginTransactionSafe()) using (var cmdReader = m_connection.CreateCommand()) using (var cmd = m_connection.CreateCommand(transaction)) { diff --git a/Duplicati/Library/Main/Database/ReusableTransaction.cs b/Duplicati/Library/Main/Database/ReusableTransaction.cs index 36d23e142..4f681deec 100644 --- a/Duplicati/Library/Main/Database/ReusableTransaction.cs +++ b/Duplicati/Library/Main/Database/ReusableTransaction.cs @@ -64,14 +64,14 @@ internal class ReusableTransaction : IDisposable /// /// The log message to use /// True if the transaction should be restarted - public void Commit(string? message = null, bool restart = true) + public void Commit(string? message, bool restart = true) { if (m_transaction == null) throw new InvalidOperationException("Transaction is already disposed"); if (m_transaction != null) { - using (var timer = string.IsNullOrWhiteSpace(message) ? null : new Logging.Timer(LOGTAG, message, "CommitTransaction")) + using (var timer = string.IsNullOrWhiteSpace(message) ? null : new Logging.Timer(LOGTAG, message, $"CommitTransaction: {message}")) m_transaction.Commit(); m_transaction.Dispose(); m_transaction = null; diff --git a/Duplicati/Library/Main/Operation/CompactHandler.cs b/Duplicati/Library/Main/Operation/CompactHandler.cs index 070d443be..6a05d3de5 100644 --- a/Duplicati/Library/Main/Operation/CompactHandler.cs +++ b/Duplicati/Library/Main/Operation/CompactHandler.cs @@ -203,7 +203,7 @@ namespace Duplicati.Library.Main.Operation // Commit as we have uploaded a volume if (!m_options.Dryrun) - rtr.Commit(); + rtr.Commit("CommitCompact"); } } } @@ -317,7 +317,7 @@ namespace Duplicati.Library.Main.Operation await backend.WaitForEmptyAsync(db, rtr.Transaction, cancellationToken).ConfigureAwait(false); if (!m_options.Dryrun) - rtr.Commit(); + rtr.Commit("CommitDelete"); await foreach (var d in PerformDelete(backend, remoteFilesToRemove, cancellationToken).ConfigureAwait(false)) yield return d; diff --git a/Duplicati/Library/Main/Operation/DeleteHandler.cs b/Duplicati/Library/Main/Operation/DeleteHandler.cs index d39b14190..79725890c 100644 --- a/Duplicati/Library/Main/Operation/DeleteHandler.cs +++ b/Duplicati/Library/Main/Operation/DeleteHandler.cs @@ -94,7 +94,7 @@ namespace Duplicati.Library.Main.Operation db.UpdateRemoteVolume(f.Key, RemoteVolumeState.Deleting, f.Value, null, rtr.Transaction); if (!m_options.Dryrun) - rtr.Commit(); + rtr.Commit("CommitBeforeDelete"); foreach (var f in lst) { diff --git a/Duplicati/Library/Main/Operation/PurgeFilesHandler.cs b/Duplicati/Library/Main/Operation/PurgeFilesHandler.cs index 878568246..0748482b8 100644 --- a/Duplicati/Library/Main/Operation/PurgeFilesHandler.cs +++ b/Duplicati/Library/Main/Operation/PurgeFilesHandler.cs @@ -247,7 +247,7 @@ namespace Duplicati.Library.Main.Operation .DoCompactAsync(cdb, true, ctr, backendManager) .ConfigureAwait(false); - ctr.Commit(restart: false); + ctr.Commit("PostCompact", restart: false); } } diff --git a/Duplicati/Library/Main/Operation/RepairHandler.cs b/Duplicati/Library/Main/Operation/RepairHandler.cs index bfd52a0fc..ee2edb257 100644 --- a/Duplicati/Library/Main/Operation/RepairHandler.cs +++ b/Duplicati/Library/Main/Operation/RepairHandler.cs @@ -649,7 +649,7 @@ namespace Duplicati.Library.Main.Operation using (var rdb = new LocalRecreateDatabase(db, m_options)) RecreateDatabaseHandler.RecreateFilesetFromRemoteList(rdb, tr.Transaction, compressor, entry.Key, m_options, new FilterExpression()); - tr.Commit(); + tr.Commit("PostRepairFileset"); } } diff --git a/Duplicati/Library/RestAPI/Database/Connection.cs b/Duplicati/Library/RestAPI/Database/Connection.cs index 0af4fdb54..ab05de2b9 100644 --- a/Duplicati/Library/RestAPI/Database/Connection.cs +++ b/Duplicati/Library/RestAPI/Database/Connection.cs @@ -218,7 +218,7 @@ namespace Duplicati.Server.Database internal void SetMetadata(IDictionary values, long id, IDbTransaction? transaction) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { OverwriteAndUpdateDb( tr, @@ -254,7 +254,7 @@ namespace Duplicati.Server.Database internal void SetFilters(IEnumerable values, long id, IDbTransaction? transaction = null) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { OverwriteAndUpdateDb( tr, @@ -292,7 +292,7 @@ namespace Duplicati.Server.Database internal void SetSettings(IEnumerable values, long id, IDbTransaction? transaction = null) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { if (m_encryptSensitiveFields) values = values.Select(x => new Setting @@ -338,7 +338,7 @@ namespace Duplicati.Server.Database internal void SetSources(IEnumerable values, long id, IDbTransaction transaction) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { OverwriteAndUpdateDb( tr, @@ -595,7 +595,7 @@ namespace Duplicati.Server.Database { lock (m_lock) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { using (var cmd = m_connection.CreateCommand(tr, @"UPDATE ""Backup"" SET ""DBPath""= @Dbpath WHERE ""ID""= @Id")) { @@ -636,7 +636,7 @@ namespace Duplicati.Server.Database throw new Exception("Unable to generate a unique database file name"); } - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { OverwriteAndUpdateDb( tr, @@ -728,7 +728,7 @@ namespace Duplicati.Server.Database internal void AddOrUpdateSchedule(ISchedule item) { lock (m_lock) - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { AddOrUpdateSchedule(item, tr); tr.Commit(); @@ -778,7 +778,7 @@ namespace Duplicati.Server.Database lock (m_lock) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { var existing = GetScheduleIDsFromTags(new string[] { "ID=" + ID.ToString() }); if (existing.Any()) @@ -958,7 +958,7 @@ namespace Duplicati.Server.Database //Workaround to clean up the database after invalid settings update public void FixInvalidBackupId() { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { cmd.SetCommandAndParameters(@"DELETE FROM ""Option"" WHERE ""BackupID"" = @BackupId") @@ -1009,7 +1009,7 @@ namespace Duplicati.Server.Database public void SetUISettings(string scheme, IDictionary values, IDbTransaction? transaction = null) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { OverwriteAndUpdateDb( tr, @@ -1033,7 +1033,7 @@ namespace Duplicati.Server.Database public void UpdateUISettings(string scheme, IDictionary values, IDbTransaction? transaction = null) { lock (m_lock) - using (var tr = transaction == null ? m_connection.BeginTransaction() : null) + using (var tr = transaction == null ? m_connection.BeginTransactionSafe() : null) { OverwriteAndUpdateDb( tr, @@ -1086,7 +1086,7 @@ namespace Duplicati.Server.Database { var t = Library.Utility.Utility.NormalizeDateTimeToEpochSeconds(purgeDate); - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) using (var cmd = m_connection.CreateCommand(tr)) { cmd.SetCommandAndParameters(@"DELETE FROM ""ErrorLog"" WHERE ""Timestamp"" < @Time") @@ -1165,7 +1165,7 @@ namespace Duplicati.Server.Database { if (transaction == null) { - using (var tr = m_connection.BeginTransaction()) + using (var tr = m_connection.BeginTransactionSafe()) { var r = DeleteFromDb(tablename, id, tr); tr.Commit();