Persist zones by ximon18 · Pull Request #550 · NLnetLabs/cascade

ximon18 · 2026-03-26T00:42:16Z

WIP: Exploring an idea, learning how the new zone storage state works.

Status

Zone edits (e.g. via file edit and zone reload or due to changes received via XFR) are persisted both for loaded and signed zones. On application restart persisted zones are restored and cascade zone status and dig AXFR appear to work as they did prior to application shutdown.

The code has been initially reviewed and led to some wanted changes:

@bal-e will extend the zonedata crate interface to enable moving some of the changes to Cascade itself.
Invoke save_now() instead of mark_dirty(). This is because persisting zone data to disk not enough, as if there was a power outage between calling mark_dirty() and publication of the zone the paths to the persisted files (that are recorded in state) would not have been written to disk, thus save_now() should be invoked instead. We may actually want to invoke save_now() in the publication server itself to ensure all published zone related state is persisted, but that is out of scope for this PR.

tl;dr

Extend Persister to write snapshots and deltas to disk (currently in DNS AXFR/IXFR wire format).
Extend ZoneState with two vecs of paths to written snapshot and delta files.
Introduce new storage state machine flows Uninitialized -> Passive and Uninitialized -> Restoring -> Passive
Extend Loader::init() to invoke new Loader::restore() prior to invoking enqueue_refresh().
Loader::restore() creates a Passive storage state populated from parsed snapshot and delta files as if the storage had moved through the pipeline being approved and signed (without actually invoking review hooks or doing signing).

PROs/CONs:

PRO: Doesn't touch the existing pipeline flow.
CON: Re-uses as much as it can but still has to emulate/copy some parts of the existing state machine logic to achieve the wanted end state.

Known issues:

No/minimal logging, docs, tests, or proper error handling (there's a discussion to be had over whether or not failure to persist/restore should be fatal or not).
No support (yet) for condensing/compacting/purging old persisted deltas.
Binary snapshot/delta format is not inspectable by operators. Could add a `cascade debug subcommand to render a snapshot/delta in XFR presentation format.
Will panic if snapshot/delta file is written then Cascade crashes before the vec of persisted file paths is updated/persisted, as on subsequent restart an attempt to persist to any existing file will be made which results in a panic.
There's no "scheduling" or spreading out of zone restoration activity, like refreshing it happens as soon as possible on loader initialisation.
The restoring state is not reflected in any status command output (yet).
There are no metrics for the restoration process (yet).
I have no idea if the unsafe { ... } blocks I had to add are "safe".
I have no idea if the curr_/next_ loaded_/signed_ boolean flags are set correctly, though when they were wrong Cascade bailed out so I think they are now correct.
Snapshots and deltas are loaded entirely into memory, while they could be processed in a streaming manner instead.
The persistence location is hard-coded at the moment.
Only tested (manually) on a small example.com zone so far.
Clippy currently fails.

Alternative ideas

Don't introduce new states, instead introduce a restoring flag that affects the behaviour of existing states so that a zone being restored can be moved through the pipeline without side effects (such as invoking review hooks or signing). This would however complicate the code/logic at multiple existing points in the pipeline for every update to every zone for a situation that only occurs once at startup, while the implemented approach leaves the existing pipeline flow untouched.
Don't use the pipeline at all to restore, instead serialize the entire final zone state to disk and deserialize it back on restart.

Alternative snapshot/delta storage formats

A choice would need to be driven by requirements:

Do we need to support IXFR out at the loaded review nameserver? If so, is just one delta enough? Actually do we even need to keep a loaded diff, a review hook would only need it for the next received zone update, not after restart.
What do we tune for? Memory usage? Persistence speed? Restoration speed?

Some ideas:

Presentation XFR format: was not done in this PR due to the pain of having to convert new base to old base to render in presentation format, and parsing DNS records in presentation format is only currently properly supported by domain/cascade for entire zones, though one could use the "treat a single RR line as a zonefile" hack that the Stelline code uses.
(De)serialize cascade DiffData to JSON/CBOR/whatever. Would tie the format to the internal format, changing the internal format would require writing additional code to handle storage format migration or support parsing old as well as new format. A PRO of this approach is being able to storage additional meta data with the snapshot/delta, if needed.

Issues encountered

State machine naming is quite confusing, e.g. transition() does not do a transition, in fact things happen after such calls prior to an actual move_to transition occurring.
I found trying to understand what the correct values for the instance data "double buffer" boolean flags should be at any particular point in the flow hard.
The zone builder / replacer / patcher didn't work as I expected they would, I initially tried to populate then repeatedly patch a single builder but that failed, instead I have to jump through extra steps, e.g. see switch_loaded_data() and switch_signed_data().
LoadedZonePatcher and SignedZonePatcher being separate types with identical interfaces but no common trait required creating duplicate code to use them even though the logic in both implementations is identical. A macro could be used to work around this but feels a bit heavy...
The Persister interface has no context to work with. I had to extend it to pass it a file path for example. Maybe it should take a closer so that the non-zonedata related code like file i/o can be specified at the call site?
Using booleans to index into the "double buffer" is confusing, and they have to be converted to usize anyway before they can be used as indices.

If you are changing Rust code or integration tests (Cargo.*, crates/, etc/, integration-tests/, src/):
- Did you run the integration tests with act through the act-wrapper (as described in TESTING.md)?

Initial zone version only so far, no diffs yet. Lots of code duplication, no docs, etc, i.e. PoC only.

As the DNS XFR wire format used is just an implementation detail, and it could be confusing if it looks like these perhaps relate to incoming XFRs which they do not.

bal-e

@ximon18 and I had a discussion while I was writing this review, and decided that I will add commits changing the PR to implement a new approach for the zone data storage state machine. I've left these comments here for us to come back once I'm done; I also didn't review everything, so there will be more to come.

bal-e · 2026-04-07T08:41:01Z

+
+    /// The index of the current loaded instance.
+    pub(super) curr_loaded_index: bool,
+
+    /// The index of the current signed instance.
+    pub(super) curr_signed_index: bool,


Are these fields necessary, if this state is only used at the very beginning (where they would both be 0/false)?

bal-e · 2026-04-07T08:41:37Z

    pub(super) curr_signed_index: bool,
 }

+pub struct UninitializedStorage {


This type (and RestoringPersistedStorage below) need documentation, similar to the other types in this file. I'm happy to write it by adding to the PR.

bal-e · 2026-04-07T08:42:11Z

        assert!(
            old_reviewer.loaded_index == self.curr_loaded_index,
-            "'old_reviewer' does not point to the current instance",
+            "'old_reviewer' does not point to the current instance: {} vs:{}",


{} vs:{} -> {} vs {}?

bal-e · 2026-04-07T08:46:10Z

+        let viewer = unsafe { ZoneViewer::new(self.data.clone(), true, true) };
+
+        let signed = unsafe { &*self.data.signed[0].get() };
+        let serial = signed.soa.as_ref().unwrap().rdata.serial;


I don't think the serial number should be extracted here. Perhaps we need methods on LoadedZoneBuilt and SignedZoneBuilt that give you readers so you can observe the data, or just getters for the serial?

bal-e · 2026-04-07T08:48:28Z

+        assert!(
+            Arc::ptr_eq(&signed_built.data, &self.data),
+            "'built' is for a different zone"
+        );


You should also check loaded_built. And the assert message should clarify which parameter it is referring to.

bal-e · 2026-04-07T09:51:42Z

+        let (transition, state) = if let ZoneDataStorage::Uninitialized(s) = state {
+            let s = s.initialize();
+            t.move_to(ZoneDataStorage::Passive(s));
+            transition(&mut self.state.storage.machine)
+        } else {
+            (t, state)
+        };


I'd prefer the above commented-out initialize() function over this silent automatic conversion. Especially since it should only occur at startup and we can handle it outside this function.

With the current setup, we might actually want to do this check in other places, but it is hard to tell.

bal-e · 2026-04-07T09:54:00Z

+                let (s, loaded_reviewer, signed_reviewer, viewer, serial) =
+                    s.finish(loaded_built, signed_built);
+                self.state.storage.loaded_reviewer = loaded_reviewer;
+                self.state.storage.signed_reviewer = signed_reviewer;


This code does not correctly handle the reviewer types. They should be passed back into the storage state machine, so it can guarantee that viewers only exist for the permitted 0/1 instances. We should discuss what the right adjustments to the storage state machine's flow are.

bal-e · 2026-04-07T09:58:47Z

+    /// The zone is in an empty initial uninitialized state pending possible
+    /// restoration from persisted state or initialization directly to an
+    /// empty passive state.
+    Uninitialized(UninitializedStorage),


It might be better to omit this state entirely and only have a Restoring state, which can then decide to proceed with or skip restoring. Restoring might already need that functionality, e.g. if the previously persisted instances could not be found.

bal-e · 2026-04-07T09:59:19Z

+    Uninitialized(UninitializedStorage),
+
+    /// The zone is being restored from persisted state.
+    Restoring(RestoringPersistedStorage),


The enum variant name should match the underlying name, for consistency. I think the underlying type should be RestoringStorage.

bal-e · 2026-04-07T09:59:56Z

@@ -86,9 +94,6 @@ pub enum ZoneDataStorage {
 impl ZoneDataStorage {
    /// Construct a new [`ZoneDataStorage`].
    pub fn new() -> (Self, LoadedZoneReviewer, SignedZoneReviewer, ZoneViewer) {


Perhaps we want to change this function so it no longer returns reviewers? That might be easier all around.

ximon18 added 7 commits March 24, 2026 22:43

WIP: Exploring zone persistence and reload.

b88426f

Initial zone version only so far, no diffs yet. Lots of code duplication, no docs, etc, i.e. PoC only.

Merge branch 'main' into persist-zones

d04a6dd

Merge in main.

a6a80c2

Factor out write rr persistence.

2526551

Fixes, extensions, refactoring.

1ce45b7

Revert unintended change.

46af7dd

Revert unintended change.

33a3130

ximon18 added this to the 0.1.0-beta1 milestone Mar 26, 2026

ximon18 added the enhancement New feature or request label Mar 26, 2026

ximon18 added 5 commits March 26, 2026 01:45

Remove outdated comment.

ff79761

Fix incorrect enum string representation.

28c1667

Remove unused added fn.

99fd24d

Undo unintended change.

adf3437

Make rename from _diffs to _xfrs Cascade wide, not just in one place.

7c2efba

ximon18 requested review from Philip-NLnetLabs, bal-e and tertsdiepraam March 27, 2026 09:12

ximon18 added 3 commits March 27, 2026 10:30

Correct copy-pasted enum variant comment.

08f0455

RustDoc improvement.

34aa751

Rename _xfrs back to _diffs.

0a1778c

As the DNS XFR wire format used is just an implementation detail, and it could be confusing if it looks like these perhaps relate to incoming XFRs which they do not.

ximon18 linked an issue Mar 30, 2026 that may be closed by this pull request

Zone persistence #295

Open

bal-e reviewed Apr 7, 2026

View reviewed changes

bal-e mentioned this pull request Apr 7, 2026

Integrate the zone server(s) with the new zone storage #552

Merged

1 task

ximon18 mentioned this pull request Apr 10, 2026

Cascade doesn't save state on SIGTERM. #573

Open

bal-e mentioned this pull request Apr 15, 2026

Implement restoring functionality in cascade-zonedata #589

Draft

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Persist zones#550

Persist zones#550
ximon18 wants to merge 15 commits intomainfrom
persist-zones

ximon18 commented Mar 26, 2026 •

edited

Loading

Uh oh!

bal-e left a comment

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

bal-e Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ximon18 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

tl;dr

PROs/CONs:

Known issues:

Alternative ideas

Alternative snapshot/delta storage formats

Issues encountered

Uh oh!

bal-e left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ximon18 commented Mar 26, 2026 •

edited

Loading