Skip to content

Use transaction mechanism instead of explicit BEGIN/COMMIT #264

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Use transaction mechanism instead of explicit BEGIN/COMMIT #264

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 56 additions & 64 deletions stack-graphs/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,56 +139,56 @@ pub struct SQLiteWriter {
impl SQLiteWriter {
/// Open an in-memory database.
pub fn open_in_memory() -> Result<Self> {
let conn = Connection::open_in_memory()?;
Self::init(&conn)?;
let mut conn = Connection::open_in_memory()?;
Self::init(&mut conn)?;
Ok(Self { conn })
}

/// Open a file database. If the file does not exist, it is automatically created.
/// An error is returned if the database version is not supported.
pub fn open<P: AsRef<Path>>(path: P) -> Result<Self> {
let is_new = !path.as_ref().exists();
let conn = Connection::open(path)?;
let mut conn = Connection::open(path)?;
set_pragmas_and_functions(&conn)?;
if is_new {
Self::init(&conn)?;
Self::init(&mut conn)?;
} else {
check_version(&conn)?;
}
Ok(Self { conn })
}

/// Create database tables and write metadata.
fn init(conn: &Connection) -> Result<()> {
conn.execute("BEGIN", [])?;
conn.execute_batch(SCHEMA)?;
conn.execute("INSERT INTO metadata (version) VALUES (?)", [VERSION])?;
conn.execute("COMMIT", [])?;
fn init(conn: &mut Connection) -> Result<()> {
let tx = conn.transaction()?;
tx.execute_batch(SCHEMA)?;
tx.execute("INSERT INTO metadata (version) VALUES (?)", [VERSION])?;
tx.commit()?;
Ok(())
}

/// Clean all data from the database.
pub fn clean_all(&mut self) -> Result<usize> {
self.conn.execute("BEGIN", [])?;
let count = self.clean_all_inner()?;
self.conn.execute("COMMIT", [])?;
let tx = self.conn.transaction()?;
let count = Self::clean_all_inner(&tx)?;
tx.commit()?;
Ok(count)
}

/// Clean all data from the database.
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn clean_all_inner(&mut self) -> Result<usize> {
fn clean_all_inner(conn: &Connection) -> Result<usize> {
{
let mut stmt = self.conn.prepare_cached("DELETE FROM file_paths")?;
let mut stmt = conn.prepare_cached("DELETE FROM file_paths")?;
stmt.execute([])?;
}
{
let mut stmt = self.conn.prepare_cached("DELETE FROM root_paths")?;
let mut stmt = conn.prepare_cached("DELETE FROM root_paths")?;
stmt.execute([])?;
}
let count = {
let mut stmt = self.conn.prepare_cached("DELETE FROM graphs")?;
let mut stmt = conn.prepare_cached("DELETE FROM graphs")?;
stmt.execute([])?
};
Ok(count)
Expand All @@ -197,33 +197,27 @@ impl SQLiteWriter {
/// Clean file data from the database. If recursive is true, data for all descendants of
/// that file is cleaned.
pub fn clean_file(&mut self, file: &Path) -> Result<usize> {
self.conn.execute("BEGIN", [])?;
let count = self.clean_file_inner(file)?;
self.conn.execute("COMMIT", [])?;
let tx = self.conn.transaction()?;
let count = Self::clean_file_inner(&tx, file)?;
tx.commit()?;
Ok(count)
}

/// Clean file data from the database.
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn clean_file_inner(&mut self, file: &Path) -> Result<usize> {
fn clean_file_inner(conn: &Connection, file: &Path) -> Result<usize> {
let file = file.to_string_lossy();
{
let mut stmt = self
.conn
.prepare_cached("DELETE FROM file_paths WHERE file=?")?;
let mut stmt = conn.prepare_cached("DELETE FROM file_paths WHERE file=?")?;
stmt.execute([&file])?;
}
{
let mut stmt = self
.conn
.prepare_cached("DELETE FROM root_paths WHERE file=?")?;
let mut stmt = conn.prepare_cached("DELETE FROM root_paths WHERE file=?")?;
stmt.execute([&file])?;
}
let count = {
let mut stmt = self
.conn
.prepare_cached("DELETE FROM graphs WHERE file=?")?;
let mut stmt = conn.prepare_cached("DELETE FROM graphs WHERE file=?")?;
stmt.execute([&file])?
};
Ok(count)
Expand All @@ -232,55 +226,56 @@ impl SQLiteWriter {
/// Clean file or directory data from the database. Data for all decendants of the given path
/// is cleaned.
pub fn clean_file_or_directory(&mut self, file_or_directory: &Path) -> Result<usize> {
self.conn.execute("BEGIN", [])?;
let count = self.clean_file_or_directory_inner(file_or_directory)?;
self.conn.execute("COMMIT", [])?;
let tx = self.conn.transaction()?;
let count = Self::clean_file_or_directory_inner(&tx, file_or_directory)?;
tx.commit()?;
Ok(count)
}

/// Clean file or directory data from the database. Data for all decendants of the given path
/// is cleaned.
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn clean_file_or_directory_inner(&mut self, file_or_directory: &Path) -> Result<usize> {
fn clean_file_or_directory_inner(conn: &Connection, file_or_directory: &Path) -> Result<usize> {
let file_or_directory = file_or_directory.to_string_lossy();
{
let mut stmt = self
.conn
.prepare_cached("DELETE FROM file_paths WHERE path_descendant_of(file, ?)")?;
let mut stmt =
conn.prepare_cached("DELETE FROM file_paths WHERE path_descendant_of(file, ?)")?;
stmt.execute([&file_or_directory])?;
}
{
let mut stmt = self
.conn
.prepare_cached("DELETE FROM root_paths WHERE path_descendant_of(file, ?)")?;
let mut stmt =
conn.prepare_cached("DELETE FROM root_paths WHERE path_descendant_of(file, ?)")?;
stmt.execute([&file_or_directory])?;
}
let count = {
let mut stmt = self
.conn
.prepare_cached("DELETE FROM graphs WHERE path_descendant_of(file, ?)")?;
let mut stmt =
conn.prepare_cached("DELETE FROM graphs WHERE path_descendant_of(file, ?)")?;
stmt.execute([&file_or_directory])?
};
Ok(count)
}

/// Store an error, indicating that indexing this file failed.
pub fn store_error_for_file(&mut self, file: &Path, tag: &str, error: &str) -> Result<()> {
self.conn.execute("BEGIN", [])?;
self.store_error_for_file_inner(file, tag, error)?;
self.conn.execute("COMMIT", [])?;
let tx = self.conn.transaction()?;
Self::store_error_for_file_inner(&tx, file, tag, error)?;
tx.commit()?;
Ok(())
}

/// Store an error, indicating that indexing this file failed.
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn store_error_for_file_inner(&mut self, file: &Path, tag: &str, error: &str) -> Result<()> {
fn store_error_for_file_inner(
conn: &Connection,
file: &Path,
tag: &str,
error: &str,
) -> Result<()> {
copious_debugging!("--> Store error for {}", file.display());
let mut stmt = self
.conn
.prepare_cached("INSERT INTO graphs (file, tag, error, json) VALUES (?, ?, ?, ?)")?;
let mut stmt =
conn.prepare_cached("INSERT INTO graphs (file, tag, error, json) VALUES (?, ?, ?, ?)")?;
let graph = crate::serde::StackGraph::default();
stmt.execute((
&file.to_string_lossy(),
Expand All @@ -304,28 +299,27 @@ impl SQLiteWriter {
IP: IntoIterator<Item = &'a PartialPath>,
{
let path = Path::new(graph[file].name());
self.conn.execute("BEGIN", [])?;
self.clean_file_inner(path)?;
self.store_graph_for_file_inner(graph, file, tag)?;
self.store_partial_paths_for_file_inner(graph, file, partials, paths)?;
self.conn.execute("COMMIT", [])?;
let tx = self.conn.transaction()?;
Self::clean_file_inner(&tx, path)?;
Self::store_graph_for_file_inner(&tx, graph, file, tag)?;
Self::store_partial_paths_for_file_inner(&tx, graph, file, partials, paths)?;
tx.commit()?;
Ok(())
}

/// Store the file graph.
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn store_graph_for_file_inner(
&mut self,
conn: &Connection,
graph: &StackGraph,
file: Handle<File>,
tag: &str,
) -> Result<()> {
let file_str = graph[file].name();
copious_debugging!("--> Store graph for {}", file_str);
let mut stmt = self
.conn
.prepare_cached("INSERT INTO graphs (file, tag, json) VALUES (?, ?, ?)")?;
let mut stmt =
conn.prepare_cached("INSERT INTO graphs (file, tag, json) VALUES (?, ?, ?)")?;
let graph = serde::StackGraph::from_graph_filter(graph, &FileFilter(file));
stmt.execute((file_str, tag, &serde_json::to_vec(&graph)?))?;
Ok(())
Expand All @@ -335,7 +329,7 @@ impl SQLiteWriter {
///
/// This is an inner method, which does not wrap individual SQL statements in a transaction.
fn store_partial_paths_for_file_inner<'a, IP>(
&mut self,
conn: &Connection,
graph: &StackGraph,
file: Handle<File>,
partials: &mut PartialPaths,
Expand All @@ -345,11 +339,9 @@ impl SQLiteWriter {
IP: IntoIterator<Item = &'a PartialPath>,
{
let file_str = graph[file].name();
let mut node_stmt = self
.conn
.prepare_cached("INSERT INTO file_paths (file, local_id, json) VALUES (?, ?, ?)")?;
let mut root_stmt = self
.conn
let mut node_stmt =
conn.prepare_cached("INSERT INTO file_paths (file, local_id, json) VALUES (?, ?, ?)")?;
let mut root_stmt = conn
.prepare_cached("INSERT INTO root_paths (file, symbol_stack, json) VALUES (?, ?, ?)")?;
#[cfg_attr(not(feature = "copious-debugging"), allow(unused))]
let mut node_path_count = 0usize;
Expand Down