Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
cli/node: Improve log rotation
Merged lorenz opened 9 months ago

Only the latest log was kept as node.log.old. Now, log files are numbered (node.log.1, node.log.2 and so on). Also node.log is now a hard link to the current log file. rad node stop will delete node.log (note that node.log.x stays intact) and rad node start will delete node.log before it creates node.log.y in case the node crashed.

Also, when running in foreground mode, now a log file is created. It just contains the hint that the node was started in foreground mode, just to avoid confusion.

3 files changed +196 -29 3d352f23 4934473b
modified crates/radicle-cli/src/commands/node.rs
@@ -340,7 +340,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
            control::status(&node, &profile)?;
        }
        Operation::Stop => {
-
            control::stop(node)?;
+
            control::stop(node, &profile);
        }
    }

modified crates/radicle-cli/src/commands/node/control.rs
@@ -1,8 +1,11 @@
+
mod logs;
+
use logs::{LogRotatorFileSystem, Rotated};
+

use std::collections::HashMap;
use std::ffi::OsString;
-
use std::fs::{File, OpenOptions};
-
use std::io::{BufRead, BufReader, Read, Seek, SeekFrom};
-
use std::{fs, io, path::Path, process, thread, time};
+
use std::fs::File;
+
use std::io::{BufRead, BufReader, Read, Seek, SeekFrom, Write};
+
use std::{path::Path, process, thread, time};

use anyhow::{anyhow, Context};
use localtime::LocalTime;
@@ -17,10 +20,6 @@ use crate::terminal::Element as _;

/// How long to wait for the node to start before returning an error.
pub const NODE_START_TIMEOUT: time::Duration = time::Duration::from_secs(6);
-
/// Node log file name.
-
pub const NODE_LOG: &str = "node.log";
-
/// Node log old file name, after rotation.
-
pub const NODE_LOG_OLD: &str = "node.log.old";

pub fn start(
    node: Node,
@@ -55,14 +54,19 @@ pub fn start(
    if !options.contains(&OsString::from("--force")) {
        options.push(OsString::from("--force"));
    }
+

+
    let Rotated {
+
        path: log_path,
+
        log: log_file,
+
    } = LogRotatorFileSystem::from_profile(profile).rotate()?;
+

    if daemon {
-
        let log = log_rotate(profile)?;
        let child = process::Command::new(cmd)
            .args(options)
            .envs(envs)
            .stdin(process::Stdio::null())
-
            .stdout(process::Stdio::from(log.try_clone()?))
-
            .stderr(process::Stdio::from(log))
+
            .stdout(process::Stdio::from(log_file.try_clone()?))
+
            .stderr(process::Stdio::from(log_file))
            .spawn()
            .map_err(|e| anyhow!("failed to start node process {cmd:?}: {e}"))?;
        let pid = term::format::parens(term::format::dim(child.id()));
@@ -98,6 +102,10 @@ pub fn start(
            }
        }
    } else {
+
        // Write a hint to the log file, but swallow any errors.
+
        let mut log_file = log_file;
+
        let _ = log_file.write_all(format!("radicle-node started in foreground, no futher log messages are written to '{}' (this file).\n", log_path.display()).as_bytes());
+

        let mut child = process::Command::new(cmd)
            .args(options)
            .envs(envs)
@@ -110,7 +118,7 @@ pub fn start(
    Ok(())
}

-
pub fn stop(node: Node) -> anyhow::Result<()> {
+
pub fn stop(node: Node, profile: &Profile) {
    let mut spinner = term::spinner("Stopping node...");
    if node.shutdown().is_err() {
        spinner.error("node is not running");
@@ -118,7 +126,8 @@ pub fn stop(node: Node) -> anyhow::Result<()> {
        spinner.message("Node stopped");
        spinner.finish();
    }
-
    Ok(())
+
    let rotator = LogRotatorFileSystem::from_profile(profile);
+
    rotator.remove().ok();
}

pub fn debug(node: &mut Node) -> anyhow::Result<()> {
@@ -394,22 +403,6 @@ pub fn config(node: &Node) -> anyhow::Result<()> {
    Ok(())
}

-
fn log_rotate(profile: &Profile) -> io::Result<File> {
-
    let base = profile.home.node();
-
    if base.join(NODE_LOG).exists() {
-
        // Let this fail, eg. if the file doesn't exist.
-
        fs::remove_file(base.join(NODE_LOG_OLD)).ok();
-
        fs::rename(base.join(NODE_LOG), base.join(NODE_LOG_OLD))?;
-
    }
-

-
    let log = OpenOptions::new()
-
        .write(true)
-
        .create_new(true)
-
        .open(base.join(NODE_LOG))?;
-

-
    Ok(log)
-
}
-

fn state_label() -> term::Paint<String> {
    term::Paint::from("?".to_string())
}
added crates/radicle-cli/src/commands/node/logs.rs
@@ -0,0 +1,174 @@
+
use std::collections::BinaryHeap;
+
use std::fs::{File, OpenOptions};
+
use std::path::PathBuf;
+
use std::{fs, io};
+

+
use radicle::Profile;
+

+
/// [`LogRotator`] manages the rotation of the log files when the Radicle node
+
/// is run in the background, without being managed by a service manager.
+
pub struct LogRotator {
+
    /// The base path where the logs should live.
+
    base: PathBuf,
+
    /// All existing logs, identified by a suffix.
+
    existing_logs: BinaryHeap<u16>,
+
}
+

+
impl LogRotator {
+
    /// Node log file name.
+
    pub const NODE_LOG: &str = "node.log";
+
    /// Node log old file name, after rotation.
+
    pub const NODE_LOG_OLD_PREFIX: &str = "node.log.";
+

+
    /// Construct a new [`LogRotator`] with the give `base` path.
+
    pub fn new(base: PathBuf) -> Self {
+
        Self {
+
            base,
+
            existing_logs: BinaryHeap::new(),
+
        }
+
    }
+

+
    /// Add a set of existing suffixes to known logs.
+
    pub fn found_logs(&mut self, suffixes: impl Iterator<Item = u16>) {
+
        self.existing_logs.extend(suffixes);
+
    }
+

+
    /// Specify that the [`LogRotator::current`] log should be removed.
+
    ///
+
    /// Returns `None` if the file does not exist.
+
    pub fn remove_current(&self) -> Option<Remove> {
+
        let current = self.current();
+
        current.exists().then_some(Remove { current })
+
    }
+

+
    /// Specify that the logs should be rotated.
+
    pub fn rotate(&self) -> Rotate {
+
        let next = self.next_log();
+
        let remove = self.remove_current();
+
        Rotate {
+
            next,
+
            link: self.current(),
+
            remove,
+
        }
+
    }
+

+
    /// The current log file that should be logged to.
+
    pub fn current(&self) -> PathBuf {
+
        self.base.join(Self::NODE_LOG)
+
    }
+

+
    fn next_log(&self) -> PathBuf {
+
        let suffix = self
+
            .existing_logs
+
            .peek()
+
            .copied()
+
            .unwrap_or(0)
+
            .saturating_add(1);
+
        self.base
+
            .join(Self::NODE_LOG_OLD_PREFIX.to_owned() + suffix.to_string().as_str())
+
    }
+
}
+

+
/// A [`LogRotator`] that implements the removal and rotation by accessing the
+
/// filesystem.
+
pub struct LogRotatorFileSystem {
+
    rotator: LogRotator,
+
}
+

+
impl LogRotatorFileSystem {
+
    /// Create a new [`LogRotatorFileSystem`] from a [`Profile`].
+
    ///
+
    /// The [`LogRotator`]'s base path will be the node path.
+
    pub fn from_profile(profile: &Profile) -> Self {
+
        Self {
+
            rotator: LogRotator::new(profile.home.node()),
+
        }
+
    }
+

+
    /// Rotate the log files, returning [`Rotated`].
+
    pub fn rotate(mut self) -> io::Result<Rotated> {
+
        self.rotator.found_logs(self.existing_logs().into_iter());
+
        self.rotator.rotate().execute()
+
    }
+

+
    /// Remove the current log file, returning `true` if the file existed and
+
    /// was removed.
+
    pub fn remove(self) -> io::Result<bool> {
+
        self.rotator
+
            .remove_current()
+
            .map(|remove| remove.execute())
+
            .transpose()
+
            .map(|res| res.is_some())
+
    }
+

+
    fn parse_suffix(filename: String) -> Option<u16> {
+
        filename
+
            .strip_prefix(LogRotator::NODE_LOG_OLD_PREFIX)
+
            .and_then(|suffix| suffix.parse::<u16>().ok())
+
    }
+

+
    fn existing_logs(&self) -> BinaryHeap<u16> {
+
        self.rotator
+
            .base
+
            .read_dir()
+
            .ok()
+
            .map(|dir| {
+
                dir.filter_map(Result::ok)
+
                    .filter_map(|entry| entry.file_name().into_string().ok())
+
                    .filter_map(Self::parse_suffix)
+
                    .collect()
+
            })
+
            .unwrap_or_default()
+
    }
+
}
+

+
/// Remove the path identified by [`Remove::current`].
+
pub struct Remove {
+
    current: PathBuf,
+
}
+

+
impl Remove {
+
    /// Use [`fs::remove_file`] to remove the file.
+
    pub fn execute(self) -> io::Result<()> {
+
        fs::remove_file(self.current)
+
    }
+
}
+

+
/// Rotate the logs to the next log.
+
pub struct Rotate {
+
    /// The next log that needs to be created
+
    next: PathBuf,
+
    /// The path to create a hard link to.
+
    link: PathBuf,
+
    /// If the current log exists, then we need to remove it
+
    remove: Option<Remove>,
+
}
+

+
impl Rotate {
+
    /// Remove the existing file, if it exists. Then create the next log, and
+
    /// create a hard link to it.
+
    pub fn execute(self) -> io::Result<Rotated> {
+
        if let Some(to_remove) = self.remove {
+
            if let Err(err) = to_remove.execute() {
+
                log::warn!(target: "cli", "Failed to remove current log file: {err}");
+
            }
+
        }
+
        let log = OpenOptions::new()
+
            .write(true)
+
            .create_new(true)
+
            .open(&self.next)?;
+
        fs::hard_link(&self.next, &self.link)?;
+
        Ok(Rotated {
+
            path: self.next,
+
            log,
+
        })
+
    }
+
}
+

+
/// The result of rotating the logs.
+
pub struct Rotated {
+
    /// The [`PathBuf`] to the new log file.
+
    pub path: PathBuf,
+
    /// The [`File`] handle for the log file.
+
    pub log: File,
+
}