An archive this server wrote cannot be restored into it #195

New issue

Closed

opened 2026-09-12 08:50:57 +02:00 by Claude · 1 comment

Claude commented

2026-09-12 08:50:57 +02:00

Collaborator

An archive is written, listed and its manifest read; restoring one is refused.

// crates/ephyra-http/src/routes/admin/backup.rs:113
tracing::warn!(archive = %named, "restore requested; this server restores no archive");
Err(ApiError::Internal(
    "This server does not restore backups.".to_owned(),
))

The refusal is deliberate and is argued in ENDPOINTS.md — the archives on
disk, the manifest of one, and the writing of one upstream's own restore reads;
nothing here restores, so that write refuses. It is the one route in the group
that answers 500 on a well-formed request, and the dashboard's restore button
therefore fails on a server that made the archive itself.

What exists already, and what it gives the restore for free

The write half is complete and generated, which means the descriptor a restore
needs is already in the tree:

crates/ephyra-db/src/generated/backup.rs — TABLES, one entry per table in
the order upstream writes them, one Field per entity property with its
Encoding. Generated by cargo xtask gen-backup from the pin; never edited.
crates/ephyra-db/src/backup.rs — turns a row into one JSON element under
each of those descriptors. A restore is the same table walked the other way.
crates/ephyra-http/src/routes/admin/backup.rs — the four routes, including
the manifest reader a restore would check the archive's version with.

So the missing piece is a decoder per Encoding, the order the tables are
replayed in, and what the server does around it.

What upstream does

upstream/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs:

RestoreBackupAsync (line 88) — opens the zip, reads manifest.json,
refuses an archive made by a newer server version, then replays the
tables and the configuration.
ScheduleRestoreAndRestartServer (line 75) — a restore does not happen in
the request. The path is stashed on the application host, a restart is
notified, and the work runs on the way back up.

That second one is the shape question: upstream restores outside a running
server because the tables it is rewriting are the ones the server is reading.

Open

Where the restore runs. Ours has no equivalent of the restart-into-restore
hook, and doing it in-process means every open handle on the database is
wrong afterwards. The honest options are a restart hook of our own or a
documented restore-then-start mode of ephyra-server.
How it is verified. An archive is round-trippable by construction here —
write one from a fixture, restore into an empty state directory, and diff the
resulting database against the fixture it came from. That test needs no
upstream beside it, which makes this one of the few things in the tree whose
proof is not a diff. Worth deciding whether an upstream-written archive is
also replayed, since upstream's own restore reads ours already.

An archive is written, listed and its manifest read; restoring one is refused. ```rust // crates/ephyra-http/src/routes/admin/backup.rs:113 tracing::warn!(archive = %named, "restore requested; this server restores no archive"); Err(ApiError::Internal( "This server does not restore backups.".to_owned(), )) ``` The refusal is deliberate and is argued in `ENDPOINTS.md` — *the archives on disk, the manifest of one, and the writing of one upstream's own restore reads; nothing here restores, so that write refuses*. It is the one route in the group that answers `500` on a well-formed request, and the dashboard's restore button therefore fails on a server that made the archive itself. ## What exists already, and what it gives the restore for free The write half is complete and generated, which means the descriptor a restore needs is already in the tree: - `crates/ephyra-db/src/generated/backup.rs` — `TABLES`, one entry per table in the order upstream writes them, one `Field` per entity property with its `Encoding`. Generated by `cargo xtask gen-backup` from the pin; never edited. - `crates/ephyra-db/src/backup.rs` — turns a row into one JSON element under each of those descriptors. A restore is the same table walked the other way. - `crates/ephyra-http/src/routes/admin/backup.rs` — the four routes, including the manifest reader a restore would check the archive's version with. So the missing piece is a decoder per `Encoding`, the order the tables are replayed in, and what the server does around it. ## What upstream does `upstream/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs`: - `RestoreBackupAsync` (line 88) — opens the zip, reads `manifest.json`, **refuses an archive made by a newer server version**, then replays the tables and the configuration. - `ScheduleRestoreAndRestartServer` (line 75) — a restore does not happen in the request. The path is stashed on the application host, a restart is notified, and the work runs on the way back up. That second one is the shape question: upstream restores *outside* a running server because the tables it is rewriting are the ones the server is reading. ## Open - **Where the restore runs.** Ours has no equivalent of the restart-into-restore hook, and doing it in-process means every open handle on the database is wrong afterwards. The honest options are a restart hook of our own or a documented `restore-then-start` mode of `ephyra-server`. - **How it is verified.** An archive is round-trippable by construction here — write one from a fixture, restore into an empty state directory, and diff the resulting database against the fixture it came from. That test needs no upstream beside it, which makes this one of the few things in the tree whose proof is not a diff. Worth deciding whether an upstream-written archive is also replayed, since upstream's own restore reads ours already.

Claude added the

operations

labels

2026-09-12 08:50:57 +02:00

Claude commented

2026-09-12 12:11:18 +02:00

Author

Collaborator

Done in 72b953be, with the review's findings in 02bfac62.

The replay is crates/ephyra-db/src/backup.rs — the generated TABLES
descriptors walked the other way, one decode per Encoding inverting the
column beside it, over a streaming reader so a table larger than memory
replays. crates/ephyra-server/src/backup.rs does the guards, the tables and
then upstream's five tree copies.

Where it runs

The restart loop main.rs already had. Server::run drops the context, closes
the pool and answers Again::Restoring(path); main replays with nothing
holding the database and builds a fresh server on the result. That is upstream's
own shape — stash the path, restart, do the work on the way back up — without a
second entry point to document.

An archive upstream wrote replays too: the guards and the decode are written
against what upstream's serializer spells rather than against what ours does,
and a hand-built archive claiming 10.11.0.0 with lowercase guids and Z
stamps is one of the tests.

What the review found

Four of the six were real defects and one was the whole feature being untested.

Every test passed with the feature broken. Again::Restoring appeared in
three places and in no test: deleting the hook turned a restore into a plain
restart with the suite still green. Two tests now assemble a real server, do
what the route does, run it, and assert on what it hands back.
HomeSection was destroyed where upstream keeps it. A cascade action
still fires under defer_foreign_keys — deferral delays the check, not the
cascade — so every restore lost the home-screen sections. Upstream purges
under foreign_keys = OFF; so does this now, with a per-table key check at
the end to keep what deferral was giving.
A member the archive does not carry aborted the replay. It is now what a
deserializer would have left: default(T) where the column takes no null.
Since this port has no migrations, an older archive is the only kind the
version guard admits, and it is exactly the kind that has a column missing.
A malformed version replayed. "nonsense" parsed as [0,0,0,0] and
compared as older; it is refused, as System.Version refuses it.

What it is not

There is no durable marker for an interrupted restore, so a crash between the
tables committing and the tree copies finishing leaves a directory that the
next start will serve. Upstream has the same hazard — RestoreBackupPath is in
memory there too — so this matches rather than improves on it. Worth its own
issue if it should be better than upstream.

One deliberate cost: the closing key check is a scan per foreign key, so a
large library adds seconds to a restore. That buys back what upstream gets free
from EF's insert ordering.

Done in `72b953be`, with the review's findings in `02bfac62`. The replay is `crates/ephyra-db/src/backup.rs` — the generated `TABLES` descriptors walked the other way, one `decode` per `Encoding` inverting the `column` beside it, over a streaming reader so a table larger than memory replays. `crates/ephyra-server/src/backup.rs` does the guards, the tables and then upstream's five tree copies. ### Where it runs The restart loop `main.rs` already had. `Server::run` drops the context, closes the pool and answers `Again::Restoring(path)`; `main` replays with nothing holding the database and builds a fresh server on the result. That is upstream's own shape — stash the path, restart, do the work on the way back up — without a second entry point to document. An archive upstream wrote replays too: the guards and the decode are written against what upstream's serializer spells rather than against what ours does, and a hand-built archive claiming `10.11.0.0` with lowercase guids and `Z` stamps is one of the tests. ### What the review found Four of the six were real defects and one was the whole feature being untested. - **Every test passed with the feature broken.** `Again::Restoring` appeared in three places and in no test: deleting the hook turned a restore into a plain restart with the suite still green. Two tests now assemble a real server, do what the route does, run it, and assert on what it hands back. - **`HomeSection` was destroyed where upstream keeps it.** A cascade *action* still fires under `defer_foreign_keys` — deferral delays the check, not the cascade — so every restore lost the home-screen sections. Upstream purges under `foreign_keys = OFF`; so does this now, with a per-table key check at the end to keep what deferral was giving. - **A member the archive does not carry aborted the replay.** It is now what a deserializer would have left: `default(T)` where the column takes no null. Since this port has no migrations, an older archive is the only kind the version guard admits, and it is exactly the kind that has a column missing. - **A malformed version replayed.** `"nonsense"` parsed as `[0,0,0,0]` and compared as older; it is refused, as `System.Version` refuses it. ### What it is not There is no durable marker for an interrupted restore, so a crash between the tables committing and the tree copies finishing leaves a directory that the next start will serve. Upstream has the same hazard — `RestoreBackupPath` is in memory there too — so this matches rather than improves on it. Worth its own issue if it should be better than upstream. One deliberate cost: the closing key check is a scan per foreign key, so a large library adds seconds to a restore. That buys back what upstream gets free from EF's insert ordering.

Claude closed this issue

2026-09-12 12:11:18 +02:00