Skip to content

Commit 86bd1fe

Browse files
authored
[Collections] Call VACUUM in async queue (#3850)
Motivation: I noticed that when I run `package-collection remove` in CLI there is a warning in the output: ``` Failed to 'VACUUM' the database: SQL logic error ``` Modifications: - Not sure what the real cause is, but the problem doesn't occur if there is a sleep between `DELETE` and `VACUUM`, so perhaps `VACUUM` fails because other operations are not done yet. Instead of calling `VACUUM` synchronously, do that in an async queue, which is probably more appropriate since `VACUUM` can be time consuming. - Fix a bug in the same file where we didn't do `ROLLBACK` when a transaction fails.
1 parent f02d201 commit 86bd1fe

File tree

1 file changed

+54
-46
lines changed

1 file changed

+54
-46
lines changed

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

Lines changed: 54 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -706,60 +706,64 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
706706
try self.removeFromSearchIndices(identifier: collection.identifier)
707707
// Update search indices
708708
try self.withDB { db in
709-
try db.exec(query: "BEGIN TRANSACTION;")
710-
711709
let packagesStatement = try db.prepare(query: "INSERT INTO \(Self.packagesFTSName) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);")
712710
let targetsStatement = try db.prepare(query: "INSERT INTO \(Self.targetsFTSName) VALUES (?, ?, ?);")
711+
712+
try db.exec(query: "BEGIN TRANSACTION;")
713+
do {
714+
// Then insert new data
715+
try collection.packages.forEach { package in
716+
var targets = Set<String>()
717+
718+
try package.versions.forEach { version in
719+
try version.manifests.values.forEach { manifest in
720+
// Packages FTS
721+
let packagesBindings: [SQLite.SQLiteValue] = [
722+
.string(try self.encoder.encode(collection.identifier).base64EncodedString()),
723+
.string(package.identity.description),
724+
.string(version.version.description),
725+
.string(manifest.packageName),
726+
.string(package.location),
727+
package.summary.map { .string($0) } ?? .null,
728+
package.keywords.map { .string($0.joined(separator: ",")) } ?? .null,
729+
.string(manifest.products.map { $0.name }.joined(separator: ",")),
730+
.string(manifest.targets.map { $0.name }.joined(separator: ",")),
731+
]
732+
try packagesStatement.bind(packagesBindings)
733+
try packagesStatement.step()
734+
735+
try packagesStatement.clearBindings()
736+
try packagesStatement.reset()
737+
738+
manifest.targets.forEach { targets.insert($0.name) }
739+
}
740+
}
713741

714-
// Then insert new data
715-
try collection.packages.forEach { package in
716-
var targets = Set<String>()
742+
let collectionPackage = CollectionPackage(collection: collection.identifier, package: package.identity)
743+
try targets.forEach { target in
744+
// Targets in-memory trie
745+
self.targetTrie.insert(word: target.lowercased(), foundIn: collectionPackage)
717746

718-
try package.versions.forEach { version in
719-
try version.manifests.values.forEach { manifest in
720-
// Packages FTS
721-
let packagesBindings: [SQLite.SQLiteValue] = [
747+
// Targets FTS
748+
let targetsBindings: [SQLite.SQLiteValue] = [
722749
.string(try self.encoder.encode(collection.identifier).base64EncodedString()),
723-
.string(package.identity.description),
724-
.string(version.version.description),
725-
.string(manifest.packageName),
726750
.string(package.location),
727-
package.summary.map { .string($0) } ?? .null,
728-
package.keywords.map { .string($0.joined(separator: ",")) } ?? .null,
729-
.string(manifest.products.map { $0.name }.joined(separator: ",")),
730-
.string(manifest.targets.map { $0.name }.joined(separator: ",")),
751+
.string(target),
731752
]
732-
try packagesStatement.bind(packagesBindings)
733-
try packagesStatement.step()
753+
try targetsStatement.bind(targetsBindings)
754+
try targetsStatement.step()
734755

735-
try packagesStatement.clearBindings()
736-
try packagesStatement.reset()
737-
738-
manifest.targets.forEach { targets.insert($0.name) }
756+
try targetsStatement.clearBindings()
757+
try targetsStatement.reset()
739758
}
740759
}
741-
742-
let collectionPackage = CollectionPackage(collection: collection.identifier, package: package.identity)
743-
try targets.forEach { target in
744-
// Targets in-memory trie
745-
self.targetTrie.insert(word: target.lowercased(), foundIn: collectionPackage)
746-
747-
// Targets FTS
748-
let targetsBindings: [SQLite.SQLiteValue] = [
749-
.string(try self.encoder.encode(collection.identifier).base64EncodedString()),
750-
.string(package.location),
751-
.string(target),
752-
]
753-
try targetsStatement.bind(targetsBindings)
754-
try targetsStatement.step()
755-
756-
try targetsStatement.clearBindings()
757-
try targetsStatement.reset()
758-
}
760+
761+
try db.exec(query: "COMMIT;")
762+
} catch {
763+
try db.exec(query: "ROLLBACK;")
764+
throw error
759765
}
760766

761-
try db.exec(query: "COMMIT;")
762-
763767
try packagesStatement.finalize()
764768
try targetsStatement.finalize()
765769
}
@@ -785,12 +789,16 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
785789
try statement.step()
786790
}
787791

788-
// Repack database file to reduce size (rdar://77077510)
789-
try self.withDB { db in
792+
// Run VACUUM asynchronously since it's just an optimization measure and could take some time to complete.
793+
// Add a delay since VACUUM often fails if run immediately after the deletions.
794+
DispatchQueue.sharedConcurrent.asyncAfter(deadline: .now() + 0.5) {
795+
// Repack database file to reduce size (rdar://77077510)
790796
do {
791-
try db.exec(query: "VACUUM;")
797+
try self.withDB { db in
798+
try db.exec(query: "VACUUM;")
799+
}
792800
} catch {
793-
self.observabilityScope.emit(warning: "Failed to 'VACUUM' the database: \(error)")
801+
self.observabilityScope.emit(info: "Failed to 'VACUUM' the database: \(error)")
794802
}
795803
}
796804

0 commit comments

Comments
 (0)