mirror of
https://github.com/TheFunny/TelegramTwitterMediaBot.git
synced 2026-09-23 23:32:05 +00:00
perf: index the link-cache prune, evict chats with no live prompt
Two things the 300 s sweep did the hard way: - The link cache is pruned by `created_at` (`DELETE FROM link_cache WHERE created_at < ?`) and had no index on it, so every sweep scanned the whole table — every post sent inside the TTL window, which is up to a week of them — while the `url` primary key served none of it. A new migration (appended; migration 1 is frozen and already shipped) creates the index, and the upgrade test now asserts it exists after an upgrade. - `ChatStore::prune_expired` only ever *looked* at chats that had an expired edit-before-forward record, so a chat with no prompt at all — the common case: every chat that ever sent a message or ran a command — stayed in the cache and in the per-chat lock map for the process lifetime. The candidate set now includes chats holding no records, which is what the eviction below was written for; the DB keeps the row, so the next use costs one SELECT (pinned by a new test that also shows the durable settings come back). Deliberately *not* done: skipping the write in `ChatStore::set` when the state is unchanged. Comparing against the cached copy would skip a serialize plus a blocking DB round trip for a no-op update — but every one of the 13 `update` callers mutates something, so the no-op case is a user repeating an identical command, and the same comparison would also skip the write that repairs a row whose earlier write failed. A rare saving against a rare repair, and the write is what makes the cache a cache rather than a source of truth. Verified: the new eviction test fails without the candidate change (checked by reverting it) and passes with it; 118 bot tests and 91 x-media tests pass. `cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D warnings` and `cargo test --workspace --locked` clean.
This commit is contained in:
@@ -139,6 +139,11 @@ const MIGRATIONS: &[&str] = &[
|
|||||||
// written by its former holder — which used to duplicate a send or drop
|
// written by its former holder — which used to duplicate a send or drop
|
||||||
// the new holder's retry state, silently.
|
// the new holder's retry state, silently.
|
||||||
"ALTER TABLE tasks ADD COLUMN lease_token TEXT",
|
"ALTER TABLE tasks ADD COLUMN lease_token TEXT",
|
||||||
|
// 2: the 300 s sweep prunes the link cache by `created_at`
|
||||||
|
// (`DELETE FROM link_cache WHERE created_at < ?`). Without an index that
|
||||||
|
// is a full scan of every post sent inside the TTL window — up to a week
|
||||||
|
// of them — on every sweep; the `url` primary key cannot serve it.
|
||||||
|
"CREATE INDEX IF NOT EXISTS idx_link_cache_created_at ON link_cache(created_at)",
|
||||||
];
|
];
|
||||||
|
|
||||||
/// Brings an existing database up to [`MIGRATIONS`]. Idempotent: a database
|
/// Brings an existing database up to [`MIGRATIONS`]. Idempotent: a database
|
||||||
@@ -285,6 +290,18 @@ mod tests {
|
|||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(payload, "{\"chat_id\":1}", "rows survive the upgrade");
|
assert_eq!(payload, "{\"chat_id\":1}", "rows survive the upgrade");
|
||||||
|
// The link-cache prune's index arrives with the migrations (the
|
||||||
|
// baseline schema has none): without it every sweep scans the
|
||||||
|
// whole table.
|
||||||
|
let index: i64 = conn
|
||||||
|
.query_row(
|
||||||
|
"SELECT COUNT(*) FROM sqlite_master \
|
||||||
|
WHERE type = 'index' AND name = 'idx_link_cache_created_at'",
|
||||||
|
[],
|
||||||
|
|row| row.get(0),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(index, 1, "the migration's index must exist");
|
||||||
Ok(())
|
Ok(())
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
|
|||||||
@@ -131,18 +131,24 @@ impl ChatStore {
|
|||||||
pub async fn prune_expired(&self, ttl: Duration) -> Vec<(i64, i64)> {
|
pub async fn prune_expired(&self, ttl: Duration) -> Vec<(i64, i64)> {
|
||||||
let now = unix_now();
|
let now = unix_now();
|
||||||
let ttl_secs = ttl.as_secs() as i64;
|
let ttl_secs = ttl.as_secs() as i64;
|
||||||
// Chats that may have an expired record, from a cache snapshot; the
|
// Chats worth looking at, from a cache snapshot: the ones with an
|
||||||
// pruning itself re-reads and writes under the per-chat lock below
|
// expired record, plus the ones holding no record at all. The latter
|
||||||
// (see the eviction note). Takes no lock of its own, so a chat
|
// used to be left alone for the process lifetime — every chat that ever
|
||||||
// appearing later is simply picked up by the next sweep.
|
// sent a message or ran a command stayed in the cache and in the
|
||||||
|
// per-chat lock map — even though a chat with no live prompt is exactly
|
||||||
|
// what the eviction below is for. The pruning itself re-reads and
|
||||||
|
// writes under the per-chat lock below; taking no lock here means a
|
||||||
|
// chat appearing later is simply picked up by the next sweep.
|
||||||
let candidates: Vec<i64> = {
|
let candidates: Vec<i64> = {
|
||||||
let cache = self.cache.lock();
|
let cache = self.cache.lock();
|
||||||
cache
|
cache
|
||||||
.iter()
|
.iter()
|
||||||
.filter(|(_, data)| {
|
.filter(|(_, data)| {
|
||||||
data.edit_message
|
data.edit_message.is_empty()
|
||||||
.values()
|
|| data
|
||||||
.any(|entry| entry.created_at + ttl_secs <= now)
|
.edit_message
|
||||||
|
.values()
|
||||||
|
.any(|entry| entry.created_at + ttl_secs <= now)
|
||||||
})
|
})
|
||||||
.map(|(chat_id, _)| *chat_id)
|
.map(|(chat_id, _)| *chat_id)
|
||||||
.collect()
|
.collect()
|
||||||
@@ -265,6 +271,58 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn an_idle_chat_is_evicted_and_its_state_reloads() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let pool = crate::db::open_store(dir.path().join("e.db").to_str().unwrap()).unwrap();
|
||||||
|
let store = ChatStore::new(pool);
|
||||||
|
// Durable settings and no prompt at all: this chat used to sit in the
|
||||||
|
// cache (and in the per-chat lock map) for the process lifetime,
|
||||||
|
// because the sweep only ever looked at chats with an *expired* record.
|
||||||
|
store
|
||||||
|
.update(9, |data| {
|
||||||
|
data.forward_channel_id = Some(-100);
|
||||||
|
data.message_format.insert("twitter".into(), "{url}".into());
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
assert!(store.cache.lock().contains_key(&9));
|
||||||
|
|
||||||
|
let removed = store.prune_expired(Duration::from_secs(60)).await;
|
||||||
|
|
||||||
|
assert!(removed.is_empty(), "nothing had expired");
|
||||||
|
assert!(
|
||||||
|
!store.cache.lock().contains_key(&9),
|
||||||
|
"a chat with no live prompt must leave the cache"
|
||||||
|
);
|
||||||
|
assert!(!store.locks.lock().contains_key(&9), "…and its lock");
|
||||||
|
// The DB kept the row, so the next use reloads everything it held.
|
||||||
|
let data = store.get(9).await;
|
||||||
|
assert_eq!(data.forward_channel_id, Some(-100));
|
||||||
|
assert_eq!(
|
||||||
|
data.message_format.get("twitter").map(String::as_str),
|
||||||
|
Some("{url}")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_live_prompt_keeps_its_chat_cached() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let pool = crate::db::open_store(dir.path().join("k.db").to_str().unwrap()).unwrap();
|
||||||
|
let store = ChatStore::new(pool);
|
||||||
|
store
|
||||||
|
.update(10, |data| {
|
||||||
|
data.edit_message.insert(1, edit_entry(10, unix_now()));
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
store.prune_expired(Duration::from_secs(3600)).await;
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
store.cache.lock().contains_key(&10),
|
||||||
|
"a live prompt holds its chat in the cache"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn prune_eviction_keeps_the_persisted_state() {
|
async fn prune_eviction_keeps_the_persisted_state() {
|
||||||
// Every record expires → the chat is evicted from the cache; the
|
// Every record expires → the chat is evicted from the cache; the
|
||||||
|
|||||||
Reference in New Issue
Block a user