Skip to content

Commit e05fa51

Browse files
committed
refactor(cache): use random suffix and cleanup on exit
- Replace deterministic hash with random suffix for cache directories - Add cleanup_cache_dir() to remove sot_path and job dirs on exit - Remove unused `source` field from RunnerActor - git_poll now receives sot_path directly instead of source URL
1 parent 6f919a3 commit e05fa51

File tree

5 files changed

+101
-55
lines changed

5 files changed

+101
-55
lines changed

CLAUDE.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,13 @@ jobs:
107107
108108
```
109109
~/.cache/rollcron/
110-
├── <repo>-<hash>/ # SoT: git repository
111-
└── <repo>-<hash>@<job-id>/ # Per-job snapshot (no .git)
110+
├── <repo>-<random>/ # SoT: git repository (random suffix per run)
111+
└── <repo>-<random>@<job-id>/ # Per-job snapshot (no .git)
112112
```
113113

114-
**Important**: Directory names use `job.id` (the YAML key), not `job.name`.
114+
**Important**:
115+
- Directory names use `job.id` (the YAML key), not `job.name`
116+
- Each run creates new directories with a random suffix (cleaned up on exit)
115117

116118
## Assumptions
117119

@@ -143,6 +145,11 @@ jobs:
143145
4. Apply task jitter (random delay 0 to jitter) before first execution
144146
5. On failure: apply exponential backoff + retry jitter before retry
145147

148+
### Shutdown (Ctrl+C)
149+
1. Wait for running jobs to complete (graceful stop)
150+
2. Stop all job actors
151+
3. Remove all cache directories (sot_path + job dirs)
152+
146153
## Logging
147154

148155
When `log_file` is set, command stdout/stderr is appended to the specified file. If not set, output is discarded.

src/actor/runner/git_poll.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use xtra::refcount::Weak;
1111

1212
const CONFIG_FILE: &str = "rollcron.yaml";
1313

14-
pub async fn run<A>(source: String, pull_interval: Duration, addr: Address<A, Weak>)
14+
pub async fn run<A>(sot_path: PathBuf, pull_interval: Duration, addr: Address<A, Weak>)
1515
where
1616
A: Handler<ConfigUpdate> + Handler<GetRunnerConfig, Return = RunnerConfig>,
1717
{
@@ -20,7 +20,7 @@ where
2020
loop {
2121
ticker.tick().await;
2222

23-
let (sot_path, update_info) = match git::ensure_repo(&source) {
23+
let update_info = match git::sync_repo(&sot_path) {
2424
Ok(r) => r,
2525
Err(e) => {
2626
error!(target: "rollcron::runner", error = %e, "Git sync failed");
@@ -38,7 +38,7 @@ where
3838
Ok((runner, jobs)) => {
3939
if let Err(e) = addr
4040
.send(ConfigUpdate {
41-
sot_path,
41+
sot_path: sot_path.clone(),
4242
runner,
4343
jobs,
4444
})

src/actor/runner/mod.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const CONFIG_FILE: &str = "rollcron.yaml";
1616

1717
/// Runner Actor - manages the lifecycle of all job actors
1818
pub struct RunnerActor {
19-
source: String,
2019
pull_interval: Duration,
2120
sot_path: PathBuf,
2221
runner_config: RunnerConfig,
@@ -28,13 +27,11 @@ pub struct RunnerActor {
2827

2928
impl RunnerActor {
3029
pub fn new(
31-
source: String,
3230
pull_interval: Duration,
3331
sot_path: PathBuf,
3432
runner_config: RunnerConfig,
3533
) -> Self {
3634
Self {
37-
source,
3835
pull_interval,
3936
sot_path,
4037
runner_config,
@@ -67,11 +64,11 @@ impl Actor for RunnerActor {
6764
self.self_addr = Some(addr.clone());
6865

6966
// Start git poll loop
70-
let source = self.source.clone();
67+
let sot_path = self.sot_path.clone();
7168
let pull_interval = self.pull_interval;
7269
let poll_addr = addr.clone();
7370
self.poll_handle = Some(tokio::spawn(async move {
74-
git_poll::run(source, pull_interval, poll_addr).await;
71+
git_poll::run(sot_path, pull_interval, poll_addr).await;
7572
}));
7673

7774
// Start supervisor loop
@@ -245,6 +242,17 @@ impl Handler<RespawnJob> for RunnerActor {
245242
}
246243
}
247244

245+
/// Get all job IDs for cleanup
246+
pub struct GetJobIds;
247+
248+
impl Handler<GetJobIds> for RunnerActor {
249+
type Return = Vec<String>;
250+
251+
async fn handle(&mut self, _msg: GetJobIds, _ctx: &mut Context<Self>) -> Self::Return {
252+
self.job_actors.keys().cloned().collect()
253+
}
254+
}
255+
248256
/// Graceful shutdown
249257
pub struct GracefulShutdown;
250258

src/git.rs

Lines changed: 64 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,29 @@ impl Drop for TempDirGuard<'_> {
2626
}
2727
}
2828

29-
/// Ensures repo is cloned/synced to cache. Returns (cache path, commit_range if updated).
30-
pub fn ensure_repo(source: &str) -> Result<(PathBuf, Option<String>)> {
31-
let cache_dir = get_cache_dir(source)?;
29+
/// Generates a cache directory path with random suffix.
30+
pub fn generate_cache_path(source: &str) -> PathBuf {
31+
let cache_base = dirs::cache_dir()
32+
.unwrap_or_else(|| PathBuf::from("/tmp"))
33+
.join("rollcron");
34+
35+
let repo_name = source
36+
.trim_end_matches('/')
37+
.trim_end_matches(".git")
38+
.rsplit('/')
39+
.next()
40+
.unwrap_or("repo");
41+
42+
let random_suffix = generate_random_suffix();
43+
cache_base.join(format!("{}-{}", repo_name, random_suffix))
44+
}
45+
46+
/// Clones repo to specified cache path.
47+
pub fn clone_to(source: &str, cache_dir: &Path) -> Result<()> {
3248
if let Some(parent) = cache_dir.parent() {
3349
std::fs::create_dir_all(parent)?;
3450
}
35-
36-
let update_info = if cache_dir.exists() {
37-
sync_repo(&cache_dir)?
38-
} else {
39-
clone_repo(source, &cache_dir)?;
40-
Some("initial".to_string())
41-
};
42-
43-
Ok((cache_dir, update_info))
51+
clone_repo(source, cache_dir)
4452
}
4553

4654
fn clone_repo(source: &str, dest: &Path) -> Result<()> {
@@ -59,8 +67,8 @@ fn clone_repo(source: &str, dest: &Path) -> Result<()> {
5967
Ok(())
6068
}
6169

62-
/// Returns commit range (e.g. "abc123..def456") if new commits were fetched
63-
fn sync_repo(dest: &Path) -> Result<Option<String>> {
70+
/// Syncs an existing repo. Returns commit range (e.g. "abc123..def456") if new commits were fetched.
71+
pub fn sync_repo(dest: &Path) -> Result<Option<String>> {
6472
// git clone sets up tracking branches for both local and remote repos
6573
let has_upstream = Command::new("git")
6674
.args(["rev-parse", "--abbrev-ref", "@{upstream}"])
@@ -128,29 +136,15 @@ fn sync_repo(dest: &Path) -> Result<Option<String>> {
128136
Ok(None)
129137
}
130138

131-
fn get_cache_dir(source: &str) -> Result<PathBuf> {
132-
let cache_base = dirs::cache_dir()
133-
.unwrap_or_else(|| PathBuf::from("/tmp"))
134-
.join("rollcron");
135-
136-
let repo_name = source
137-
.trim_end_matches('/')
138-
.trim_end_matches(".git")
139-
.rsplit('/')
140-
.next()
141-
.unwrap_or("repo");
142-
143-
let hash = &format!("{:x}", hash_str(source))[..8];
144-
145-
Ok(cache_base.join(format!("{}-{}", repo_name, hash)))
146-
}
147-
148-
fn hash_str(input: &str) -> u64 {
149-
use std::collections::hash_map::DefaultHasher;
150-
use std::hash::{Hash, Hasher};
151-
let mut hasher = DefaultHasher::new();
152-
input.hash(&mut hasher);
153-
hasher.finish()
139+
fn generate_random_suffix() -> String {
140+
use std::time::{SystemTime, UNIX_EPOCH};
141+
let nanos = SystemTime::now()
142+
.duration_since(UNIX_EPOCH)
143+
.map(|d| d.as_nanos())
144+
.unwrap_or(0);
145+
// Use lower 32 bits of nanoseconds XOR'd with process ID for uniqueness
146+
let unique = (nanos as u32) ^ std::process::id();
147+
format!("{:08x}", unique)
154148
}
155149

156150
pub fn get_job_dir(sot_path: &Path, job_id: &str) -> PathBuf {
@@ -252,13 +246,44 @@ pub fn sync_to_job_dir(sot_path: &Path, job_dir: &Path) -> Result<()> {
252246
Ok(())
253247
}
254248

249+
/// Removes the sot_path and all associated job directories.
250+
pub fn cleanup_cache_dir(sot_path: &Path, job_ids: &[String]) {
251+
use tracing::info;
252+
253+
// Remove job directories
254+
for job_id in job_ids {
255+
let job_dir = get_job_dir(sot_path, job_id);
256+
if job_dir.exists() {
257+
info!(path = %job_dir.display(), "Removing job directory");
258+
let _ = std::fs::remove_dir_all(&job_dir);
259+
}
260+
// Also remove temp/old variants
261+
let _ = std::fs::remove_dir_all(job_dir.with_extension("tmp"));
262+
let _ = std::fs::remove_dir_all(job_dir.with_extension("old"));
263+
}
264+
265+
// Remove sot_path
266+
if sot_path.exists() {
267+
info!(path = %sot_path.display(), "Removing cache directory");
268+
let _ = std::fs::remove_dir_all(sot_path);
269+
}
270+
}
271+
255272
#[cfg(test)]
256273
mod tests {
257274
use super::*;
258275

259276
#[test]
260-
fn cache_dir_from_url() {
261-
let dir = get_cache_dir("https://github.com/user/myrepo.git").unwrap();
277+
fn cache_path_from_url() {
278+
let dir = generate_cache_path("https://github.com/user/myrepo.git");
262279
assert!(dir.to_str().unwrap().contains("myrepo"));
263280
}
281+
282+
#[test]
283+
fn cache_path_is_random() {
284+
let dir1 = generate_cache_path("https://github.com/user/repo.git");
285+
std::thread::sleep(std::time::Duration::from_millis(1));
286+
let dir2 = generate_cache_path("https://github.com/user/repo.git");
287+
assert_ne!(dir1, dir2);
288+
}
264289
}

src/main.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ mod git;
55
mod logging;
66
mod webhook;
77

8-
use actor::runner::{GracefulShutdown, Initialize, RunnerActor};
8+
use actor::runner::{GetJobIds, GracefulShutdown, Initialize, RunnerActor};
99
use anyhow::{Context, Result};
1010
use clap::Parser;
1111
use std::path::PathBuf;
@@ -45,18 +45,18 @@ async fn main() -> Result<()> {
4545

4646
info!(source = %source, pull_interval = args.pull_interval, "Starting rollcron");
4747

48-
// Initial sync
49-
let (sot_path, _) = git::ensure_repo(&source)?;
48+
// Initial clone
49+
let sot_path = git::generate_cache_path(&source);
50+
git::clone_to(&source, &sot_path)?;
5051
info!(cache = %sot_path.display(), "Repository ready");
5152

5253
let (initial_runner, initial_jobs) = load_config(&sot_path)?;
5354

5455
// Spawn Runner actor
5556
let runner = xtra::spawn_tokio(
5657
RunnerActor::new(
57-
source,
5858
Duration::from_secs(args.pull_interval),
59-
sot_path,
59+
sot_path.clone(),
6060
initial_runner,
6161
),
6262
Mailbox::unbounded(),
@@ -72,9 +72,15 @@ async fn main() -> Result<()> {
7272
tokio::signal::ctrl_c().await?;
7373
info!("Shutting down...");
7474

75+
// Get job IDs for cleanup
76+
let job_ids = runner.send(GetJobIds).await.unwrap_or_default();
77+
7578
// Graceful shutdown
7679
let _ = runner.send(GracefulShutdown).await;
7780

81+
// Cleanup cache directories
82+
git::cleanup_cache_dir(&sot_path, &job_ids);
83+
7884
Ok(())
7985
}
8086

0 commit comments

Comments
 (0)