-
Notifications
You must be signed in to change notification settings - Fork 331
Refactor/improve performance sqlite db #5472
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
91438c5
8b072ec
a99f0c9
102b4cf
65a9bc0
f517587
fd32c5a
4b7800b
22e29ac
5af4d49
59c40c4
734cd94
c838f2a
de0e85f
f2afd18
a308973
1def8d6
88aaffc
4d7dd6c
587331d
1625c16
b157860
063ab19
98a6e36
9718b79
d0d68b4
69e8ee0
601893b
f7a24ed
7c8b3c5
ff5ccef
d930a36
d8dfda6
eca9e89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ use std::{ | |
}; | ||
|
||
use deadpool_sqlite::PoolConfig; | ||
use zeroize::Zeroize; | ||
|
||
#[cfg(feature = "crypto-store")] | ||
pub use self::crypto_store::SqliteCryptoStore; | ||
|
@@ -43,13 +44,21 @@ pub use self::state_store::{SqliteStateStore, DATABASE_NAME as STATE_STORE_DATAB | |
#[cfg(test)] | ||
matrix_sdk_test_utils::init_tracing_for_tests!(); | ||
|
||
#[derive(Clone, Debug, Eq, PartialEq, Zeroize)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should derive |
||
pub enum Secret { | ||
#[zeroize] | ||
Key([u8; 32]), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be inside a In short, every time you pass the If a |
||
#[zeroize] | ||
PassPhrase(String), | ||
Comment on lines
+49
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The enum variants need to be documented. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, this will produce unintended copies, |
||
} | ||
|
||
/// A configuration structure used for opening a store. | ||
#[derive(Clone)] | ||
pub struct SqliteStoreConfig { | ||
/// Path to the database, without the file name. | ||
path: PathBuf, | ||
/// Passphrase to open the store, if any. | ||
passphrase: Option<String>, | ||
/// Secret to open the store, if any | ||
secret: Option<Secret>, | ||
/// The pool configuration for [`deadpool_sqlite`]. | ||
pool_config: PoolConfig, | ||
/// The runtime configuration to apply when opening an SQLite connection. | ||
|
@@ -82,9 +91,9 @@ impl SqliteStoreConfig { | |
{ | ||
Self { | ||
path: path.as_ref().to_path_buf(), | ||
passphrase: None, | ||
pool_config: PoolConfig::new(max(POOL_MINIMUM_SIZE, num_cpus::get_physical() * 4)), | ||
runtime_config: RuntimeConfig::default(), | ||
secret: None, | ||
} | ||
} | ||
|
||
|
@@ -121,7 +130,13 @@ impl SqliteStoreConfig { | |
|
||
/// Define the passphrase if the store is encoded. | ||
pub fn passphrase(mut self, passphrase: Option<&str>) -> Self { | ||
self.passphrase = passphrase.map(|passphrase| passphrase.to_owned()); | ||
self.secret = passphrase.map(|passphrase| Secret::PassPhrase(passphrase.to_owned())); | ||
self | ||
} | ||
|
||
/// Define the key if the store is encoded. | ||
pub fn key(mut self, key: Option<&[u8; 32]>) -> Self { | ||
self.secret = key.map(|key| Secret::Key(*key)); | ||
self | ||
} | ||
|
||
|
@@ -225,7 +240,7 @@ mod tests { | |
path::{Path, PathBuf}, | ||
}; | ||
|
||
use super::{SqliteStoreConfig, POOL_MINIMUM_SIZE}; | ||
use super::{Secret, SqliteStoreConfig, POOL_MINIMUM_SIZE}; | ||
|
||
#[test] | ||
fn test_new() { | ||
|
@@ -248,7 +263,7 @@ mod tests { | |
} | ||
|
||
#[test] | ||
fn test_store_config() { | ||
fn test_store_config_when_passphrase() { | ||
let store_config = SqliteStoreConfig::new(Path::new("foo")) | ||
.passphrase(Some("bar")) | ||
.pool_max_size(42) | ||
|
@@ -257,7 +272,36 @@ mod tests { | |
.journal_size_limit(44); | ||
|
||
assert_eq!(store_config.path, PathBuf::from("foo")); | ||
assert_eq!(store_config.passphrase, Some("bar".to_owned())); | ||
assert_eq!(store_config.secret, Some(Secret::PassPhrase("bar".to_owned()))); | ||
assert_eq!(store_config.pool_config.max_size, 42); | ||
assert!(store_config.runtime_config.optimize.not()); | ||
assert_eq!(store_config.runtime_config.cache_size, 43); | ||
assert_eq!(store_config.runtime_config.journal_size_limit, 44); | ||
} | ||
|
||
#[test] | ||
fn test_store_config_when_key() { | ||
let store_config = SqliteStoreConfig::new(Path::new("foo")) | ||
.key(Some(&[ | ||
143, 27, 202, 78, 96, 55, 13, 149, 247, 8, 33, 120, 204, 92, 171, 66, 19, 238, 61, | ||
107, 132, 211, 40, 244, 71, 190, 99, 14, 173, 225, 6, 156, | ||
])) | ||
.pool_max_size(42) | ||
.optimize(false) | ||
.cache_size(43) | ||
.journal_size_limit(44); | ||
|
||
assert_eq!(store_config.path, PathBuf::from("foo")); | ||
assert_eq!( | ||
store_config.secret, | ||
Some(Secret::Key( | ||
[ | ||
143, 27, 202, 78, 96, 55, 13, 149, 247, 8, 33, 120, 204, 92, 171, 66, 19, 238, | ||
61, 107, 132, 211, 40, 244, 71, 190, 99, 14, 173, 225, 6, 156, | ||
] | ||
.to_owned() | ||
)) | ||
); | ||
assert_eq!(store_config.pool_config.max_size, 42); | ||
assert!(store_config.runtime_config.optimize.not()); | ||
assert_eq!(store_config.runtime_config.cache_size, 43); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.