fix(session): fsync sessions on graceful shutdown to prevent data loss
On filesystems with write-back caching (rclone VFS, NFS, FUSE mounts) the OS page cache may buffer recent session writes. If the process is killed before the cache flushes, the most recent conversation turns are silently lost — causing the agent to "forget" recent context and respond to stale history on the next startup. Changes: - session/manager.py: add fsync=True option to save() that flushes the file and its parent directory to durable storage. Add flush_all() that re-saves every cached session with fsync. Default save() behavior is unchanged (no fsync) to avoid performance regression in normal operation. - cli/commands.py: call agent.sessions.flush_all() in the gateway shutdown finally block, after stopping heartbeat/cron/channels. - tests/session/test_session_fsync.py: 8 tests covering fsync flag behavior, flush_all with empty/multiple/errored sessions, and data survival across simulated process restart. - tests/cli/test_commands.py: add sessions attribute to _FakeAgentLoop so the gateway health endpoint test passes with the new shutdown flush.
This commit is contained in:
@@ -946,6 +946,12 @@ def _run_gateway(
|
||||
cron.stop()
|
||||
agent.stop()
|
||||
await channels.stop_all()
|
||||
# Flush all cached sessions to durable storage before exit.
|
||||
# This prevents data loss on filesystems with write-back
|
||||
# caching (rclone VFS, NFS, FUSE mounts, etc.).
|
||||
flushed = agent.sessions.flush_all()
|
||||
if flushed:
|
||||
logger.info("Shutdown: flushed {} session(s) to disk", flushed)
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
|
||||
@@ -262,8 +262,16 @@ class SessionManager:
|
||||
"messages": session.messages,
|
||||
}
|
||||
|
||||
def save(self, session: Session) -> None:
|
||||
"""Save a session to disk atomically."""
|
||||
def save(self, session: Session, *, fsync: bool = False) -> None:
|
||||
"""Save a session to disk atomically.
|
||||
|
||||
When *fsync* is ``True`` the final file and its parent directory are
|
||||
explicitly flushed to durable storage. This is intentionally off by
|
||||
default (the OS page-cache is sufficient for normal operation) but
|
||||
should be enabled during graceful shutdown so that filesystems with
|
||||
write-back caching (e.g. rclone VFS, NFS, FUSE mounts) do not lose
|
||||
the most recent writes.
|
||||
"""
|
||||
path = self._get_session_path(session.key)
|
||||
tmp_path = path.with_suffix(".jsonl.tmp")
|
||||
|
||||
@@ -280,14 +288,41 @@ class SessionManager:
|
||||
f.write(json.dumps(metadata_line, ensure_ascii=False) + "\n")
|
||||
for msg in session.messages:
|
||||
f.write(json.dumps(msg, ensure_ascii=False) + "\n")
|
||||
if fsync:
|
||||
f.flush()
|
||||
os.fsync(f.fileno())
|
||||
|
||||
os.replace(tmp_path, path)
|
||||
|
||||
if fsync:
|
||||
# fsync the directory so the rename is durable.
|
||||
fd = os.open(str(path.parent), os.O_RDONLY)
|
||||
try:
|
||||
os.fsync(fd)
|
||||
finally:
|
||||
os.close(fd)
|
||||
except BaseException:
|
||||
tmp_path.unlink(missing_ok=True)
|
||||
raise
|
||||
|
||||
self._cache[session.key] = session
|
||||
|
||||
def flush_all(self) -> int:
|
||||
"""Re-save every cached session with fsync for durable shutdown.
|
||||
|
||||
Returns the number of sessions flushed. Errors on individual
|
||||
sessions are logged but do not prevent other sessions from being
|
||||
flushed.
|
||||
"""
|
||||
flushed = 0
|
||||
for key, session in list(self._cache.items()):
|
||||
try:
|
||||
self.save(session, fsync=True)
|
||||
flushed += 1
|
||||
except Exception:
|
||||
logger.warning("Failed to flush session {}", key, exc_info=True)
|
||||
return flushed
|
||||
|
||||
def invalidate(self, key: str) -> None:
|
||||
"""Remove a session from the in-memory cache."""
|
||||
self._cache.pop(key, None)
|
||||
|
||||
Reference in New Issue
Block a user