Skip to content

feat: Implement zstd checksums and other chunk corruption handling methods - #500

Open
Saul5662 wants to merge 10 commits into
Steel-Foundation:masterfrom
Saul5662:zstd-checksums
Open

feat: Implement zstd checksums and other chunk corruption handling methods#500
Saul5662 wants to merge 10 commits into
Steel-Foundation:masterfrom
Saul5662:zstd-checksums

Conversation

@Saul5662

@Saul5662 Saul5662 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Type of change

  • Block implementation
  • Item implementation
  • Command implementation
  • Entity implementation
  • Bug fix
  • New feature
  • Breaking change
  • Refactor / code cleanup
  • Performance improvement
  • Chore / tooling

Description

This PR implements zstd checksums for chunk corruption detection which passes corrupted chunks to regenerator along with other corruption related features which include:

  • Player data corruption detection and backup promotion. If the backup if corrupted it will fallback to the default behavior before this PR which was to disconnect the Player.
  • Region entry table corruption detection. Each chunk now records its own position and status inside its check-summed payload. On load these are compared against the entry table. If the position in the table and chunk are mismatched it is rejected as corruption. A flipped/corrupted offset could point at another chunks payload and decode cleanly as wrong terrain. A flipped status byte could could regenerate Full chunks causing further data loss.
  • Bumped to FORMAT_VERSION 23 to reflect the structure change. This will cause older versioned worlds to NOT load.
  • Version mismatches no longer use the ErrorKind:InvalidData and now use ErrorKind:Unsupported to prevent backup promotion from treating a version mismatch as invalid data.
  • Compression level and checksum policy are consolidated in one module instead of 2 zstd::encode_all calls.
  • Storage failures are now triaged instead of retried forever. See below.

There are a lot of tests added with this PR but since this is quite critical as its persistent data I felt they were justified. Please review and change these at will.

Fix infinite chunk-load retry loop

This PR also fixes a critical issue where if any of the following happen it triggers an infinite loop pinning a CPU core and flooding log output:

  • Disk fills
  • Disk quota is exceeded (linux mostly)
  • The world directory becomes read-only
  • The world directory has wrong ownership or permissions (common after restoring a backup as a different user)
  • The process runs out of file descriptors
  • A region file's header is corrupted
  • A region file is truncated
  • The disk is failing and a read returns an I/O error
  • A network mount (NFS/SMB) stalls or disconnects
  • An external process deletes or replaces a region file while the server is running

The cause is an error generated from storage.acquire_chunk which flows out of the spawned task and drops _status_claim. The claim's Drop is explicitly there to roll back abandoned work. Rolling back resets started_work to its parent status so the chunk looks empty. The scheduler sees it, re-drives it, and open_region then fails identically and the cycle repeats.

Storage errors are now triaged so callers only ever see retryable ones:

  • Fatal (StorageFull, QuotaExceeded, ReadOnlyFilesystem, PermissionDenied) nothing can be persisted again, so the server stops rather than silently discarding everything players do from that point on.
  • Corrupt (InvalidData, UnexpectedEof) the region file is moved to .srg.corrupt.bak and rebuilt, matching what a format version mismatch already does.
  • Transient (everything else) per-chunk backoff from 1s to a 60s ceiling, clearing on success, so a transient fault still recovers without a restart.

How this was tested

cargo build
cargo test --workspace
cargo clippy --all-targets --all-features
cargo fmt --all --check
All tests pass. Clippy passes with no warnings. Format passes with no suggestions.

Retry loop reproduced and measured with a corrupt region header and PREGEN_SIZE=9: 2571 errors sustained over 11 seconds before, 256 (one per ticketed chunk) finishing within 2 seconds after. With a read-only region directory: one fatal line and a clean stop at 0.42s, zero retry spam. An undamaged world is unaffected.

Screenshots / logs

0.00s [Info] Starting Steel Server (d18a67d)
0.03s [Info] Vanilla registry loaded in 29.731032ms
0.03s [Info] Behavior registries initialized
0.03s [Info] SteelMC is not affiliated with Mojang or Microsoft. Use is subject to the Minecraft EULA: https://aka.ms/MinecraftEULA
0.21s [Info] Preparing spawn area: 81 chunks (9x9) around chunk (0, 0)
0.21s [Info] Pregeneration windowing: 32x32 target chunks, 2 active windows, dependency halo 11 chunks
0.22s [Fatal] Unrecoverable storage failure, stopping the server: cannot access region storage at saves/minecraft/worlds/overworld/region: Permission denied (os error 13)
0.22s [Info] Spawn area preparation cancelled after 0.02s
0.28s [Info] Saving world data...
0.28s [Info] Saved 0 domain scoreboards
0.28s [Info] Saved 0 domain command storages
0.28s [Info] World minecraft:overworld level data saved successfully
0.28s [Info] World minecraft:overworld saved chunk ticket data successfully
0.28s [Info] Saving chunks
0.28s [Info] Chunk save complete
0.28s [Info] World minecraft:the_nether level data saved successfully
0.28s [Info] World minecraft:the_nether saved chunk ticket data successfully
0.28s [Info] Saving chunks
0.28s [Info] Chunk save complete
0.28s [Info] World minecraft:the_end level data saved successfully
0.28s [Info] World minecraft:the_end saved chunk ticket data successfully
0.28s [Info] Saving chunks
0.28s [Info] Chunk save complete
0.28s [Info] Saved 0 chunks
0.28s [Info] Saving player data...
0.28s [Info] Saved 0 players

Checklist

  • Code builds w/o errors or warnings
  • Self-reviewed the diff
  • Docs updated (if applicable)
  • No leftover debug code / comments

Additional notes

DISCLAIMER: This PR was AI assisted and manually reviewed.


@github-actions github-actions Bot added Breaking change Bug Something isn't working Enhancement New feature or request labels Aug 25, 2026
@kdcokenny

Copy link
Copy Markdown
Contributor

Is there any way you can simulate one of these corruption states in game?

@Saul5662

Saul5662 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Tests implemented in this PR should cover corruption handling. I don't exactly know how I would test corruption in-game? Let me experiment with that. Maybe just hex edit the zstd and see what happens.

@Saul5662

Copy link
Copy Markdown
Contributor Author

Created a bash script to test all this. Running through it.

@Saul5662
Saul5662 marked this pull request as draft August 25, 2026 22:00
@Saul5662

Copy link
Copy Markdown
Contributor Author

Forgot to add. All the tests I ran passed and confirmed chunks regenerate when they are supposed to and the server recovers from header corruption without losing data.

@Saul5662

Copy link
Copy Markdown
Contributor Author

Updated PR description to reflect the new fix within this PR.

@Saul5662
Saul5662 marked this pull request as ready for review August 26, 2026 00:40
@Saul5662 Saul5662 changed the title feat: Implement zstd checksums and other chunk corruption methods feat: Implement zstd checksums and other chunk corruption handling methods Aug 26, 2026

@kdcokenny kdcokenny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if disk space runs out after a region has already been opened and also, should one invalid chunk-table entry cause the entire region to regenerate?

@Saul5662

Copy link
Copy Markdown
Contributor Author

Ah I didnt catch these that is my fault for not testing these cases properly. Addressing these issues right now. Give me a sec.

@kdcokenny

Copy link
Copy Markdown
Contributor

Ah I didnt catch these that is my fault for not testing these cases properly. Addressing these issues right now. Give me a sec.

no worries at all. this is a complex area with a ton of edge cases haha

@Saul5662

Saul5662 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

I also noticed an issue where region writes dont fsync only file.flush(). On ZFS/BTRFS this could cause an in-place header rewrite basically getting it a partial write on power loss which currently would also nuke the region. Is this something to fix in this PR or was this an intentional thing? Player data does fsync.

@kdcokenny

Copy link
Copy Markdown
Contributor

I also noticed an issue where region writes dont fsync only file.flush(). On ZFS/BTRFS this could cause an in-place header rewrite basically getting it a partial write on power loss which currently would also nuke the region. Is this something to fix in this PR or was this an intentional thing? Player data does fsync.

not intentional, and i think you should fix it in this pr. ideally use vanilla’s sync-chunk-writes behavior, including writing chunk data durably before updating the header. Not sure if this would take a hit on perf tho. testing would be needed

@Saul5662

Saul5662 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

not intentional, and i think you should fix it in this pr. ideally use vanilla’s sync-chunk-writes behavior, including writing chunk data durably before updating the header. Not sure if this would take a hit on perf tho. testing would be needed

Gotcha. Running some benchmarks to test if this is a massive performance hit and will start implementing it.

EDIT: Sync chunk writes are 1478 µs per chunk without batching on my NVMe so thats a no go. What I propose is batched saves. Basically we write all the payloads in the batch, sync, write dirty headers, sync. Per chunk it goes from 8.4 µs to 19.8 µs. Which IS slower than our current implementation

@Saul5662

Copy link
Copy Markdown
Contributor Author

Just adding a comment to say first comment's issues should be fixed.

@kdcokenny kdcokenny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering what would happen if two chunk-table entries overlap, but the lower-index entry is the corrupt one? It seems to me that the current repair would keep that entry and delete the potentially valid one, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking change Bug Something isn't working Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants