Implement connection pool. #451
Conversation
Uses WAL mode to support multiple reads and a single writer.
Allows customizing new connections once created
ConnectionType is now Connection and Connection is now DBConnection
vfs seems to be a more reliable handling of exclusive mode.
Was using incorrect dispatch_once token.
# Conflicts: # SQLiteTests/ConnectionPoolTests.swift
… into connection-pool
| @@ -151,44 +153,41 @@ public final class ConnectionPool { | |||
|
|
|||
| var borrowed : BorrowedConnection! | |||
|
|
|||
| repeat { | |||
| dispatch_semaphore_wait(connectionSemaphore, DISPATCH_TIME_FOREVER) | |||
There was a problem hiding this comment.
The diff didn't come through very well on the changes below. All I did was remove the repeat loop and add this semaphore wait before the dispatch_sync call.
|
I guess I'm going to have to do #457 first... |
|
Tests passed! Woot! |
| public protocol Connection { | ||
|
|
||
| /// Whether or not the database was opened in a read-only state. | ||
| var readonly : Bool { get } |
There was a problem hiding this comment.
No space before colons on property declarations.
| @@ -138,7 +138,7 @@ extension Tokenizer : CustomStringConvertible { | |||
|
|
|||
| } | |||
|
|
|||
| extension Connection { | |||
| extension DirectConnection { | |||
|
|
|||
| public func registerTokenizer(submoduleName: String, next: String -> (String, Range<String.Index>)?) throws { | |||
There was a problem hiding this comment.
Can I use FTS with a connection pool? Because this extension is on DirectConnection, I'm inclined to think no.
|
Also, please update the documentation. It references Connection, which has been renamed, and it needs a section on using a connection pool. |
| private var unavailableReadConnections = [DirectConnection]() | ||
| private let lockQueue : dispatch_queue_t | ||
| private var writeConnection : DirectConnection! | ||
| private let connectionSemaphore = dispatch_semaphore_create(5) |
There was a problem hiding this comment.
This is a fine default, but should be configurable.
There was a problem hiding this comment.
Why do we want this configurable?
There was a problem hiding this comment.
Because we do not know that 5 is the right connection pool size for every app.
|
I'll update the source documentation, but should the user documentation coincide with the latest release or what's in master? I usually expect the former, in which case we wouldn't want to change any documentation yet (unless it's in a branch or something). |
|
Is https://github.com/stephencelis/SQLite.swift/blob/master/Documentation/Index.md#sqliteswift-documentation what you mean by user documentation? That's part of the repo and should be kept in sync with the code. |
|
Yes that is what I mean. And I agree that it should be kept in sync with the code, but as a user I would want that kept in sync with the latest release of the code so I can understand how to use the library I'm consuming rather than an unreleased version. But I can just as easily do it now, just didn't want to confuse users. What do you prefer @stephencelis? |
|
I changed the API so it's backwards-compatible (and I think also better than what it was, but I would love some feedback on it). |
|
I looked at the history of the documentation, and it looks like 77a0f8e updated the documentation along with the code, so I'll go ahead and update the docs (but I'd still love some feedback on my changes in the meantime). |
|
Love the new API. 👍 |
|
👍 This looks great to me. |
| public init(_ name: String) { | ||
| queue = dispatch_queue_create(name, DISPATCH_QUEUE_SERIAL) | ||
| queueContext = unsafeBitCast(queue, UnsafeMutablePointer<Void>.self) | ||
| dispatch_queue_set_specific(queue, ReentrantDispatcher.queueKey, queueContext, nil) |
There was a problem hiding this comment.
@stephencelis: Sorry, looks like this (some part of the refactoring) broke the ability to use a single connection across multiple threads. I have a broken test ready, working on a fix.
Most of this work was done in #411, which seems to have been abandoned, so I'm trying to get it through. Let me know if there's a better way than opening a new PR.