electric-sql / electric-sql/electric
DiskUsage: handle missing directory, make scan period configurable
- Dominant language
- TypeScript
- Stars
- 10.4k
- Forks
- 375
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 18
Description
## Context
`ElectricTelemetry.DiskUsage` is a GenServer that periodically scans a storage directory to calculate disk usage. It was recently integrated into the cloud-sync-service's per-stack telemetry. During integration testing, several issues surfaced.
## Issues
### 1. DiskUsage crashes when the storage directory doesn't exist
`DiskUsage` starts eagerly via `handle_continue(:calculate_usage)` and calls `save_usage!/1` which does `File.write!` to `/.disk-usage`. If the storage directory hasn't been created yet (e.g. by a lazily-initializing storage module like `PureFileStorage`), this crashes the process:
```
** (File.Error) could not write to file ".../shapes/.disk-usage": no such file or directory
```
After 3 rapid restarts, the `StackTelemetry` supervisor shuts down entirely, taking all telemetry exporters with it.
**Proposed fix:** `DiskUsage` should ensure the directory exists (via `File.mkdir_p/1` or similar) before attempting to write, rather than relying on callers to pre-create it.
### 2. Scan period should be configurable (and disableable)
The current `update_period` defaults to 60 seconds and is only settable via the GenServer's init args. There's no way to configure it through application config or environment variables.
For integration testing, 60s is far too long — tests need to verify non-zero disk usage metrics but can't wait a full minute for a rescan after data is written. Conversely, in some deployments scanning may not be needed at all.
**Proposed behavior:**
- Add an `ELECTRIC_DISK_USAGE_SCAN_PERIOD` environment variable (or similar), parsed as a human-readable duration (e.g. `5s`, `60s`).
- If set to `0` or `disabled`, `DiskUsage` should not start at all. This is consistent with how the process already works — `StackTelemetry.disk_usage_child_specs/1` skips starting it when `storage_dir` is absent. The same conditional could check the configured scan period.
- The scan period should remain decoupled from the telemetry reporting interval (`export_period`), since directory scanning can be slow on large storage volumes and shouldn't block metric export.
### 3. Eager scanning is wasteful when reporting is not configured
`DiskUsage` currently starts and scans unconditionally as long as `storage_dir` is present in the telemetry opts. However, if no reporter (OTEL, StatsD, Prometheus, etc.) is configured to consume the `electric.storage.used` metric, the scanning work is wasted.
This is less of a bug and more of an optimization opportunity — the process could be gated on whether any metric export is actually enabled, or deferred until the first report is requested.
## Current workarounds (in cloud-sync-service)
1. `TenantManager` calls `File.mkdir_p!` on the shapes directory before starting the stack (to prevent crash).
2. Integration tests pre-create a seed file in the shapes directory to ensure non-zero initial measurements (to avoid waiting 60s for a rescan).
Both would become unnecessary with the fixes above.
Contributor guide
Assessment
This issue has not been assessed yet.