Skip to content

[Collections] CLI 'remove' results in "database is locked" error #3225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 27, 2021
Merged

[Collections] CLI 'remove' results in "database is locked" error #3225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8129be
[Collections] CLI 'remove' results in "database is locked" error
yim-lee Jan 26, 2021
0a70a65
Exponential backoff
yim-lee Jan 27, 2021
5623ca4
No 'pow'
yim-lee Jan 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 78 additions & 83 deletions Sources/Commands/SwiftPackageCollectionsTool.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,11 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
}

let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
let collection = try with { collections in
return try tsc_await { collections.getCollection(source, callback: $0) }
try with { collections in
let collection = try tsc_await { collections.getCollection(source, callback: $0) }
_ = try tsc_await { collections.removeCollection(source, callback: $0) }
print("Removed \"\(collection.name)\" from your package collections.")
}

_ = try with { collections in try tsc_await { collections.removeCollection(source, callback: $0) } }
print("Removed \"\(collection.name)\" from your package collections.")
}
}

Expand All @@ -163,31 +162,29 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
var searchQuery: String

mutating func run() throws {
switch searchMethod {
case .keywords:
let results = try with { collections in
return try tsc_await { collections.findPackages(searchQuery, collections: nil, callback: $0) }
}

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(results.items)
} else {
results.items.forEach {
print("\($0.package.repository.url): \($0.package.summary ?? "")")
try with { collections in
switch searchMethod {
case .keywords:
let results = try tsc_await { collections.findPackages(searchQuery, collections: nil, callback: $0) }
if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(results.items)
} else {
results.items.forEach {
print("\($0.package.repository.url): \($0.package.summary ?? "")")
}
}
}

case .module:
let results = try with { collections in
return try tsc_await { collections.findTargets(searchQuery, searchType: .exactMatch, collections: nil, callback: $0) }
}
case .module:
let results = try tsc_await { collections.findTargets(searchQuery, searchType: .exactMatch, collections: nil, callback: $0) }

let packages = Set(results.items.flatMap { $0.packages })
if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(packages)
} else {
packages.forEach {
print("\($0.repository.url): \($0.summary ?? "")")
let packages = Set(results.items.flatMap { $0.packages })
if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(packages)
} else {
packages.forEach {
print("\($0.repository.url): \($0.summary ?? "")")
}
}
}
}
Expand Down Expand Up @@ -229,71 +226,69 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
}

mutating func run() throws {
let identity = PackageIdentity(url: packageUrl)
let reference = PackageReference(identity: identity, path: packageUrl)

do { // assume URL is for a package
let result = try with { collections in
return try tsc_await { collections.getPackageMetadata(reference, callback: $0) }
}
try with { collections in
let identity = PackageIdentity(url: packageUrl)
let reference = PackageReference(identity: identity, path: packageUrl)

if let versionString = version {
guard let version = TSCUtility.Version(string: versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {
throw CollectionsError.invalidVersionString(versionString)
}
do { // assume URL is for a package
let result = try tsc_await { collections.getPackageMetadata(reference, callback: $0) }

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(result)
if let versionString = version {
guard let version = TSCUtility.Version(string: versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {
throw CollectionsError.invalidVersionString(versionString)
}

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(result)
} else {
print("\(indent())Version: \(printedResult)")
}
} else {
print("\(indent())Version: \(printedResult)")
let description = optionalRow("Description", result.package.summary)
let versions = result.package.versions.map { "\($0.version)" }.joined(separator: ", ")
let watchers = optionalRow("Watchers", result.package.watchersCount?.description)
let readme = optionalRow("Readme", result.package.readmeURL?.absoluteString)
let authors = optionalRow("Authors", result.package.authors?.map { $0.username }.joined(separator: ", "))
let latestVersion = optionalRow("\(String(repeating: "-", count: 60))\n\(indent())Latest Version", printVersion(result.package.latestVersion))

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(result.package)
} else {
print("""
\(description)
Available Versions: \(versions)\(watchers)\(readme)\(authors)\(latestVersion)
""")
}
}
} else {
let description = optionalRow("Description", result.package.summary)
let versions = result.package.versions.map { "\($0.version)" }.joined(separator: ", ")
let watchers = optionalRow("Watchers", result.package.watchersCount?.description)
let readme = optionalRow("Readme", result.package.readmeURL?.absoluteString)
let authors = optionalRow("Authors", result.package.authors?.map { $0.username }.joined(separator: ", "))
let latestVersion = optionalRow("\(String(repeating: "-", count: 60))\n\(indent())Latest Version", printVersion(result.package.latestVersion))

} catch { // assume URL is for a collection
// If a version argument was given, we do not perform the fallback.
if version != nil {
throw error
}

guard let collectionUrl = URL(string: packageUrl) else {
throw CollectionsError.invalidArgument("collectionUrl")
}

let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
let collection = try tsc_await { collections.getCollection(source, callback: $0) }

let description = optionalRow("Description", collection.overview)
let keywords = optionalRow("Keywords", collection.keywords?.joined(separator: ", "))
let createdAt = optionalRow("Created At", DateFormatter().string(from: collection.createdAt))
let packages = collection.packages.map { "\($0.repository.url)" }.joined(separator: "\n\(indent(levels: 2))")

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(result.package)
try JSONEncoder.makeWithDefaults().print(collection)
} else {
print("""
\(description)
Available Versions: \(versions)\(watchers)\(readme)\(authors)\(latestVersion)
""")
Name: \(collection.name)
Source: \(collection.source.url)\(description)\(keywords)\(createdAt)
Packages:
\(packages)
""")
}
}
} catch { // assume URL is for a collection
// If a version argument was given, we do not perform the fallback.
if version != nil {
throw error
}

guard let collectionUrl = URL(string: packageUrl) else {
throw CollectionsError.invalidArgument("collectionUrl")
}

let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
let collection = try with { collections in
try tsc_await { collections.getCollection(source, callback: $0) }
}

let description = optionalRow("Description", collection.overview)
let keywords = optionalRow("Keywords", collection.keywords?.joined(separator: ", "))
let createdAt = optionalRow("Created At", DateFormatter().string(from: collection.createdAt))
let packages = collection.packages.map { "\($0.repository.url)" }.joined(separator: "\n\(indent(levels: 2))")

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(collection)
} else {
print("""
Name: \(collection.name)
Source: \(collection.source.url)\(description)\(keywords)\(createdAt)
Packages:
\(packages)
""")
}
}
}
}
Expand Down
109 changes: 91 additions & 18 deletions Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
private let cache = ThreadSafeKeyValueStore<Model.CollectionIdentifier, Model.Collection>()
private let cacheLock = Lock()

private let isShuttingdown = ThreadSafeBox<Bool>()

// Lock helps prevent concurrency errors with transaction statements during e.g. `refreshCollections`,
// since only one transaction is allowed per SQLite connection. We need transactions to speed up bulk updates.
// TODO: we could potentially optimize this with db connection pool
Expand All @@ -50,7 +52,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable

// Targets have in-memory trie in addition to SQLite FTS as optimization
private let targetTrie = Trie<CollectionPackage>()
private var targetTrieReady = ThreadSafeBox<Bool>(false)
private var targetTrieReady = ThreadSafeBox<Bool>()

init(location: SQLite.Location? = nil, diagnosticsEngine: DiagnosticsEngine? = nil) {
self.location = location ?? .path(localFileSystem.swiftPMCacheDirectory.appending(components: "package-collection.db"))
Expand Down Expand Up @@ -78,14 +80,81 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
}

func close() throws {
// Signal long-running operation (e.g., populateTargetTrie) to stop
self.isShuttingdown.put(true)

func retryClose(db: SQLite, exponentialBackoff: inout ExponentialBackoff) throws {
let semaphore = DispatchSemaphore(value: 0)
let callback = { (result: Result<Void, Error>) in
// If it has failed, the semaphore will timeout in which case we will retry
if case .success = result {
semaphore.signal()
}
}

// This throws error if we have exhausted our attempts
let delay = try exponentialBackoff.nextDelay()
self.queue.asyncAfter(deadline: .now() + delay) {
do {
try db.close()
callback(.success(()))
} catch {
callback(.failure(error))
}
}
// Add some buffer to allow `asyncAfter` to run
guard case .success = semaphore.wait(timeout: .now() + delay + .milliseconds(50)) else {
return try retryClose(db: db, exponentialBackoff: &exponentialBackoff)
}
}

try self.stateLock.withLock {
if case .connected(let db) = self.state {
try db.close()
do {
try db.close()
} catch {
var exponentialBackoff = ExponentialBackoff()
do {
try retryClose(db: db, exponentialBackoff: &exponentialBackoff)
} catch {
throw StringError("Failed to close database")
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general the approach seems fine, but the delay here could be fragile - if the DB operation is longer than 200/300ms it will still fail with "db is locked", we could potentially do a recursive "exponential backoff" style thing here which would be safer. alternatively we could try to eliminate the issue using "sqlite3_interrupt" before trying to close again

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exponential backoff: 0a70a65

}
self.state = .disconnected
}
}

private struct ExponentialBackoff {
let intervalInMilliseconds: Int
let randomizationFactor: Int
let maximumAttempts: Int

var attempts: Int = 0
var multipler: Int = 1

var canRetry: Bool {
self.attempts < self.maximumAttempts
}

init(intervalInMilliseconds: Int = 100, randomizationFactor: Int = 100, maximumAttempts: Int = 3) {
self.intervalInMilliseconds = intervalInMilliseconds
self.randomizationFactor = randomizationFactor
self.maximumAttempts = maximumAttempts
}

mutating func nextDelay() throws -> DispatchTimeInterval {
guard self.canRetry else {
throw StringError("Maximum attempts reached")
}
let delay = self.multipler * intervalInMilliseconds
let jitter = Int.random(in: 0 ... self.randomizationFactor)
self.attempts += 1
self.multipler *= 2
return .milliseconds(delay + jitter)
}
}

func put(collection: Model.Collection,
callback: @escaping (Result<Model.Collection, Error>) -> Void) {
self.queue.async {
Expand Down Expand Up @@ -756,25 +825,29 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable

func populateTargetTrie(callback: @escaping (Result<Void, Error>) -> Void = { _ in }) {
self.queue.async {
do {
// Use FTS to build the trie
let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
try self.executeStatement(query) { statement in
while let row = try statement.step() {
let targetName = row.string(at: 2)

if let collectionData = Data(base64Encoded: row.string(at: 0)),
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
self.targetTrieReady.memoize {
do {
// Use FTS to build the trie
let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
try self.executeStatement(query) { statement in
while let row = try statement.step() {
if self.isShuttingdown.get() ?? false { return }

let targetName = row.string(at: 2)

if let collectionData = Data(base64Encoded: row.string(at: 0)),
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
}
}
}
callback(.success(()))
return true
} catch {
callback(.failure(error))
return false
}
self.targetTrieReady.put(true)
callback(.success(()))
} catch {
self.targetTrieReady.put(false)
callback(.failure(error))
}
}
}
Expand Down