Synvael/synvael

Watch

Fork 0

feat/region-incremental-save #4

Merged

Serkyo merged 8 commits from feat/region-incremental-save into dev

2026-09-13 12:55:54 +00:00

Serkyo commented

2026-09-13 12:55:48 +00:00

Owner

What does this PR do?

I made region saves incremental, and I added the observed bitset that depends on the same groundwork.

The format change comes first, in crates/shared/src/save/region.rs. RegionIndex gains two fields: payload_start, the offset the first chunk record may begin at, and observed, a packed 4096-byte bitset with one bit per chunk slot in the region's 32x32x32 cube. encode now pads out to payload_start and raises the new SaveError::IndexReservationExceeded if the index content no longer fits its reservation, so the index has fixed-size space on disk it can grow into without shifting a single record offset. encoded_len reports the index's natural length so a caller can size a reservation before laying out records. observed_slot maps a ChunkPos to its local slot with rem_euclid on each axis, and is_observed / mark_observed / clear_observed / observed_count read and write the bits. I also exposed take_free, clear_free, payload_start and set_payload_start, which the allocator needs. REGION_FORMAT_VERSION goes from 1 to 3.

crates/server/src/save/region_file.rs carries the actual save rework. RegionFile tracks file_len, file_exists and a dirty_records set, and write_chunk no longer assigns an offset: it records the bytes and marks the position dirty, leaving placement to save time. save now branches. The common path is write_incremental, which allocates a slot per dirty record through allocate (best-fit over the free list, splitting the leftover back onto it, falling back to an append at file_len), writes those records at their offsets, fsyncs, then rewrites the header in place and fsyncs again. Records first and header second, so a crash between the two leaves the file readable at its previous state. A brand-new region, or one whose index has outgrown its reservation and hit IndexReservationExceeded, falls through to rewrite_whole_file, which clears the free list, sizes a fresh reservation via next_reservation (doubling from a 64 KiB default), packs every record contiguously after it and commits the image through the existing atomic_write. That replaces the serialize whole-file path and the TODO: incremental save that sat on it. remove_chunk now pushes the vacated span onto the free list instead of dropping it.

read_chunk in crates/server/src/save/region_actor.rs marks the chunk's slot observed as it reads, since every read on that path is an LOD0 load today. flush_dirty evicts any region whose save failed rather than keeping it cached, because a partial incremental write can leave the in-memory index naming offsets the on-disk header never got, and reopening from disk is the only sound recovery.

The autosave timer closes the loop. ServerWorld::flush in crates/server/src/world_server.rs wraps the SaveRequest::Flush round trip that the tests were building by hand, and run_simulation in crates/server/src/main.rs fires it every 45 seconds off a new AUTOSAVE_INTERVAL, seeded at startup so the first autosave lands one interval in.

I added crates/shared/examples/inspect_save.rs to read this back. It prints a summary of a world's level.dat and its region files through the same decode functions the server uses, so the observed counts and the payload-start reservation can be checked on real files rather than only in tests.

Why is this change necessary?

Every save was a whole-file rewrite, so save cost scaled with a region's total size rather than with what changed. That was tolerable while only edits dirtied a region. It stops being tolerable with the observed bitset, because marking a slot observed dirties the region on a plain read: exploring a world would have triggered repeated full rewrites to set single bits.

The blocker on incremental save was that header_table is variable-length, so index growth shifted every absolute record offset and forced the whole file out. Padding the index to a fixed reservation is what removes that coupling, and it is what makes the observed bits affordable, which is why Voxel Chunk System II cards the two as wanting to be done together.

The bitset itself answers a question base_worldgen_version and the stamp table cannot. Those say at what version a chunk is pinned; they cannot say whether a chunk was observed at all, because a chunk pinned to the region base has no stamp entry and absence therefore means both "never seen" and "pinned to base". Worldgen versioning needs that distinction: an unvisited chunk in an old region must generate at the new version while its visited neighbour must not. Architecture/Worldgen.md also needs it for the no-re-blend rule. One bit per chunk is 4096 fixed bytes per region against 512 KiB for a fully explored region's worth of 16-byte stamp entries, and RegionIndex::encode writes raw little-endian with no zstd wrapper, so nothing downstream would compress that away.

The format bump is the reason to land this now rather than later. No world exists off my machine, so changing the SYNR header costs nothing today and costs a permanent migrator afterwards.

Scope of Changes

server, shared

Testing

Automated, all on this branch at 88319a0:

cargo check -p shared, cargo check -p server: clean.
cargo test -p shared: 215 passed, 0 failed.
cargo test -p server: 54 passed, 0 failed.
cargo clippy --all-targets --all-features -- -D warnings: clean.
cargo fmt --all -- --check: clean.

No Lua changed, so selene and stylua were not run.

New tests covering the change:

crates/shared/src/tests/region.rs: the observed bit round-trips through encode/decode, marking and clearing are independent per slot, observed_slot wraps to the same slot regardless of which region the position falls in, encode pads out to the reservation, encode rejects a reservation smaller than its content, a pre-payload_start region is rejected rather than misread, and take_free removes exactly the named span.
crates/server/src/tests/region_file.rs: an incremental save leaves the other records untouched on disk, a reservation overflow falls back to a full rewrite, an observed mark round-trips through disk, marking an already-observed slot does not re-dirty the region, and a failed incremental write is reported as an error rather than swallowed.
crates/server/src/tests/region_actor.rs: a failed flush evicts the region so the next access reopens it from disk.
crates/server/src/tests/world_server.rs: loading a chunk marks its region slot observed even when no diff is ever stored for it.

Additional Context

REGION_FORMAT_VERSION moves 1 to 3, and there is no migrator. Any region file written before this branch fails to decode. That is intended while no world exists outside my machine, and it is the window in which this change is free. The second bump is payload_start, which changes the header's byte layout ahead of the bitset and needed its own version so an old file fails loudly instead of reading a record offset as a reservation.

RegionFile::save has a documented failure contract worth a reviewer's attention: an I/O error partway through write_incremental can leave the in-memory index naming offsets the on-disk header never received. It propagates rather than swallowing so flush_dirty can evict the region, and that eviction is now the correctness boundary rather than a nicety.

The reservation policy is a decision that could have gone the other way. I picked a 64 KiB default doubling on overflow, which means a region whose index outgrows it pays one full rewrite and then has headroom again. A tighter default would save disk on regions nobody visits, at the cost of more full rewrites early in a world's life.

No response

Checklist

I have branched from dev (or a feature branch off dev) and my PR targets dev.
I have kept my changes focused to a single concept.
I have added or updated documentation (/// doc comments for Rust) where necessary.
I have tested my changes and described any relevant automated or manual testing above.

### What does this PR do? I made region saves incremental, and I added the observed bitset that depends on the same groundwork. The format change comes first, in `crates/shared/src/save/region.rs`. `RegionIndex` gains two fields: `payload_start`, the offset the first chunk record may begin at, and `observed`, a packed 4096-byte bitset with one bit per chunk slot in the region's 32x32x32 cube. `encode` now pads out to `payload_start` and raises the new `SaveError::IndexReservationExceeded` if the index content no longer fits its reservation, so the index has fixed-size space on disk it can grow into without shifting a single record offset. `encoded_len` reports the index's natural length so a caller can size a reservation before laying out records. `observed_slot` maps a `ChunkPos` to its local slot with `rem_euclid` on each axis, and `is_observed` / `mark_observed` / `clear_observed` / `observed_count` read and write the bits. I also exposed `take_free`, `clear_free`, `payload_start` and `set_payload_start`, which the allocator needs. `REGION_FORMAT_VERSION` goes from 1 to 3. `crates/server/src/save/region_file.rs` carries the actual save rework. `RegionFile` tracks `file_len`, `file_exists` and a `dirty_records` set, and `write_chunk` no longer assigns an offset: it records the bytes and marks the position dirty, leaving placement to save time. `save` now branches. The common path is `write_incremental`, which allocates a slot per dirty record through `allocate` (best-fit over the free list, splitting the leftover back onto it, falling back to an append at `file_len`), writes those records at their offsets, fsyncs, then rewrites the header in place and fsyncs again. Records first and header second, so a crash between the two leaves the file readable at its previous state. A brand-new region, or one whose index has outgrown its reservation and hit `IndexReservationExceeded`, falls through to `rewrite_whole_file`, which clears the free list, sizes a fresh reservation via `next_reservation` (doubling from a 64 KiB default), packs every record contiguously after it and commits the image through the existing `atomic_write`. That replaces the `serialize` whole-file path and the `TODO: incremental save` that sat on it. `remove_chunk` now pushes the vacated span onto the free list instead of dropping it. `read_chunk` in `crates/server/src/save/region_actor.rs` marks the chunk's slot observed as it reads, since every read on that path is an LOD0 load today. `flush_dirty` evicts any region whose save failed rather than keeping it cached, because a partial incremental write can leave the in-memory index naming offsets the on-disk header never got, and reopening from disk is the only sound recovery. The autosave timer closes the loop. `ServerWorld::flush` in `crates/server/src/world_server.rs` wraps the `SaveRequest::Flush` round trip that the tests were building by hand, and `run_simulation` in `crates/server/src/main.rs` fires it every 45 seconds off a new `AUTOSAVE_INTERVAL`, seeded at startup so the first autosave lands one interval in. I added `crates/shared/examples/inspect_save.rs` to read this back. It prints a summary of a world's `level.dat` and its region files through the same decode functions the server uses, so the observed counts and the payload-start reservation can be checked on real files rather than only in tests. ### Why is this change necessary? Every save was a whole-file rewrite, so save cost scaled with a region's total size rather than with what changed. That was tolerable while only edits dirtied a region. It stops being tolerable with the observed bitset, because marking a slot observed dirties the region on a plain read: exploring a world would have triggered repeated full rewrites to set single bits. The blocker on incremental save was that `header_table` is variable-length, so index growth shifted every absolute record offset and forced the whole file out. Padding the index to a fixed reservation is what removes that coupling, and it is what makes the observed bits affordable, which is why [[Voxel Chunk System II]] cards the two as wanting to be done together. The bitset itself answers a question `base_worldgen_version` and the stamp table cannot. Those say at what version a chunk is pinned; they cannot say whether a chunk was observed at all, because a chunk pinned to the region base has no stamp entry and absence therefore means both "never seen" and "pinned to base". Worldgen versioning needs that distinction: an unvisited chunk in an old region must generate at the new version while its visited neighbour must not. `Architecture/Worldgen.md` also needs it for the no-re-blend rule. One bit per chunk is 4096 fixed bytes per region against 512 KiB for a fully explored region's worth of 16-byte stamp entries, and `RegionIndex::encode` writes raw little-endian with no zstd wrapper, so nothing downstream would compress that away. The format bump is the reason to land this now rather than later. No world exists off my machine, so changing the `SYNR` header costs nothing today and costs a permanent migrator afterwards. ### Scope of Changes server, shared ### Testing Automated, all on this branch at 88319a0: - `cargo check -p shared`, `cargo check -p server`: clean. - `cargo test -p shared`: 215 passed, 0 failed. - `cargo test -p server`: 54 passed, 0 failed. - `cargo clippy --all-targets --all-features -- -D warnings`: clean. - `cargo fmt --all -- --check`: clean. No Lua changed, so `selene` and `stylua` were not run. New tests covering the change: - `crates/shared/src/tests/region.rs`: the observed bit round-trips through encode/decode, marking and clearing are independent per slot, `observed_slot` wraps to the same slot regardless of which region the position falls in, `encode` pads out to the reservation, `encode` rejects a reservation smaller than its content, a pre-`payload_start` region is rejected rather than misread, and `take_free` removes exactly the named span. - `crates/server/src/tests/region_file.rs`: an incremental save leaves the other records untouched on disk, a reservation overflow falls back to a full rewrite, an observed mark round-trips through disk, marking an already-observed slot does not re-dirty the region, and a failed incremental write is reported as an error rather than swallowed. - `crates/server/src/tests/region_actor.rs`: a failed flush evicts the region so the next access reopens it from disk. - `crates/server/src/tests/world_server.rs`: loading a chunk marks its region slot observed even when no diff is ever stored for it. ### Additional Context `REGION_FORMAT_VERSION` moves 1 to 3, and there is no migrator. Any region file written before this branch fails to decode. That is intended while no world exists outside my machine, and it is the window in which this change is free. The second bump is `payload_start`, which changes the header's byte layout ahead of the bitset and needed its own version so an old file fails loudly instead of reading a record offset as a reservation. `RegionFile::save` has a documented failure contract worth a reviewer's attention: an I/O error partway through `write_incremental` can leave the in-memory index naming offsets the on-disk header never received. It propagates rather than swallowing so `flush_dirty` can evict the region, and that eviction is now the correctness boundary rather than a nicety. The reservation policy is a decision that could have gone the other way. I picked a 64 KiB default doubling on overflow, which means a region whose index outgrows it pays one full rewrite and then has headroom again. A tighter default would save disk on regions nobody visits, at the cost of more full rewrites early in a world's life. ### Related Issues _No response_ ### Checklist - [x] I have branched from `dev` (or a feature branch off `dev`) and my PR targets `dev`. - [x] I have kept my changes focused to a single concept. - [x] I have added or updated documentation (`///` doc comments for Rust) where necessary. - [x] I have tested my changes and described any relevant automated or manual testing above.

Serkyo added 8 commits

2026-09-13 12:55:48 +00:00

feat(shared): add observed bitset to region index 124bebf926

feat(server): mark region slot observed on LOD0 load 2fa9eb4766

feat(server): incremental region saves and a periodic autosave 93b3560195

feat(shared): add an inspect_save example for level.dat and regions 73fff019de

style(workspace): un-wrap manually wrapped comments cfa762c93f

fix(shared): thread the legacy worldgen config through inspect_save 5e0274f28e

level::decode now takes the shipped version-0 config to migrate a
legacy level.dat, so the example needs one too. It loads the same
assets/data/worldgen/default/0.json the server bootstraps from.

fix(shared): bump REGION_FORMAT_VERSION for the payload_start field e3d002ff23

The payload_start field was added to the encoded header without moving
REGION_FORMAT_VERSION off 2, so a region saved under the prior layout
would be misread rather than rejected: its header_table_length sits
exactly where payload_start's low bytes now do. Bumped to 3 and added
a regression test that hand-builds the pre-bump byte layout to confirm
decode rejects it instead of misparsing it.

fix(server): evict a region whose flush failed instead of reusing it

Auto Labeler / label-scope (pull_request_target) Successful in 4s

Details

CLA Signed All authors have signed the CLA.

CLA Check / cla-check (pull_request_target) Successful in 4s

Details

CI / Rust Check, Lint & Test (pull_request) Successful in 45m41s

Details

CI / Dependency Licenses & Advisories (pull_request) Successful in 17s

Details

CI / Lua Lint & Format (pull_request) Successful in 7s

Details

CI / Commit Message Lint (pull_request) Successful in 3s

Details

CI / LFS Pointer Guard (pull_request) Successful in 6s

Details

88319a0a0f

write_incremental's own doc comment already warned that a partial I/O
failure can leave the in-memory index naming offsets never written to
disk, and that the region should be reopened from disk rather than
reused for further saves. Nothing enforced that: flush_dirty logged
the error and left the same corrupted RegionFile cached, so a later
save could persist a header pointing at bytes that were never
actually written. flush_dirty now evicts any region whose save fails,
so the next access reopens it from disk.