Dedupe daemon#40
Conversation
- Introduced a new `Daemon` command to periodically run deduplication on a specified path. - Refactored `Dedupe` command to use a dedicated `DedupeOptions` struct for better organization. - Added `ctrlc` support for graceful shutdown of the daemon. - Updated `Cargo.toml` to include the `ctrlc` dependency.
There was a problem hiding this comment.
Pull request overview
Adds a periodic “dedupe daemon” mode to the bdstorage CLI and introduces example systemd units/docs (both WSL user units and bare-metal system units) to run scheduled or continuously-looping dedupe runs.
Changes:
- Refactors
dedupeCLI args into a reusableDedupeOptionsand adds a newdaemonsubcommand that repeatedly spawnsbdstorage dedupeuntil interrupted. - Adds
ctrlcdependency to support clean interruption of the daemon loop. - Adds
contrib/wsl/*andcontrib/systemd/*docs, unit files, and an install helper script.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
src/main.rs |
Adds DedupeOptions, daemon subcommand, and run_daemon loop with ctrl-c handling. |
Cargo.toml |
Adds ctrlc dependency needed by the daemon command. |
contrib/wsl/README.md |
Documents WSL2 + systemd setup and how to run user units/timers. |
contrib/wsl/wsl.conf.example |
Example snippet to enable systemd in WSL. |
contrib/wsl/install-wsl-user-units.sh |
Installs user unit files and creates an env file for WSL usage. |
contrib/wsl/env.example |
Example env file for user units. |
contrib/wsl/bdstorage-dedupe.service |
User service to run bdstorage daemon … under systemctl --user. |
contrib/wsl/bdstorage-dedupe-once.service |
User oneshot unit to run one bdstorage dedupe …. |
contrib/wsl/bdstorage-dedupe.timer |
User timer to trigger the oneshot daily. |
contrib/systemd/README.md |
Bare-metal systemd deployment instructions. |
contrib/systemd/bdstorage.env.example |
Example /etc/bdstorage.env for the system units. |
contrib/systemd/bdstorage-dedupe.service |
System service to run bdstorage daemon … as user bdstorage. |
contrib/systemd/bdstorage-dedupe-once.service |
System oneshot unit to run one bdstorage dedupe …. |
contrib/systemd/bdstorage-dedupe.timer |
System timer to trigger the oneshot daily. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _ in 0..interval_secs { | ||
| if !running.load(Ordering::SeqCst) { | ||
| break; | ||
| } | ||
| thread::sleep(Duration::from_secs(1)); | ||
| } |
There was a problem hiding this comment.
interval_secs can be set to 0, which makes the daemon immediately start the next dedupe run with no delay (tight loop after the child exits). Consider validating interval_secs >= 1 (or enforcing a minimum via clap) to avoid accidental high-CPU / high-IO behavior.
|
|
||
| [Unit] | ||
| Description=bdstorage deduplication daemon (WSL user) | ||
| After=default.target |
There was a problem hiding this comment.
After=default.target combined with WantedBy=default.target creates an ordering cycle (the target pulls the service in, but the service is ordered after the target). Dropping After=default.target (or ordering after a more specific target) avoids systemd cycle warnings and ensures reliable startup ordering.
| After=default.target |
|
|
||
| [Service] | ||
| Type=simple | ||
| EnvironmentFile=-%h/.config/bdstorage/env |
There was a problem hiding this comment.
EnvironmentFile is marked optional (-...), but ExecStart depends on variables from that file. If the env file is missing, the unit will fail in a more confusing way (empty command/path/args). Consider removing the leading - so systemd reports a missing environment file explicitly, or provide safe defaults in the unit.
| EnvironmentFile=-%h/.config/bdstorage/env | |
| EnvironmentFile=%h/.config/bdstorage/env |
| Type=simple | ||
| User=bdstorage | ||
| Group=bdstorage | ||
| EnvironmentFile=-/etc/bdstorage.env |
There was a problem hiding this comment.
EnvironmentFile is marked optional (-...), but ExecStart depends on variables from that file. If the env file is missing, the unit will fail in a more confusing way (missing target/interval). Consider removing the leading - so systemd reports the missing env file explicitly, or provide defaults.
| EnvironmentFile=-/etc/bdstorage.env | |
| EnvironmentFile=/etc/bdstorage.env |
| Type=oneshot | ||
| User=bdstorage | ||
| Group=bdstorage | ||
| EnvironmentFile=-/etc/bdstorage.env |
There was a problem hiding this comment.
EnvironmentFile is marked optional (-...), but ExecStart relies on $BDSTORAGE_TARGET. If the env file is missing (or the var is unset), the oneshot will fail with a clap usage error rather than a clear systemd env-file error. Consider making the env file required or hardcoding the target in the unit.
| EnvironmentFile=-/etc/bdstorage.env | |
| EnvironmentFile=/etc/bdstorage.env |
| User=bdstorage | ||
| Group=bdstorage | ||
| EnvironmentFile=-/etc/bdstorage.env | ||
| ExecStart=/usr/local/bin/bdstorage daemon $BDSTORAGE_TARGET --interval-secs $BDSTORAGE_INTERVAL_SECS |
There was a problem hiding this comment.
ExecStart uses $BDSTORAGE_TARGET unquoted; if the target path contains spaces, systemd will split it into multiple arguments and the dedupe invocation will fail. Consider quoting $BDSTORAGE_TARGET in ExecStart= (systemd supports quoting) to keep it a single argument.
| ExecStart=/usr/local/bin/bdstorage daemon $BDSTORAGE_TARGET --interval-secs $BDSTORAGE_INTERVAL_SECS | |
| ExecStart=/usr/local/bin/bdstorage daemon "$BDSTORAGE_TARGET" --interval-secs $BDSTORAGE_INTERVAL_SECS |
| User=bdstorage | ||
| Group=bdstorage | ||
| EnvironmentFile=-/etc/bdstorage.env | ||
| ExecStart=/usr/local/bin/bdstorage dedupe $BDSTORAGE_TARGET |
There was a problem hiding this comment.
ExecStart uses $BDSTORAGE_TARGET unquoted; if the target path contains spaces, systemd will split it into multiple arguments and the oneshot will fail. Consider quoting $BDSTORAGE_TARGET in ExecStart= to keep it a single argument.
| ExecStart=/usr/local/bin/bdstorage dedupe $BDSTORAGE_TARGET | |
| ExecStart=/usr/local/bin/bdstorage dedupe "$BDSTORAGE_TARGET" |
| [Service] | ||
| Type=oneshot | ||
| EnvironmentFile=-%h/.config/bdstorage/env | ||
| ExecStart=$BDSTORAGE_BIN dedupe $BDSTORAGE_TARGET |
There was a problem hiding this comment.
ExecStart uses $BDSTORAGE_TARGET unquoted; if the target path contains spaces, systemd will split it into multiple arguments and the oneshot will fail. Consider quoting $BDSTORAGE_TARGET in ExecStart=.
| ExecStart=$BDSTORAGE_BIN dedupe $BDSTORAGE_TARGET | |
| ExecStart=$BDSTORAGE_BIN dedupe "$BDSTORAGE_TARGET" |
| umask 077 | ||
| cat >"$ENV_FILE" <<EOF | ||
| HOME=${HOME} | ||
| BDSTORAGE_BIN=${BIN} | ||
| BDSTORAGE_TARGET=${TARGET} | ||
| BDSTORAGE_INTERVAL_SECS=${INTERVAL} | ||
| EOF |
There was a problem hiding this comment.
The script sets umask 077 when creating the env file but does not restore the previous umask afterward. This can unintentionally affect permissions of files created later in the script (including installed unit files). Consider saving/restoring the prior umask (or doing env-file creation in a subshell).
| umask 077 | |
| cat >"$ENV_FILE" <<EOF | |
| HOME=${HOME} | |
| BDSTORAGE_BIN=${BIN} | |
| BDSTORAGE_TARGET=${TARGET} | |
| BDSTORAGE_INTERVAL_SECS=${INTERVAL} | |
| EOF | |
| ( | |
| umask 077 | |
| cat >"$ENV_FILE" <<EOF | |
| HOME=${HOME} | |
| BDSTORAGE_BIN=${BIN} | |
| BDSTORAGE_TARGET=${TARGET} | |
| BDSTORAGE_INTERVAL_SECS=${INTERVAL} | |
| EOF | |
| ) |
| Commands::Daemon { | ||
| dedupe, | ||
| interval_secs, | ||
| } => { | ||
| run_daemon(dedupe, interval_secs)?; | ||
| } |
There was a problem hiding this comment.
New daemon subcommand / run_daemon behavior isn’t covered by the existing integration tests (which already exercise dedupe/restore). Adding an integration test that starts bdstorage daemon <tmpdir> --interval-secs 1 and then terminates it (after verifying at least one dedupe run occurred) would help prevent regressions in CLI parsing and daemon loop behavior.
|
background daemon for native systemd execution (Closes #38 )
|
feat: add daemon mode for deduplication with configurable interval
Daemoncommand to periodically run deduplication on a specified path.Dedupecommand to use a dedicatedDedupeOptionsstruct for better organization.ctrlcsupport for graceful shutdown of the daemon.Cargo.tomlto include thectrlcdependency.