[PR #322] [MERGED] Fix race condition in chunk removal and compaction leading to metadata corruption #369

New issue

Closed

opened 2026-10-26 09:49:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 09:49:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/3FS/pull/322
Author: @cccshuang
Created: 8/19/2025
Status: ✅ Merged
Merged: 8/19/2025
Merged by: @SF-Zhou

Base: main ← Head: bugfix-chunk-removal-and-compaction-race-condition

📝 Commits (2)

5ae844c Fix race condition in chunk removal and compaction leading to metadata corruption
44f0781 remove useless enumerate in batch_remove

📊 Changes

1 file changed (+14 additions, -9 deletions)

View changed files

📝 src/storage/chunk_engine/src/core/engine.rs (+14 -9)

📄 Description

Issue

When performing chunk deletion alongside compaction in the chunk_engine of the storage component, metadata inconsistency issues can occur due to a race condition.

Specifically:

A deleted chunk (ck1) may still appear in the metadata as if it exists.
The position (pos1) previously used by ck1 is later reused by another chunk (ck2).
This results in an inconsistent state: ck1 → pos1, ck2 → pos1.

Root Cause

The issue arises from a concurrency problem between:

Chunk removal in commit_chunk
Position replace in compaction's move_chunk

Both operations modify the meta_store and meta_cache, but their updates are not atomic and lack proper synchronization. For example:

commit_chunk deletes ck1 from the store but leaves it in the cache;
github.com/deepseek-ai/3FS@394583dbf0/src/storage/chunk_engine/src/core/engine.rs (L475-L480)
move_chunk checks the cache and mistakenly assumes ck1 is valid;
github.com/deepseek-ai/3FS@394583dbf0/src/storage/chunk_engine/src/core/engine.rs (L230-L240)
It then writes new metadata using stale information about ck1, causing corruption.

A similar issue also exists in batch_remove.
github.com/deepseek-ai/3FS@394583dbf0/src/storage/chunk_engine/src/core/engine.rs (L666-L670)

Example Flow

remove chunk1	compact group: move chunk1	chunk2 write
In `commit_chunk`, check if `chunk.is_removed`
Delete metadata in `meta_store`: `ck1 -> pos1`, `pos1 -> ck1`
	Acquire `meta_cache` lock
	Check if `chunk1` exists in the cache. If yes:
	Modify metadata in `meta_store`: - Remove `ck1 -> pos1`, `pos1 -> ck1` - Insert `ck1 -> pos2`, `pos2 -> ck1`
	Update cache: `ck1 -> pos2`
	Release `meta_cache` lock
Acquire `meta_cache` lock
Remove cache entry: `ChunkArc` reference count drops to zero, chunk is dereferenced, releasing `pos1` and `pos2`
Release `meta_cache` lock
`chunk1` no longer exists in cache, but metadata still contains `ck1 -> pos2`, `pos2 -> ck1`
		Acquire `pos2`, insert metadata `ck2 -> pos2`, `pos2 -> ck2`, overwriting `pos2 -> ck1`
`chunk1` no longer exists in cache, but metadata still contains `ck1 -> pos2`

Solution

This PR introduces proper locking around commit_chunk and batch_remove operations to ensure all changes to meta_cache and meta_store are synchronized and atomic. This prevents race conditions and ensures consistent metadata across concurrent chunk removal and compaction's move chunk workflows.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/3FS/pull/322 **Author:** [@cccshuang](https://github.com/cccshuang) **Created:** 8/19/2025 **Status:** ✅ Merged **Merged:** 8/19/2025 **Merged by:** [@SF-Zhou](https://github.com/SF-Zhou) **Base:** `main` ← **Head:** `bugfix-chunk-removal-and-compaction-race-condition` --- ### 📝 Commits (2) - [`5ae844c`](https://github.com/deepseek-ai/3FS/commit/5ae844c84fbbde0e88e281acba817ac2d50d9954) Fix race condition in chunk removal and compaction leading to metadata corruption - [`44f0781`](https://github.com/deepseek-ai/3FS/commit/44f0781a7441972d8a34b9017984f7a32d1dc36d) remove useless enumerate in batch_remove ### 📊 Changes **1 file changed** (+14 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/storage/chunk_engine/src/core/engine.rs` (+14 -9) </details> ### 📄 Description ## Issue When performing chunk deletion alongside compaction in the `chunk_engine` of the storage component, metadata inconsistency issues can occur due to a race condition. Specifically: - A deleted chunk (`ck1`) may still appear in the metadata as if it exists. - The position (`pos1`) previously used by `ck1` is later reused by another chunk (`ck2`). - This results in an inconsistent state: `ck1 → pos1`, `ck2 → pos1`. <img width="1098" height="665" alt="image" src="https://github.com/user-attachments/assets/9ddcb043-aeda-4422-acf8-df70e8490cf2" /> ## Root Cause The issue arises from a concurrency problem between: - **Chunk removal** in `commit_chunk` - **Position replace** in compaction's `move_chunk` Both operations modify the `meta_store` and `meta_cache`, but their updates are not atomic and lack proper synchronization. For example: - `commit_chunk` deletes `ck1` from the store but leaves it in the cache; https://github.com/deepseek-ai/3FS/blob/394583dbf08d2d4931f236217ccdfaaeba0c515b/src/storage/chunk_engine/src/core/engine.rs#L475-L480 - `move_chunk` checks the cache and mistakenly assumes `ck1` is valid; https://github.com/deepseek-ai/3FS/blob/394583dbf08d2d4931f236217ccdfaaeba0c515b/src/storage/chunk_engine/src/core/engine.rs#L230-L240 - It then writes new metadata using stale information about `ck1`, causing corruption. A similar issue also exists in `batch_remove`. https://github.com/deepseek-ai/3FS/blob/394583dbf08d2d4931f236217ccdfaaeba0c515b/src/storage/chunk_engine/src/core/engine.rs#L666-L670 ## Example Flow | remove chunk1 | compact group: move chunk1 | chunk2 write | | --- | --- | --- | | In `commit_chunk`, check if `chunk.is_removed` | | | | Delete metadata in `meta_store`: `ck1 -> pos1`, `pos1 -> ck1` | | | | | Acquire `meta_cache` lock | | | | Check if `chunk1` exists in the cache. If yes: | | | | Modify metadata in `meta_store`:<br> - Remove `ck1 -> pos1`, `pos1 -> ck1`<br> - Insert `ck1 -> pos2`, `pos2 -> ck1` | | | | Update cache: `ck1 -> pos2` | | | | Release `meta_cache` lock | | | Acquire `meta_cache` lock | | | | Remove cache entry: `ChunkArc` reference count drops to zero, chunk is dereferenced, releasing `pos1` and `pos2` | | | | Release `meta_cache` lock | | | | `chunk1` no longer exists in cache, but metadata still contains `ck1 -> pos2`, `pos2 -> ck1` | | | | | | Acquire `pos2`, insert metadata `ck2 -> pos2`, `pos2 -> ck2`, overwriting `pos2 -> ck1` | | `chunk1` no longer exists in cache, but metadata still contains `ck1 -> pos2` | | | ## Solution This PR introduces proper locking around `commit_chunk` and `batch_remove` operations to ensure all changes to `meta_cache` and `meta_store` are synchronized and atomic. This prevents race conditions and ensures consistent metadata across concurrent chunk removal and compaction's move chunk workflows. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 09:49:14 +00:00