Skip to content

Commit 98006e0

Browse files
committed
fix(storage): add fsync after file writes to prevent data loss
Fixes #5299 and #5204. Files written without fsync can be lost if system crashes before OS flushes buffers to disk. Added sync_all() calls after file writes and directory sync for crash safety. Changes: - cortex-storage: Add fsync to save_session and save_session_sync - cortex-tui: Add fsync to save_meta and rewrite_history - Add directory fsync on Unix for proper crash recovery
1 parent c398212 commit 98006e0

File tree

2 files changed

+89
-5
lines changed

2 files changed

+89
-5
lines changed

src/cortex-storage/src/sessions/storage.rs

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,20 +124,67 @@ impl SessionStorage {
124124
}
125125

126126
/// Save a session to disk.
127+
///
128+
/// This function ensures data durability by calling sync_all() (fsync)
129+
/// after writing to prevent data loss on crash or forceful termination.
127130
pub async fn save_session(&self, session: &StoredSession) -> Result<()> {
128131
let path = self.paths.session_path(&session.id);
129132
let content = serde_json::to_string_pretty(session)?;
130-
fs::write(&path, content).await?;
133+
134+
// Write content to file
135+
let file = fs::OpenOptions::new()
136+
.create(true)
137+
.write(true)
138+
.truncate(true)
139+
.open(&path)
140+
.await?;
141+
142+
use tokio::io::AsyncWriteExt;
143+
let mut file = file;
144+
file.write_all(content.as_bytes()).await?;
145+
file.flush().await?;
146+
147+
// Ensure data is durably written to disk (fsync) to prevent data loss on crash
148+
file.sync_all().await?;
149+
150+
// Sync parent directory on Unix for crash safety (ensures directory entry is persisted)
151+
#[cfg(unix)]
152+
{
153+
if let Some(parent) = path.parent() {
154+
if let Ok(dir) = fs::File::open(parent).await {
155+
let _ = dir.sync_all().await;
156+
}
157+
}
158+
}
159+
131160
debug!(session_id = %session.id, "Session saved");
132161
Ok(())
133162
}
134163

135164
/// Save a session synchronously.
165+
///
166+
/// This function ensures data durability by calling sync_all() (fsync)
167+
/// after writing to prevent data loss on crash or forceful termination.
136168
pub fn save_session_sync(&self, session: &StoredSession) -> Result<()> {
137169
let path = self.paths.session_path(&session.id);
138170
let file = std::fs::File::create(&path)?;
139-
let writer = BufWriter::new(file);
140-
serde_json::to_writer_pretty(writer, session)?;
171+
let mut writer = BufWriter::new(file);
172+
serde_json::to_writer_pretty(&mut writer, session)?;
173+
writer.flush()?;
174+
175+
// Ensure data is durably written to disk (fsync) to prevent data loss on crash
176+
writer.get_ref().sync_all()?;
177+
178+
// Sync parent directory on Unix for crash safety (ensures directory entry is persisted)
179+
#[cfg(unix)]
180+
{
181+
if let Some(parent) = path.parent() {
182+
if let Ok(dir) = std::fs::File::open(parent) {
183+
let _ = dir.sync_all();
184+
}
185+
}
186+
}
187+
141188
debug!(session_id = %session.id, "Session saved");
142189
Ok(())
143190
}

src/cortex-tui/src/session/storage.rs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,47 @@ impl SessionStorage {
8787
// ========================================================================
8888

8989
/// Saves session metadata.
90+
///
91+
/// Uses atomic write (temp file + rename) with fsync for durability.
92+
/// This prevents data loss on crash or forceful termination.
9093
pub fn save_meta(&self, meta: &SessionMeta) -> Result<()> {
9194
self.ensure_session_dir(&meta.id)?;
9295

9396
let path = self.meta_path(&meta.id);
9497
let content =
9598
serde_json::to_string_pretty(meta).context("Failed to serialize session metadata")?;
9699

97-
// Atomic write: write to temp file then rename
100+
// Atomic write: write to temp file, fsync, then rename
98101
let temp_path = path.with_extension("json.tmp");
99-
fs::write(&temp_path, &content)
102+
103+
// Write and sync temp file
104+
let file = File::create(&temp_path)
105+
.with_context(|| format!("Failed to create temp metadata file: {:?}", temp_path))?;
106+
let mut writer = BufWriter::new(file);
107+
writer
108+
.write_all(content.as_bytes())
100109
.with_context(|| format!("Failed to write temp metadata file: {:?}", temp_path))?;
110+
writer.flush()?;
111+
112+
// Ensure data is durably written to disk (fsync) before rename
113+
writer.get_ref().sync_all().with_context(|| {
114+
format!("Failed to sync temp metadata file to disk: {:?}", temp_path)
115+
})?;
116+
117+
// Rename temp file to final path
101118
fs::rename(&temp_path, &path)
102119
.with_context(|| format!("Failed to rename metadata file: {:?}", path))?;
103120

121+
// Sync parent directory on Unix for crash safety (ensures directory entry is persisted)
122+
#[cfg(unix)]
123+
{
124+
if let Some(parent) = path.parent() {
125+
if let Ok(dir) = File::open(parent) {
126+
let _ = dir.sync_all();
127+
}
128+
}
129+
}
130+
104131
Ok(())
105132
}
106133

@@ -212,6 +239,16 @@ impl SessionStorage {
212239
fs::rename(&temp_path, &path)
213240
.with_context(|| format!("Failed to rename history file: {:?}", path))?;
214241

242+
// Sync parent directory on Unix for crash safety (ensures directory entry is persisted)
243+
#[cfg(unix)]
244+
{
245+
if let Some(parent) = path.parent() {
246+
if let Ok(dir) = File::open(parent) {
247+
let _ = dir.sync_all();
248+
}
249+
}
250+
}
251+
215252
Ok(())
216253
}
217254

0 commit comments

Comments
 (0)