Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions crates/matrix-sdk-sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ All notable changes to this project will be documented in this file.

- Implement the new method of `EventCacheStoreMedia` for `SqliteEventCacheStore`.
([#4603](https://github.com/matrix-org/matrix-rust-sdk/pull/4603))
- Defragment an sqlite state store after removing a room.
([#4651](https://github.com/matrix-org/matrix-rust-sdk/pull/4651))

## [0.10.0] - 2025-02-04

Expand Down
17 changes: 3 additions & 14 deletions crates/matrix-sdk-sqlite/src/event_cache_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ use matrix_sdk_store_encryption::StoreCipher;
use ruma::{time::SystemTime, MilliSecondsSinceUnixEpoch, MxcUri, RoomId};
use rusqlite::{params_from_iter, OptionalExtension, Transaction, TransactionBehavior};
use tokio::fs;
#[cfg(not(test))]
use tracing::warn;
use tracing::{debug, trace};

use crate::{
Expand Down Expand Up @@ -959,19 +957,10 @@ impl EventCacheStoreMedia for SqliteEventCacheStore {
})
.await?;

// If we removed media, use the VACUUM command to defragment the
// database and free space on the filesystem.
// If we removed media, defragment the database and free space on the
// filesystem.
if removed {
if let Err(error) = conn.execute("VACUUM", ()).await {
// Since this is an optimisation step, do not propagate the error
// but log it.
#[cfg(not(test))]
warn!("Failed to vacuum database: {error}");

// We want to know if there is an error with this step during tests.
#[cfg(test)]
return Err(error.into());
}
conn.vacuum().await?;
Copy link
Member

Choose a reason for hiding this comment

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

Preexisting, but does it make sense to vacuum all the time? If only a single media has been removed, it's probably not worth it — especially as vacuuming requires doubling the size of the database on disk, since it creates a copy of the database while it does it. Unfortunately that means heuristics to decide when to trigger, then, and maybe over a certain number of files / total size on disk, that would be profitable and simple enough to trigger often?

}

Ok(())
Expand Down
54 changes: 28 additions & 26 deletions crates/matrix-sdk-sqlite/src/state_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ impl SqliteStateStore {
// Defragment the DB and optimize its size on the filesystem.
// This should have been run in the migration for version 7, to reduce the size
// of the DB as we removed the media cache.
conn.execute_batch("VACUUM").await?;
conn.vacuum().await?;
conn.set_kv("version", vec![12]).await?;
}

Expand Down Expand Up @@ -1739,40 +1739,42 @@ impl StateStore for SqliteStateStore {
let this = self.clone();
let room_id = room_id.to_owned();

self.acquire()
.await?
.with_transaction(move |txn| {
let room_info_room_id = this.encode_key(keys::ROOM_INFO, &room_id);
txn.remove_room_info(&room_info_room_id)?;
let conn = self.acquire().await?;

let state_event_room_id = this.encode_key(keys::STATE_EVENT, &room_id);
txn.remove_room_state_events(&state_event_room_id, None)?;
conn.with_transaction(move |txn| -> Result<()> {
let room_info_room_id = this.encode_key(keys::ROOM_INFO, &room_id);
txn.remove_room_info(&room_info_room_id)?;

let member_room_id = this.encode_key(keys::MEMBER, &room_id);
txn.remove_room_members(&member_room_id, None)?;
let state_event_room_id = this.encode_key(keys::STATE_EVENT, &room_id);
txn.remove_room_state_events(&state_event_room_id, None)?;

let profile_room_id = this.encode_key(keys::PROFILE, &room_id);
txn.remove_room_profiles(&profile_room_id)?;
let member_room_id = this.encode_key(keys::MEMBER, &room_id);
txn.remove_room_members(&member_room_id, None)?;

let room_account_data_room_id = this.encode_key(keys::ROOM_ACCOUNT_DATA, &room_id);
txn.remove_room_account_data(&room_account_data_room_id)?;
let profile_room_id = this.encode_key(keys::PROFILE, &room_id);
txn.remove_room_profiles(&profile_room_id)?;

let receipt_room_id = this.encode_key(keys::RECEIPT, &room_id);
txn.remove_room_receipts(&receipt_room_id)?;
let room_account_data_room_id = this.encode_key(keys::ROOM_ACCOUNT_DATA, &room_id);
txn.remove_room_account_data(&room_account_data_room_id)?;

let display_name_room_id = this.encode_key(keys::DISPLAY_NAME, &room_id);
txn.remove_room_display_names(&display_name_room_id)?;
let receipt_room_id = this.encode_key(keys::RECEIPT, &room_id);
txn.remove_room_receipts(&receipt_room_id)?;

let send_queue_room_id = this.encode_key(keys::SEND_QUEUE, &room_id);
txn.remove_room_send_queue(&send_queue_room_id)?;
let display_name_room_id = this.encode_key(keys::DISPLAY_NAME, &room_id);
txn.remove_room_display_names(&display_name_room_id)?;

let dependent_send_queue_room_id =
this.encode_key(keys::DEPENDENTS_SEND_QUEUE, &room_id);
txn.remove_room_dependent_send_queue(&dependent_send_queue_room_id)?;
let send_queue_room_id = this.encode_key(keys::SEND_QUEUE, &room_id);
txn.remove_room_send_queue(&send_queue_room_id)?;

Ok(())
})
.await
let dependent_send_queue_room_id =
this.encode_key(keys::DEPENDENTS_SEND_QUEUE, &room_id);
txn.remove_room_dependent_send_queue(&dependent_send_queue_room_id)?;

Ok(())
})
.await?;

conn.vacuum().await
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we can vacuum as part of the transaction, or if it would lead to more trouble? It's not clear from the sqlite doc…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it's not possible it returns an error.

}

async fn save_send_queue_request(
Expand Down
18 changes: 18 additions & 0 deletions crates/matrix-sdk-sqlite/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ pub(crate) trait SqliteAsyncConnExt {
self.execute_batch("PRAGMA journal_size_limit = 10000000;").await.map_err(Error::from)?;
Ok(())
}

/// Defragment the database and free space on the filesystem.
///
/// Only returns an error in tests, otherwise the error is only logged.
async fn vacuum(&self) -> Result<()> {
if let Err(error) = self.execute_batch("VACUUM").await {
// Since this is an optimisation step, do not propagate the error
// but log it.
#[cfg(not(any(test, debug_assertions)))]
tracing::warn!("Failed to vacuum database: {error}");

// We want to know if there is an error with this step during tests.
#[cfg(any(test, debug_assertions))]
return Err(error.into());
}

Ok(())
}
}

#[async_trait]
Expand Down
Loading