Skip to content
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.

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

Core: Improve Cleanup Temporary Files in Chipmunk Home Directory #2142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions application/apps/indexer/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions application/apps/indexer/session/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@ walkdir = "2.3"

[dev-dependencies]
lazy_static.workspace = true
tempfile.workspace = true
11 changes: 9 additions & 2 deletions application/apps/indexer/session/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,6 @@ pub async fn run(
}
Api::ShutdownWithError => {
debug!("shutdown state loop with error for testing");
state.session_file.cleanup()?;
return Err(NativeError {
severity: Severity::ERROR,
kind: NativeErrorKind::Io,
Expand All @@ -780,7 +779,15 @@ pub async fn run(
}
}
}
state.session_file.cleanup()?;
debug!("task is finished");
Ok(())
}

impl Drop for SessionState {
fn drop(&mut self) {
// Ensure session files are cleaned up by calling the cleanup function on drop.
if let Err(err) = self.session_file.cleanup() {
log::error!("Cleaning up session files failed. Error: {err:#?}");
}
}
}
43 changes: 40 additions & 3 deletions application/apps/indexer/session/src/state/session_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use tokio_util::sync::CancellationToken;
use uuid::Uuid;

pub const FLUSH_DATA_IN_MS: u128 = 500;
pub const SESSION_FILE_EXTENSION: &str = "session";

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct GrabbedElement {
Expand Down Expand Up @@ -93,7 +94,7 @@ impl SessionFile {
filename
} else {
let streams = paths::get_streams_dir()?;
let filename = streams.join(format!("{}.session", Uuid::new_v4()));
let filename = streams.join(format!("{}.{SESSION_FILE_EXTENSION}", Uuid::new_v4()));
debug!("Session file setup: {}", filename.to_string_lossy());
self.writer = Some(BufWriter::new(File::create(&filename).map_err(|e| {
NativeError {
Expand Down Expand Up @@ -301,19 +302,55 @@ impl SessionFile {
})
}

/// Cleans up the temporary generated files and for the session on its attachments if exist
/// for none-linked sessions.
pub fn cleanup(&mut self) -> Result<(), NativeError> {
if self.writer.is_none() {
// Session is linked. No temporary files has been generated.
return Ok(());
}

// Remove session main file.
let filename = self.filename()?;
debug!("cleaning up files: {:?}", filename);
if filename.exists() {
std::fs::remove_file(filename).map_err(|e| NativeError {
std::fs::remove_file(&filename).map_err(|e| NativeError {
severity: Severity::ERROR,
kind: NativeErrorKind::Io,
message: Some(e.to_string()),
message: Some(format!(
"Removing session main file fialed. Error: {e}. Path: {}",
filename.display()
)),
})?;
}

// Remove attachments directory if exists.
let attachments_dir = filename
.to_str()
.and_then(|file| file.strip_suffix(&format!(".{SESSION_FILE_EXTENSION}")))
.map(PathBuf::from)
.ok_or_else(|| NativeError {
severity: Severity::ERROR,
kind: NativeErrorKind::Io,
message: Some("Session file name isn't UTF-8 valid".into()),
})?;

if attachments_dir.exists() {
debug!(
"Cleaning up attachments direcotry: {}",
attachments_dir.display()
);

std::fs::remove_dir_all(&attachments_dir).map_err(|err| NativeError {
severity: Severity::ERROR,
kind: NativeErrorKind::Io,
message: Some(format!(
"Removing attachments directory failed. Error: {err}, Path: {}",
attachments_dir.display()
)),
})?
}

Ok(())
}
}
Expand Down
106 changes: 106 additions & 0 deletions application/apps/indexer/session/src/unbound/cleanup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
use std::{
fs, io,
path::Path,
time::{Duration, SystemTime},
};

use crate::{
events::{NativeError, NativeErrorKind},
paths::get_streams_dir,
progress::Severity,
};

/// Iterates through chipmunk temporary directory and remove the entries which is older
/// than two months.
pub fn cleanup_temp_dir() -> Result<(), NativeError> {
let tmp_dir = get_streams_dir()?;

const TWO_MONTHS_SECONDS: u64 = 60 * 60 * 24 * 60;
let modified_limit = SystemTime::now()
.checked_sub(Duration::from_secs(TWO_MONTHS_SECONDS))
.ok_or_else(|| NativeError {
severity: Severity::ERROR,
kind: NativeErrorKind::Io,
message: Some(String::from(
"Error while calculating modification time limit",
)),
})?;

cleanup_dir(&tmp_dir, modified_limit)?;

Ok(())
}

// Clean files and directory within the given path that have a modified time older than
// the given modified date limit
fn cleanup_dir(path: &Path, modified_date_limit: SystemTime) -> io::Result<()> {
if !path.exists() {
return Ok(());
}

fs::read_dir(path)?
.flat_map(Result::ok)
.filter(|p| {
p.metadata()
.is_ok_and(|meta| meta.modified().is_ok_and(|date| date < modified_date_limit))
})
.map(|entry| entry.path())
.try_for_each(|path| {
if path.is_dir() {
fs::remove_dir_all(path)
} else if path.is_file() {
fs::remove_file(path)
} else {
Ok(())
}
})
}

#[cfg(test)]
mod tests {
use std::{
fs::{self, File},
thread,
time::{Duration, SystemTime},
};

use super::cleanup_dir;

#[test]
fn test_cleanup_dir() {
// Create temporary directory with some entries
let tempdir = tempfile::tempdir().unwrap();
let temp_path = tempdir.path();

let dir = temp_path.join("dir");
fs::create_dir(&dir).unwrap();
let sub_file = dir.join("sub_file");
_ = File::create(&sub_file).unwrap();
let file = temp_path.join("file");
_ = File::create(&file).unwrap();

let entries = [dir, sub_file, file];

// Make sure there differences in modification time and current time.
thread::sleep(Duration::from_millis(50));

// Cleaning up with time stamp one day ago must not remove anything.
let past = SystemTime::now()
.checked_sub(Duration::from_secs(3600))
.unwrap();
cleanup_dir(&temp_path, past).unwrap();
for entry in &entries {
assert!(entry.exists());
}

// Cleaning up with now must remove all files and directories.
cleanup_dir(&temp_path, SystemTime::now()).unwrap();

// Temp directory itself shouldn't be removed.
assert!(temp_path.exists());

for entry in entries {
assert!(!entry.exists());
}
}
}
12 changes: 12 additions & 0 deletions application/apps/indexer/session/src/unbound/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod api;
mod cleanup;
pub mod commands;
mod signal;

Expand All @@ -10,6 +11,7 @@ use crate::{
signal::Signal,
},
};
use cleanup::cleanup_temp_dir;
use log::{debug, error, warn};
use std::collections::HashMap;
use tokio::{
Expand Down Expand Up @@ -117,6 +119,16 @@ impl UnboundSession {
finished.cancel();
debug!("Unbound session is down");
});

// Call cleanup here because this function should be called once when chipmunk starts.
// Run cleaning up on a separate thread to avoid latency in startup in case temporary
// directory has a lot of entries to cleanup.
tokio::task::spawn_blocking(|| {
if let Err(err) = cleanup_temp_dir() {
log::error!("Error while cleaning up temporary directory. Error: {err:?}");
}
});

Ok(())
}

Expand Down
Loading