Skip to content

Commit 6cbb0eb

Browse files
committed
[#268] fix: CodeRabbit 리뷰 반영 - torn tail truncate 및 commit marker 순서 수정
- FileSystem 트레이트에 truncate 메서드 추가 (RealFileSystem 구현) - newest 세그먼트의 torn tail을 recovery 시 디스크에서 실제 truncate: stale bytes가 rotation 후 intermediate corruption으로 이어지는 것 방지 - write_entry의 header 슬롯 예약을 실제 frame_len이 아닌 0으로 수행: 예약 시점에 commit marker가 노출되는 문제 수정 - torn tail truncate 검증 테스트 추가
1 parent 4f01070 commit 6cbb0eb

3 files changed

Lines changed: 68 additions & 4 deletions

File tree

src/common/fs.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ pub trait FileSystem {
1414
async fn write_file(&self, path: &str, content: &[u8]) -> io::Result<()>;
1515
async fn read_dir(&self, path: &str) -> io::Result<Vec<FileSystemEntry>>;
1616
async fn read(&self, path: &Path) -> io::Result<Vec<u8>>;
17+
/// 파일을 지정한 크기로 자릅니다. (#268)
18+
/// WAL 복구 시 newest segment의 torn tail을 제거하는 데 사용합니다.
19+
async fn truncate(&self, path: &Path, size: u64) -> io::Result<()>;
1720
/// 파일의 크기(bytes)를 반환합니다. (#265)
1821
/// `read_segment_rows`가 파일 전체를 메모리로 읽기 전에 예산을 확보하는 데 사용합니다.
1922
async fn metadata(&self, path: &Path) -> io::Result<u64>;
@@ -49,6 +52,11 @@ impl FileSystem for RealFileSystem {
4952
tokio::fs::read(path).await
5053
}
5154

55+
async fn truncate(&self, path: &Path, size: u64) -> io::Result<()> {
56+
let file = tokio::fs::OpenOptions::new().write(true).open(path).await?;
57+
file.set_len(size).await
58+
}
59+
5260
async fn metadata(&self, path: &Path) -> io::Result<u64> {
5361
let metadata = tokio::fs::metadata(path).await?;
5462
Ok(metadata.len())

src/engine/wal/manager/builder.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,20 @@ impl<'a> WALBuilder<'a> {
110110
// before it, following the PostgreSQL/SQLite recovery
111111
// convention.
112112
log::warn!("discarding torn WAL tail in {:?}: {}", path, reason);
113+
114+
// Truncate the torn tail on disk. Otherwise the stale bytes
115+
// would survive a checkpoint and, once this segment rotates
116+
// into an intermediate one, the next restart would treat
117+
// them as intermediate corruption and fail to start.
118+
self.file_system
119+
.truncate(&path, used_bytes as u64)
120+
.await
121+
.map_err(|e| {
122+
WALError::wrap(format!(
123+
"failed to truncate torn WAL tail in {:?}: {}",
124+
path, e
125+
))
126+
})?;
113127
}
114128

115129
let entries: Vec<WALEntry> = decoder.decode(&content[..used_bytes]).map_err(|e| {

src/engine/wal/manager/mod.rs

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,13 @@ where
126126
// memcpy, the on-disk frame either has len == 0 (unwritten) or a
127127
// truncated body under a valid len — both are discarded on recovery.
128128
let header_offset = self.current_offset;
129-
let header = &frame[..size_of::<u32>()];
130-
// Reserve the header slot first (as zeros = "not committed"), then
131-
// write the body, then patch the header last (see #268).
132-
self.append_frame_to_mmap(header).await?;
129+
// Reserve the header slot as zeros first: an unwritten slot can never
130+
// be mistaken for a committed frame. Then write the body, and patch
131+
// the real length header last (see #268).
132+
self.append_frame_to_mmap(&[0u8; size_of::<u32>()]).await?;
133133
let body = &frame[size_of::<u32>()..];
134134
self.append_frame_to_mmap(body).await?;
135+
let header = &frame[..size_of::<u32>()];
135136
self.patch_frame_header_at(header_offset, header).await?;
136137

137138
self.unsynced_bytes += frame.len();
@@ -734,6 +735,47 @@ mod tests {
734735
assert_eq!(payloads, vec![b"complete".to_vec()]);
735736
}
736737

738+
/// #268: after discarding a torn tail the file itself must be truncated,
739+
/// so the segment stays clean even after it later becomes an intermediate
740+
/// segment (rotation) — otherwise the next restart would fail.
741+
#[tokio::test]
742+
async fn test_build_truncates_torn_tail_from_disk() {
743+
let wal_dir = setup_test_wal_dir("truncate_torn_tail").await;
744+
let config = get_test_config(&wal_dir);
745+
write_wal_file(
746+
&config,
747+
1,
748+
&vec![create_entry(EntryType::Insert, Some("complete"))],
749+
)
750+
.await;
751+
752+
let path = wal_dir.join(format!("00000001.{}", config.wal_extension));
753+
let intact_len = tokio::fs::metadata(&path).await.unwrap().len();
754+
{
755+
let mut file = tokio::fs::OpenOptions::new()
756+
.append(true)
757+
.open(&path)
758+
.await
759+
.unwrap();
760+
file.write_all(&8u32.to_le_bytes()).await.unwrap();
761+
file.write_all(&[1, 2, 3]).await.unwrap();
762+
}
763+
assert!(tokio::fs::metadata(&path).await.unwrap().len() > intact_len);
764+
765+
let wal_manager = WALBuilder::new(&config)
766+
.build(BincodeDecoder::new(), BincodeEncoder::new())
767+
.await
768+
.expect("a torn tail must not prevent startup");
769+
assert_eq!(wal_manager.pending_entries().len(), 1);
770+
771+
// The torn bytes must be gone from the file itself.
772+
assert_eq!(
773+
tokio::fs::metadata(&path).await.unwrap().len(),
774+
intact_len,
775+
"the torn tail must be truncated from disk, not just ignored"
776+
);
777+
}
778+
737779
#[tokio::test]
738780
async fn test_build_rejects_corrupt_intermediate_segment() {
739781
let wal_dir = setup_test_wal_dir("corrupt_intermediate_segment").await;

0 commit comments

Comments
 (0)