Adopt Turso-inspired performance patterns for codebase databases#491
Adopt Turso-inspired performance patterns for codebase databases#491iberi22 wants to merge 1 commit into
Conversation
- Refactor ConnectionManager to use moka::future::Cache for LRU connection pooling. - Implement lazy schema initialization in CodebaseDb and ConversationsDb via OnceCell. - Add batch insert methods to CodebaseDb using libSQL transactions. - Optimize FTS population and database connection logic for async efficiency. - Enhance path sanitization for per-project conversation databases.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the connection management from synchronous r2d2 pooling to an asynchronous moka::future::Cache using libsql::Connection. It also introduces lazy schema initialization via tokio::sync::OnceCell across ConversationsDb and CodebaseDb, alongside new batch insertion methods for chunks, symbols, and embeddings. Feedback on these changes highlights a critical compilation error in ConnectionManager::connect due to a lifetime mismatch when capturing a borrowed string slice in the cache initialization future, as well as panic risks from using .expect() instead of propagating errors via try_get_with.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub async fn connect(&self, project_id: &str, project_root: &str) -> Result<Connection> { | ||
| let project_id_owned = project_id.to_string(); | ||
| let project_root_owned = project_root.to_string(); | ||
|
|
||
| let conn_res = self.connections | ||
| .get_with(project_id_owned, async move { | ||
| let db_path = if project_id == "memory" { | ||
| PathBuf::from(project_root_owned).join("xavier_memory.db") | ||
| } else if project_id == "vec_store" { | ||
| PathBuf::from(project_root_owned).join("vec-store.sqlite3") | ||
| } else if project_id == "metrics" { | ||
| PathBuf::from(project_root_owned).join("metrics.db") | ||
| } else if project_id.starts_with("conv_") { | ||
| let pid = project_id.strip_prefix("conv_").unwrap(); | ||
| crate::codebase::conversations_db::ConversationsDb::db_path(pid) | ||
| } else { | ||
| PathBuf::from(project_root_owned) | ||
| .join(".xavier") | ||
| .join("codebase.db") | ||
| }; | ||
|
|
||
| if let Some(parent) = db_path.parent() { | ||
| let parent = parent.to_path_buf(); | ||
| tokio::task::spawn_blocking(move || { | ||
| std::fs::create_dir_all(parent) | ||
| }).await | ||
| .expect("spawn_blocking panicked") | ||
| .expect("failed to create db directory"); | ||
| } | ||
|
|
||
| let db = libsql::Builder::new_local(db_path.to_string_lossy().to_string()) | ||
| .build() | ||
| .await | ||
| .expect("failed to build libsql db"); | ||
| let conn = db.connect().expect("failed to connect to libsql db"); | ||
|
|
||
| conn.execute_batch( | ||
| "PRAGMA journal_mode=WAL; \ | ||
| PRAGMA busy_timeout=5000; \ | ||
| PRAGMA synchronous=NORMAL;", | ||
| ) | ||
| .await | ||
| .expect("failed to set pragmas"); | ||
|
|
||
| conn | ||
| }) | ||
| .await; | ||
|
|
||
| Ok(conn_res) | ||
| } |
There was a problem hiding this comment.
Critical Compilation & Safety Issues
- Lifetime / Compilation Error: The
async moveblock passed toget_withcapturesproject_id(which is a&strwith a lifetime bound to the function call). Sincemoka::future::Cache::get_withrequires the future to be'static, this will cause a compilation error (borrowed value does not live long enough). To fix this, we must use an owned clone of the project ID (e.g.,project_id_for_init) inside the async block. - Panic Risk: Using
.expect()inside the cache initialization block will panic the entire thread/task if any database or file system operation fails (e.g., disk full, permission denied). We should usetry_get_withinstead ofget_withto gracefully propagate errors asanyhow::Result.
pub async fn connect(&self, project_id: &str, project_root: &str) -> Result<Connection> {
let project_id_owned = project_id.to_string();
let project_id_for_init = project_id_owned.clone();
let project_root_owned = project_root.to_string();
let conn_res = self.connections
.try_get_with(project_id_owned, async move {
let db_path = if project_id_for_init == "memory" {
PathBuf::from(project_root_owned).join("xavier_memory.db")
} else if project_id_for_init == "vec_store" {
PathBuf::from(project_root_owned).join("vec-store.sqlite3")
} else if project_id_for_init == "metrics" {
PathBuf::from(project_root_owned).join("metrics.db")
} else if project_id_for_init.starts_with("conv_") {
let pid = project_id_for_init.strip_prefix("conv_").unwrap();
crate::codebase::conversations_db::ConversationsDb::db_path(pid)
} else {
PathBuf::from(project_root_owned)
.join(".xavier")
.join("codebase.db")
};
if let Some(parent) = db_path.parent() {
let parent = parent.to_path_buf();
tokio::task::spawn_blocking(move || {
std::fs::create_dir_all(parent)
}).await
.context("spawn_blocking panicked")?
.context("failed to create db directory")?;
}
let db = libsql::Builder::new_local(db_path.to_string_lossy().to_string())
.build()
.await
.context("failed to build libsql db")?;
let conn = db.connect().context("failed to connect to libsql db")?;
conn.execute_batch(
"PRAGMA journal_mode=WAL; \\
PRAGMA busy_timeout=5000; \\
PRAGMA synchronous=NORMAL;",
)
.await
.context("failed to set pragmas")?;
Ok::<_, anyhow::Error>(conn)
})
.await
.map_err(|e| anyhow::anyhow!("failed to connect: {}", e))?;
Ok(conn_res)
}
iberi22
added
jules
1 participant
Improved Xavier's database performance by implementing patterns inspired by Turso: batch inserts with transactions, lazy schema creation, and an LRU connection cache using
moka. These changes reduce resource usage and speed up repository indexing.Fixes #427
PR created automatically by Jules for task 10752104400361843081 started by @iberi22