From 3f389f37ea8739d39e8068a52f3f309a0c141e93 Mon Sep 17 00:00:00 2001
From: Alexis Sellier <self@cloudhead.io>
Date: Fri, 31 Mar 2023 14:07:45 +0200
Subject: [PATCH] node: Make sure we retry on lock contention

This fixes an issue that came up in tests where we would be reading
and writing to the database at the same time, and it would fail with
an error due to the lock being contended on.
---
 radicle-node/src/service.rs        |  7 ++++---
 radicle/src/node/routing.rs        |  7 +++++--
 radicle/src/node/tracking/store.rs | 14 +++++++++++---
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs
index 0597b65c..b9856a53 100644
--- a/radicle-node/src/service.rs
+++ b/radicle-node/src/service.rs
@@ -824,7 +824,7 @@ where
                         }
                     }
                     Err(e) => {
-                        error!("Error processing inventory from {}: {}", announcer, e);
+                        error!(target: "service", "Error processing inventory from {}: {}", announcer, e);
                         return Ok(false);
                     }
                 }
@@ -969,7 +969,8 @@ where
                     Ok(updated) => {
                         // Only relay if we received new information.
                         if updated {
-                            debug!(target: "service",
+                            debug!(
+                                target: "service",
                                 "Address store entry for node {announcer} updated at {timestamp}"
                             );
                             return Ok(relay);
@@ -977,7 +978,7 @@ where
                     }
                     Err(err) => {
                         // An error here is due to a fault in our address store.
-                        error!("Error processing node announcement from {announcer}: {err}");
+                        error!(target: "service", "Error processing node announcement from {announcer}: {err}");
                     }
                 }
             }
diff --git a/radicle/src/node/routing.rs b/radicle/src/node/routing.rs
index 2eec8066..09fe5bc1 100644
--- a/radicle/src/node/routing.rs
+++ b/radicle/src/node/routing.rs
@@ -11,7 +11,9 @@ use crate::{
 };
 
 /// How long to wait for the database lock to be released before failing a read.
-const DB_READ_TIMEOUT: time::Duration = time::Duration::from_secs(1);
+const DB_READ_TIMEOUT: time::Duration = time::Duration::from_secs(3);
+/// How long to wait for the database lock to be released before failing a write.
+const DB_WRITE_TIMEOUT: time::Duration = time::Duration::from_secs(6);
 
 /// An error occuring in peer-to-peer networking code.
 #[derive(Error, Debug)]
@@ -41,7 +43,8 @@ impl Table {
     /// Open a routing file store at the given path. Creates a new empty store
     /// if an existing store isn't found.
     pub fn open<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
-        let db = sql::Connection::open(path)?;
+        let mut db = sql::Connection::open(path)?;
+        db.set_busy_timeout(DB_WRITE_TIMEOUT.as_millis() as usize)?;
         db.execute(Self::SCHEMA)?;
 
         Ok(Self { db })
diff --git a/radicle/src/node/tracking/store.rs b/radicle/src/node/tracking/store.rs
index 18a45088..d1067d7e 100644
--- a/radicle/src/node/tracking/store.rs
+++ b/radicle/src/node/tracking/store.rs
@@ -1,6 +1,6 @@
 #![allow(clippy::type_complexity)]
 use std::path::Path;
-use std::{fmt, io, ops::Not as _};
+use std::{fmt, io, ops::Not as _, time};
 
 use sqlite as sql;
 use thiserror::Error;
@@ -9,6 +9,11 @@ use crate::prelude::{Id, NodeId};
 
 use super::{Node, Policy, Repo, Scope};
 
+/// How long to wait for the database lock to be released before failing a read.
+const DB_READ_TIMEOUT: time::Duration = time::Duration::from_secs(3);
+/// How long to wait for the database lock to be released before failing a write.
+const DB_WRITE_TIMEOUT: time::Duration = time::Duration::from_secs(6);
+
 #[derive(Error, Debug)]
 pub enum Error {
     /// I/O error.
@@ -36,7 +41,8 @@ impl Config {
     /// Open a policy store at the given path. Creates a new store if it
     /// doesn't exist.
     pub fn open<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
-        let db = sql::Connection::open(path)?;
+        let mut db = sql::Connection::open(path)?;
+        db.set_busy_timeout(DB_WRITE_TIMEOUT.as_millis() as usize)?;
         db.execute(Self::SCHEMA)?;
 
         Ok(Self { db })
@@ -45,7 +51,9 @@ impl Config {
     /// Same as [`Self::open`], but in read-only mode. This is useful to have multiple
     /// open databases, as no locking is required.
     pub fn reader<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
-        let db = sql::Connection::open_with_flags(path, sqlite::OpenFlags::new().set_read_only())?;
+        let mut db =
+            sql::Connection::open_with_flags(path, sqlite::OpenFlags::new().set_read_only())?;
+        db.set_busy_timeout(DB_READ_TIMEOUT.as_millis() as usize)?;
         db.execute(Self::SCHEMA)?;
 
         Ok(Self { db })