feat: enable multiple db connections with sqlcipher #3507
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (16)
|
sqlite/sqlite.go
Outdated
| return err | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
WAL mode is not set. Is it on purpose?
There was a problem hiding this comment.
Good catch! I thought setting it once would be sufficient, as once you set PRAGMA journal_mode=WAL;, the setting is stored in the database file itself. I have learned that even though switching the mode to WAL introduces a persistent change to the database, each new connection must still "opt-in" to using the WAL mode. If a connection does not do this, it defaults to the DELETE journal mode. While this connection can still read from and write to the database, it will not take advantage of the benefits of WAL.
sqlite/sqlite.go
Outdated
| // This method is used because the 'database/sql' package does not expose 'ConnectHook', | ||
| // thereby making it impossible to individually configure each connection. | ||
| // Consequently, the connections from the pool can't be properly decrypted, making them unusable. | ||
| sqlcipherDriver, ok := db.Driver().(*sqlcipher.SQLiteDriver) |
There was a problem hiding this comment.
There's one other way of doing it without breaking the abstraction:
driverName := fmt.Sprintf("sqlcipher_with_extensions-%d", len(sql.Drivers()))
sql.Register(driverName , &sqlcipher.SQLiteDriver{
ConnectHook: func(conn *sqlcipher.SQLiteConn) error {
///Running init queries
},
})
db, err := sql.Open(driverName, path)
There was a problem hiding this comment.
Smart! I will use it 👍
There was a problem hiding this comment.
With the current configuration on a test project, when trying a few writes at the same time I get database is locked on 10-20% of them. The test is configured to write 1Kb of data per task.
Might need some tweaking in order to get the async writing in a better shape.
Other projects are using the busy_timeout to fix this, but I think we need to make sure every write we do doesn't take more than the timeout value.
There was a problem hiding this comment.
I've seen the same error before these changes a couple of times too, with the discord import when the app was under heavy load.
There was a problem hiding this comment.
I've added a workaround (PRAGMA busy_timeout=60000) to mitigate the "database is locked" errors during concurrent write operations.
This approach retains behavior similar to our current setup with one 'always blocking' connection, which waits as long as needed for the connection to release lock. I believe setting busy_timeout to 60s (aka "infinite" wait) essentially approximates the same behavior.
sqlite/sqlite.go
Outdated
| // This method is used because the 'database/sql' package does not expose 'ConnectHook', | ||
| // thereby making it impossible to individually configure each connection. | ||
| // Consequently, the connections from the pool can't be properly decrypted, making them unusable. | ||
| sqlcipherDriver, ok := db.Driver().(*sqlcipher.SQLiteDriver) |
There was a problem hiding this comment.
In my tests I found the best performance when using a limited amount of connections and the same number of max idle connections. The peak performance I see on M1 Macs is when using:
db.SetMaxOpenConns(10)
db.SetMaxIdleConns(10)
Not sure it's a coincidence to be the same number as CPU count.
Leaving the max open connection to unlimited will open a new connection when it's needed, and close the idle connections. This has the potential of paying the cost of opening the connection on lots of queries.
The difference I have is (In the app the difference might be smaller because we're probably not throttling the DB at this magnitude, but it gives an idea on how the connection pool works):
32 queries/second with unlimited connections
60662 queries/second with 10 max connections, 10 max idle connections
There was a problem hiding this comment.
In my tests I found the best performance when using a limited amount of connections and the same number of max idle connections. The peak performance I see on M1 Macs is when using:
db.SetMaxOpenConns(10) db.SetMaxIdleConns(10)Not sure it's a coincidence to be the same number as CPU count.
Yeah this smells like it's supposed to ideally use the number of HW threads :)
There was a problem hiding this comment.
Something like this should be used: https://go.dev/play/p/IFnlTmTD6Q
There was a problem hiding this comment.
I've done some benchmarking and it seems for my machine the number of maxIdleConns doesn't really matter:
@alexjba could you please give it a try on M1? Benchmarking code: https://github.com/osmaczko/status-go-db-perf/
There was a problem hiding this comment.
I set maxOpenConns to nproc as suggested and left maxIdleConns as default = 2 (see comment above).
|
Closing #10723 |
osmaczko
force-pushed
the
feat/multiple-sqlcipher-connections
branch
4 times, most recently
from
b0f63a2 to
cd5b19d
Compare
osmaczko
force-pushed
the
feat/multiple-sqlcipher-connections
branch
from
cd5b19d to
6b51b7a
Compare
closes: status-im/status-desktop#10767
part of: status-im/status-desktop#10558