fix(bans): backfill tracked members + harden SQLite concurrency - #405
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability of the bans/tracked-members subsystem by adding a backfill routine for tracked_members, a new “bulk” upsert API for tracked members, and more robust SQLite connection options to reduce lock contention under concurrency.
Changes:
- Spawn a startup backfill task to populate
tracked_membersfrom the guild member list. - Add
bulk_upsert_tracked_membersto upsert many tracked-member rows within a single transaction. - Switch SQLite initialization to
SqliteConnectOptionswith WAL, normal synchronous, and a busy timeout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/rustmail/src/handlers/ready_handler.rs | Starts the tracked-members backfill in a background task on ready(). |
| crates/rustmail/src/handlers/guild_ban_handler.rs | Implements backfill_tracked_members pagination and persistence. |
| crates/rustmail/src/db/operations/init.rs | Hardens SQLite connection settings (WAL/synchronous/busy timeout). |
| crates/rustmail/src/db/operations/banned_users.rs | Adds bulk_upsert_tracked_members using a transaction around repeated upserts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tokio::spawn({ | ||
| let ctx = ctx.clone(); | ||
| let config = config.clone(); | ||
|
|
||
| async move { | ||
| backfill_tracked_members(&ctx, &config).await; | ||
| } | ||
| }); |
| if let Err(e) = bulk_upsert_tracked_members(&tracked_batch, pool).await { | ||
| eprintln!( | ||
| "Failed to backfill tracked members page in guild {}: {:?}", | ||
| guild_id, e | ||
| ); | ||
| } | ||
|
|
||
| total += page_len; | ||
|
|
||
| if page_len < PAGE_LIMIT as usize { | ||
| break; | ||
| } | ||
|
|
||
| after = last_id; | ||
| } | ||
|
|
||
| println!( | ||
| "Backfilled {} tracked members for community guild {}", | ||
| total, guild_id | ||
| ); |
| pub async fn bulk_upsert_tracked_members( | ||
| members: &[TrackedMember], | ||
| pool: &SqlitePool, | ||
| ) -> ModmailResult<()> { | ||
| if members.is_empty() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let mut tx = pool.begin().await.map_err(|e| { | ||
| eprintln!("Failed to begin tracked members transaction: {e:?}"); | ||
| validation_failed("Failed to begin tracked members transaction") | ||
| })?; | ||
|
|
||
| for member in members { | ||
| let roles_json = serde_json::to_string(&member.roles) | ||
| .map_err(|_| validation_failed("Failed to serialize member roles"))?; | ||
|
|
||
| sqlx::query( | ||
| r#" | ||
| INSERT INTO tracked_members | ||
| (guild_id, user_id, username, global_name, nickname, avatar_url, roles, | ||
| joined_at, first_seen_at, last_seen_at) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
| ON CONFLICT(guild_id, user_id) DO UPDATE SET | ||
| username = excluded.username, | ||
| global_name = excluded.global_name, | ||
| nickname = excluded.nickname, | ||
| avatar_url = excluded.avatar_url, | ||
| roles = excluded.roles, | ||
| joined_at = COALESCE(excluded.joined_at, tracked_members.joined_at), | ||
| last_seen_at = excluded.last_seen_at | ||
| "#, | ||
| ) | ||
| .bind(&member.guild_id) | ||
| .bind(&member.user_id) | ||
| .bind(&member.username) | ||
| .bind(&member.global_name) | ||
| .bind(&member.nickname) | ||
| .bind(&member.avatar_url) | ||
| .bind(&roles_json) | ||
| .bind(member.joined_at) | ||
| .bind(member.first_seen_at) | ||
| .bind(member.last_seen_at) | ||
| .execute(&mut *tx) | ||
| .await | ||
| .map_err(|e| { | ||
| eprintln!("Failed to upsert tracked member in bulk: {e:?}"); | ||
| validation_failed("Failed to upsert tracked member in bulk") | ||
| })?; | ||
| } |
|
@copilot apply changes based on the comments in this thread |
…w upsert via QueryBuilder Agent-Logs-Url: https://github.com/Rustmail/rustmail/sessions/8364de61-149d-4799-a0f7-1669d6580724 Co-authored-by: Akinator31 <99099121+Akinator31@users.noreply.github.com>
Applied all three changes from the review thread in commit 8dc180a:
|
This pull request introduces improvements to how tracked guild members are managed and backfilled in the database, as well as enhancements to database connection settings for better reliability and performance. The main changes include adding a bulk upsert function for tracked members, implementing a backfill routine to populate the tracked members table, and configuring SQLite with more robust options.
Tracked Member Management:
bulk_upsert_tracked_membersfunction to efficiently insert or update multipleTrackedMemberrecords in a single transaction, reducing database overhead and improving consistency.backfill_tracked_membersasync function, which paginates through all guild members and uses the new bulk upsert to populate thetracked_memberstable. This is useful for initializing or repairing the tracked members dataset.ReadyHandler.Database Connection Improvements:
SqliteConnectOptionswith Write-Ahead Logging (WAL) mode, normal synchronous setting, and a busy timeout for improved reliability under load.Miscellaneous: