Is there some way to retain more than one bag per fault_code?

[jardeldyonisio]

Summary

I'm trying to configure the fault manager to keep more than one rosbag capture for repeated
occurrences of the same fault. Is there a way to define a max number of bags per fault_code
(e.g. keep the last N) before older ones for that code are deleted? Right now it seems only
one bag is ever kept per fault_code.

Environment

• ROS 2 distro: Humble (Ubuntu 22.04.5 LTS)
• ros-humble-ros2-medkit-fault-manager: 0.6.0-1jammy.20260726.134439
• Storage backend: sqlite

Current behavior

Every time a fault re-confirms (e.g. PREFAILED → CONFIRMED again after healing, or
CONFIRMED while occurrence data updates), RosbagCapture writes a new bag and calls
FaultStorage::store_rosbag_file(). That function deletes the previously stored bag for the
same fault_code before recording the new one — so only the most recent bag per fault
code is ever retained, regardless of max_bag_size_mb / max_total_storage_mb:

// fault_storage.cpp — InMemoryFaultStorage::store_rosbag_file()
void InMemoryFaultStorage::store_rosbag_file(const RosbagFileInfo & info) {
  // Delete existing bag file if present (prevent orphaned files on re-confirm)
  auto it = rosbag_files_.find(info.fault_code);
  if (it != rosbag_files_.end()) {
    if (it->second.file_path != info.file_path) {
      std::filesystem::remove_all(it->second.file_path, ec);   // <-- previous bag for this code is deleted here
    }
  }
  rosbag_files_[info.fault_code] = info;
}

The SQLite backend does the same via DELETE FROM rosbag_files WHERE fault_code = ? in
SqliteFaultStorage::delete_rosbag_file(), invoked the same way on each new capture for that
code.

Importantly, this is separate from auto_cleanup: auto_cleanup only deletes a bag when
the fault is cleared (RosbagCapture::on_fault_cleared). The one-bag-per-fault_code deletion
above happens unconditionally on every re-confirmation, so setting auto_cleanup: false does
not help retain history.

Steps to reproduce

1. Enable rosbag capture with the config below (attached in full).
2. Trigger a fault that confirms, heals, and re-confirms multiple times (e.g. a flapping
   SYSTEM_CPU/sensor fault with healing_enabled: true).
3. Observe the log: each re-confirmation logs Bag file completed: <path> at a new
   fault_{CODE}_{timestamp} path, and the previous bag directory for that same fault code is
   gone from disk.

Expected / desired behavior

A way to retain a bounded history of bags per fault_code instead of only the latest — e.g.
a snapshots.rosbag.max_bags_per_fault (or similar) parameter, analogous to how
max_per_fault already works for JSON snapshots. Something like:

rosbag:
  enabled: true
  max_bags_per_fault: 5   # keep the last 5 bags for a given fault_code; delete oldest beyond that

so a fault that recurs (heals and re-confirms) leaves a trail of captures instead of only ever
showing the most recent occurrence.

Config file used

# Snapshot Capture Configuration
#
# This file configures which topics to capture when specific faults are confirmed.
# The captured data provides debugging context at the moment of fault occurrence.
#
# Topic resolution priority:
#   1. fault_specific - exact fault code match (highest priority)
#   2. patterns - regex pattern match on fault code
#   3. default_topics - fallback for all other faults (lowest priority)
#
# Usage:
#   ros2 run ros2_medkit_fault_manager fault_manager_node \
#     --ros-args -p snapshots.config_file:=/path/to/snapshots.yaml

fault_specific:
  MOTOR_OVERHEAT:
    - /joint_states
    - /motor/temperature
    - /motor/current
  BATTERY_LOW:
    - /battery_state
    - /power/consumption
  LIDAR_COMM_FAILURE:
    - /perception/lidar/scan
    - /perception/lidar/diagnostics

patterns:
  "MOTOR_.*":
    - /joint_states
    - /cmd_vel
  "SENSOR_.*":
    - /diagnostics
    - /sensor_data
  ".*_COMM_FAILURE":
    - /diagnostics

default_topics:
  - /diagnostics
  - /rosout

rosbag:
        enabled: true                  # Enable rosbag recording
        duration_sec: 5.0              # Pre-fault buffer duration
        duration_after_sec: 1.0        # Post-fault recording duration
        topics: "config"               # Topic selection: "entity" (default), "config", "all", "explicit"
        # include_topics: []             # Additional topics to include
        # exclude_topics: []             # Topics to exclude
        exclude_sensor_topics: false   # Auto-exclude image/points/depth/compressed in broad modes
        lazy_start: false              # Start recording on first fault
        format: "sqlite3"              # Storage format
        qos_match: true                # Match each topic's publisher QoS

        # Where the flushed bags are written. Empty ("") falls back to the system temp
        # dir (/tmp); set an explicit path to keep them. Bags are named
        # fault_{FAULT_CODE}_{timestamp}/. The node creates the directory if needed.
        storage_path: "/mnt/persistent/instor/bags/fault_bags"

        max_buffer_mb: 256             # Ring-buffer RAM cap
        max_bag_size_mb: 100           # Max total storage
        max_total_storage_mb: 1000

        # Keep bags after their fault is cleared (default true deletes them on ClearFault).
        auto_cleanup: false            # Auto-delete old bags

(applied via --ros-args -p snapshots.rosbag.* per the file's own note, with enabled: true,
storage_path, and auto_cleanup: false set as shown above.)

[mfaferek93]

@jardeldyonisio your diagnosis is correct and every line you pointed at says what you say it says. Many thanks for that!

I traced what it would take to fix properly, and it is bigger than a retention parameter. Three things make it structural rather than incremental:

• The in-memory backend keeps rosbags in a std::map keyed by fault_code, so dropping the SQL UNIQUE fixes only one of two shipped backends.
• GetRosbag.srv and Snapshot.msg both address a bag by fault code, and bulk_data_id is a scalar. So a fault can structurally reference exactly one recording, and the download has no way to ask for a specific one. That is a change in ros2_medkit_msgs, not just in the gateway.
• Every delete primitive is DELETE ... WHERE fault_code = ? and collects a single path, so under more than one row per fault the quota sweep would orphan directories it can no longer see.

None of that is a reason not to do it. The evidence pipeline already retains the freeze frame across clear_fault while overwriting the recording on every re-confirm, which is incoherent, and your flapping-fault case is exactly where it hurts.

I can pick this up rather than handing you a redesign as a first contribution :D (I can also support you if you want to implement the chenge)
Two things that would genuinely help if you are interested: the retention policy question (keeping the last N loses the first occurrence, which is usually the one with the root cause in it, so we are leaning toward first plus N-1 most recent), and a second pair of eyes on the migration once there is a branch.

Separately, your config avoided something the default does not: snapshots.rosbag.storage_path defaults to the system temp dir while the database is persistent, so after a reboot the quota counts bags that no longer exist. Filing that on its own.

[jardeldyonisio]

@mfaferek93 thank you for the very quick response and for breaking down the structural changes needed! I saw that the PR is already open, I'll be following it and I'll gladly be a second pair of eyes to help review/test the migration.