diff --git a/chain/src/error.rs b/chain/src/error.rs index 3ed9b3c88e..c739c9afc2 100644 --- a/chain/src/error.rs +++ b/chain/src/error.rs @@ -118,6 +118,11 @@ pub enum Error { /// Internal issue when trying to save or load data from append only files #[error("File Read Error: {0}")] FileReadErr(String), + /// Filesystem is out of space while writing chain data + #[error( + "Out of disk space: {0}. Free disk space and restart the node; avoid `grin --clean` unless recovery fails." + )] + DiskFull(String), /// Error serializing or deserializing a type #[error("Serialization Error")] SerErr { @@ -202,6 +207,7 @@ impl Error { | Error::StoreErr(_, _) | Error::SerErr { .. } | Error::TxHashSetErr(_) + | Error::DiskFull(_) | Error::GenesisBlockRequired | Error::Other(_) => false, _ => true, @@ -211,12 +217,19 @@ impl Error { impl From for Error { fn from(error: store::Error) -> Error { - Error::StoreErr(error.clone(), format!("{:?}", error)) + match &error { + store::Error::DiskFull(msg) => Error::DiskFull(msg.clone()), + _ => Error::StoreErr(error.clone(), format!("{:?}", error)), + } } } impl From for Error { fn from(e: io::Error) -> Error { - Error::TxHashSetErr(e.to_string()) + if store::is_out_of_disk_space(&e) { + Error::DiskFull(e.to_string()) + } else { + Error::TxHashSetErr(e.to_string()) + } } } diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index 4f8446b2ac..f9f067e365 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -901,10 +901,25 @@ where trees.kernel_pmmr_h.backend.discard(); } else { trace!("Committing txhashset extension. sizes {:?}", sizes); + // Sync MMR backends *before* committing the DB batch (#3425). + // If disk is full, the batch stays uncommitted so chain head + // cannot point at incomplete on-disk MMR state. + if let Err(e) = (|| -> Result<(), std::io::Error> { + trees.output_pmmr_h.backend.sync()?; + trees.rproof_pmmr_h.backend.sync()?; + trees.kernel_pmmr_h.backend.sync()?; + Ok(()) + })() { + error!( + "Failed to sync txhashset to disk (discarding extension): {}", + e + ); + trees.output_pmmr_h.backend.discard(); + trees.rproof_pmmr_h.backend.discard(); + trees.kernel_pmmr_h.backend.discard(); + return Err(e.into()); + } child_batch.commit()?; - trees.output_pmmr_h.backend.sync()?; - trees.rproof_pmmr_h.backend.sync()?; - trees.kernel_pmmr_h.backend.sync()?; trees.output_pmmr_h.size = sizes.0; trees.rproof_pmmr_h.size = sizes.1; trees.kernel_pmmr_h.size = sizes.2; @@ -994,8 +1009,16 @@ where if rollback { handle.backend.discard(); } else { + // Sync backend before committing DB batch (#3425). + if let Err(e) = handle.backend.sync() { + error!( + "Failed to sync header MMR to disk (discarding extension): {}", + e + ); + handle.backend.discard(); + return Err(e.into()); + } child_batch.commit()?; - handle.backend.sync()?; handle.size = size; } Ok(r) diff --git a/store/src/lib.rs b/store/src/lib.rs index b2408ca55e..24da40946d 100644 --- a/store/src/lib.rs +++ b/store/src/lib.rs @@ -68,11 +68,46 @@ pub fn u64_to_key(prefix: u8, val: u64) -> Vec { pub use crate::lmdb::*; use std::ffi::OsStr; -use std::fs::{remove_file, rename, File}; +use std::fs::{remove_file, rename, File, OpenOptions}; use std::path::Path; +/// Returns true if the I/O error indicates the filesystem is out of space. +/// +/// Covers `ErrorKind::StorageFull` and platform-specific `ENOSPC` (28 on most Unix). +pub fn is_out_of_disk_space(err: &io::Error) -> bool { + if err.kind() == io::ErrorKind::StorageFull { + return true; + } + // ENOSPC on Unix; also match common string forms from lower layers. + if err.raw_os_error() == Some(28) { + return true; + } + let msg = err.to_string().to_lowercase(); + msg.contains("no space left") || msg.contains("disk full") || msg.contains("not enough space") +} + +/// Map an I/O error into a clear StorageFull error when out of disk space. +pub fn map_io_err(err: io::Error) -> io::Error { + if is_out_of_disk_space(&err) { + io::Error::new( + io::ErrorKind::StorageFull, + format!( + "out of disk space: {}. Free disk space and restart; avoid wiping chain data unless recovery fails.", + err + ), + ) + } else { + err + } +} + /// Creates temporary file with name created by adding `temp_suffix` to `path`. /// Applies writer function to it and renames temporary file into original specified by `path`. +/// +/// Durability notes (see also #3352 / #3425): +/// - Writer errors leave the original file untouched (temp is removed). +/// - Temp file is fsync'd before rename. +/// - Parent directory is fsync'd after rename so the rename itself is durable. pub fn save_via_temp_file(path: P, temp_suffix: E, mut writer: F) -> Result<(), io::Error> where F: FnMut(&mut File) -> Result<(), io::Error>, @@ -83,23 +118,40 @@ where assert!(!temp_suffix.is_empty()); let original = path.as_ref(); - let mut _original = original.as_os_str().to_os_string(); - _original.push(temp_suffix); - // Write temporary file - let temp_path = Path::new(&_original); + let mut temp_os = original.as_os_str().to_os_string(); + temp_os.push(temp_suffix); + let temp_path = Path::new(&temp_os); + if temp_path.exists() { remove_file(&temp_path)?; } - let mut temp_file = File::create(&temp_path)?; - - // write the new data to the temp file - writer(&mut temp_file)?; - - // force an fsync on the temp file to ensure bytes are on disk - temp_file.sync_all()?; - - rename(&temp_path, &original)?; + let write_result = (|| { + let mut temp_file = File::create(&temp_path)?; + writer(&mut temp_file).map_err(map_io_err)?; + // force an fsync on the temp file to ensure bytes are on disk + temp_file.sync_all().map_err(map_io_err)?; + // drop file handle before rename (important on Windows) + drop(temp_file); + rename(&temp_path, &original).map_err(map_io_err)?; + + // Fsync the parent directory so the rename is durable. + if let Some(parent) = original.parent() { + // On some platforms (e.g. certain Windows configs) directory sync + // may not be supported; treat that as best-effort. + if let Ok(dir) = OpenOptions::new().read(true).open(parent) { + let _ = dir.sync_all(); + } + } + Ok(()) + })(); + + if let Err(e) = write_result { + // Best-effort cleanup of the temp file so a failed write cannot + // leave a partial .tmp that confuses a later retry. + let _ = remove_file(&temp_path); + return Err(e); + } Ok(()) } diff --git a/store/src/lmdb.rs b/store/src/lmdb.rs index 2c7b495d2c..4f52758bf2 100644 --- a/store/src/lmdb.rs +++ b/store/src/lmdb.rs @@ -49,6 +49,11 @@ pub enum Error { /// Wraps an error originating from LMDB #[error("LMDB error: {0}")] LmdbErr(String), + /// Filesystem is out of space (ENOSPC / StorageFull) + #[error( + "Out of disk space: {0}. Free disk space and restart; do not wipe chain data unless recovery fails." + )] + DiskFull(String), /// Wraps a serialization error for Writeable or Readable #[error("Serialization Error: {0}")] SerErr(ser::Error), @@ -62,7 +67,29 @@ pub enum Error { impl From for Error { fn from(e: heed::Error) -> Error { - Error::LmdbErr(e.to_string()) + let msg = e.to_string(); + // heed/lmdb may surface ENOSPC as a map full / I/O error string. + let lower = msg.to_lowercase(); + if lower.contains("map full") + || lower.contains("mdb_map_full") + || lower.contains("no space") + || lower.contains("enospc") + || lower.contains("disk full") + { + Error::DiskFull(msg) + } else { + Error::LmdbErr(msg) + } + } +} + +impl From for Error { + fn from(e: std::io::Error) -> Error { + if crate::is_out_of_disk_space(&e) { + Error::DiskFull(e.to_string()) + } else { + Error::FileErr(e.to_string()) + } } } diff --git a/store/src/pmmr.rs b/store/src/pmmr.rs index 53c6871e13..6af6e72e46 100644 --- a/store/src/pmmr.rs +++ b/store/src/pmmr.rs @@ -372,16 +372,24 @@ impl PMMRBackend { /// Syncs all files to disk. A call to sync is required to ensure all the /// data has been successfully written to disk. + /// + /// On failure the error is mapped to a clear out-of-disk-space message when + /// appropriate (#3425). Callers should discard the extension and not commit + /// the corresponding DB batch. pub fn sync(&mut self) -> io::Result<()> { - Ok(()) - .and(self.hash_file.flush()) - .and(self.data_file.flush()) - .and(self.sync_leaf_set()) - .and(self.prune_list.flush()) + self.hash_file + .flush() + .and_then(|_| self.data_file.flush()) + .and_then(|_| self.sync_leaf_set()) + .and_then(|_| self.prune_list.flush()) .map_err(|e| { + let e = crate::map_io_err(e); + if crate::is_out_of_disk_space(&e) { + error!("PMMR sync failed — out of disk space: {}", e); + } io::Error::new( - io::ErrorKind::Interrupted, - format!("Could not sync pmmr to disk: {:?}", e), + e.kind(), + format!("Could not sync pmmr to disk: {}", e), ) }) } diff --git a/store/src/types.rs b/store/src/types.rs index 70cde93a2d..946c1ab855 100644 --- a/store/src/types.rs +++ b/store/src/types.rs @@ -353,10 +353,14 @@ where /// Syncs all writes (fsync), reallocating the memory map to make the newly /// written data accessible. + /// + /// On failure (including out-of-disk-space), the in-memory buffer and rewind + /// backup are preserved so the caller can `discard()` the extension rather + /// than treating a half-written file as committed. See #3425. pub fn flush(&mut self) -> io::Result<()> { if let SizeInfo::VariableSize(ref mut size_file) = &mut self.size_info { // Flush the associated size_file if we have one. - size_file.flush()? + size_file.flush().map_err(crate::map_io_err)? } if self.buffer_start_pos_bak > 0 { @@ -369,14 +373,16 @@ where .read(true) .create(true) .write(true) - .open(&self.path)?; + .open(&self.path) + .map_err(crate::map_io_err)?; // Set length of the file to truncate it as necessary. if self.buffer_start_pos == 0 { - file.set_len(0)?; + file.set_len(0).map_err(crate::map_io_err)?; } else { let (offset, size) = self.offset_and_size(self.buffer_start_pos - 1)?; - file.set_len(offset + size as u64)?; + file.set_len(offset + size as u64) + .map_err(crate::map_io_err)?; }; } } @@ -386,14 +392,39 @@ where .read(true) .create(true) .append(true) - .open(&self.path)?; + .open(&self.path) + .map_err(crate::map_io_err)?; self.file = Some(file); - self.buffer_start_pos_bak = 0; + // Intentionally do *not* clear buffer_start_pos_bak until write+sync + // succeed, so discard()/retry still know we were in a rewound state. } - self.file.as_mut().unwrap().write_all(&self.buffer[..])?; - self.file.as_mut().unwrap().sync_all()?; + let file = self + .file + .as_mut() + .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "append-only file not open"))?; + + if let Err(e) = file.write_all(&self.buffer[..]).and_then(|_| file.sync_all()) { + let e = crate::map_io_err(e); + if crate::is_out_of_disk_space(&e) { + error!( + "Out of disk space writing {}: free space and restart. \ + In-memory changes were not committed.", + self.path.display() + ); + } else { + error!( + "Failed writing append-only file {}: {}. In-memory changes were not committed.", + self.path.display(), + e + ); + } + // Keep buffer + buffer_start_pos_bak so discard() can roll back cleanly. + return Err(e); + } + // Write durable — only now clear rewind backup and buffer. + self.buffer_start_pos_bak = 0; self.buffer.clear(); self.buffer_start_pos = self.size_in_elmts()?; diff --git a/store/tests/disk_full.rs b/store/tests/disk_full.rs new file mode 100644 index 0000000000..e19dad0ddf --- /dev/null +++ b/store/tests/disk_full.rs @@ -0,0 +1,126 @@ +// Copyright 2021 The Grin Developers +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for out-of-disk-space detection and durable temp-file writes (#3425). + +use grin_store as store; +use std::fs; +use std::io::{self, Write}; +use std::path::PathBuf; + +fn temp_dir(name: &str) -> PathBuf { + let mut dir = PathBuf::from("target"); + dir.push("disk_full_tests"); + dir.push(name); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn detects_storage_full_kind() { + let err = io::Error::new(io::ErrorKind::StorageFull, "no space left on device"); + assert!(store::is_out_of_disk_space(&err)); +} + +#[test] +fn detects_enospc_raw_code() { + // ENOSPC is 28 on Linux/macOS. + let err = io::Error::from_raw_os_error(28); + assert!(store::is_out_of_disk_space(&err)); +} + +#[test] +fn map_io_err_upgrades_disk_full() { + let err = io::Error::new(io::ErrorKind::StorageFull, "disk full"); + let mapped = store::map_io_err(err); + assert_eq!(mapped.kind(), io::ErrorKind::StorageFull); + assert!(mapped.to_string().contains("out of disk space")); +} + +#[test] +fn does_not_flag_other_errors() { + let err = io::Error::new(io::ErrorKind::PermissionDenied, "permission denied"); + assert!(!store::is_out_of_disk_space(&err)); +} + +#[test] +fn save_via_temp_file_success_replaces_original() { + let dir = temp_dir("save_ok"); + let path = dir.join("data.bin"); + fs::write(&path, b"old").unwrap(); + + store::save_via_temp_file(&path, ".tmp", |f| f.write_all(b"new")).unwrap(); + + assert_eq!(fs::read(&path).unwrap(), b"new"); + assert!(!dir.join("data.bin.tmp").exists()); + let _ = fs::remove_dir_all(dir); +} + +#[test] +fn save_via_temp_file_keeps_original_on_writer_error() { + let dir = temp_dir("save_fail"); + let path = dir.join("data.bin"); + fs::write(&path, b"original").unwrap(); + + let err = store::save_via_temp_file(&path, ".tmp", |_f| { + Err(io::Error::new( + io::ErrorKind::StorageFull, + "no space left on device", + )) + }); + assert!(err.is_err()); + assert!(store::is_out_of_disk_space(err.as_ref().unwrap_err())); + + // Original content must be preserved; temp file cleaned up. + assert_eq!(fs::read(&path).unwrap(), b"original"); + assert!(!dir.join("data.bin.tmp").exists()); + let _ = fs::remove_dir_all(dir); +} + +#[test] +fn data_file_flush_happy_path_after_hardening() { + // Full ENOSPC simulation needs a tiny filesystem; here we verify + // basic append+flush still works after the #3425 changes. + let dir = temp_dir("data_flush"); + let path = dir.join("data.dat"); + + { + let mut file = store::types::DataFile::::open( + &path, + store::types::SizeInfo::FixedSize(8), + grin_core::ser::ProtocolVersion(1), + ) + .unwrap(); + file.append(&42u64).unwrap(); + file.flush().unwrap(); + file.append(&7u64).unwrap(); + file.flush().unwrap(); + assert_eq!(file.size(), 2); + assert_eq!(file.read(1), Some(42)); + assert_eq!(file.read(2), Some(7)); + } + + let file = store::types::DataFile::::open( + &path, + store::types::SizeInfo::FixedSize(8), + grin_core::ser::ProtocolVersion(1), + ) + .unwrap(); + assert_eq!(file.size(), 2); + assert_eq!(file.read(1), Some(42)); + assert_eq!(file.read(2), Some(7)); + + let _ = fs::remove_dir_all(dir); +}