Read all five documents against the code and fix what the sixth review found
check / check (push) Successful in 12m5s
check / check (push) Successful in 12m5s
Fixes the four findings of the sixth review: `snapshot remove` with `--local-only`, the two comma-chained permission lists, six undefined terms, and the sentence about an unfinished snapshot in the local index. Then reads every sentence of `README.md`, `ARCHITECTURE.md`, `docs/DATAMODEL.md`, `docs/REPOSTRUCTURE.md` and the comments in `config.example.yml` against the code for four faults: a command's needs or behaviour stated without regard to a flag, a false absolute, three or more things chained with commas, and a technical word not defined on first use. Each sentence changed is listed on the pull request. The help string of `snapshot remove --dry-run` now says it needs no permission to delete. Model: fable-5-1
This commit is contained in:
1 parent
a133b8838e
commit
46b2a9fc78
6 files changed
+341
-193
No files matched your search
+36
-26
@@ -63,13 +63,13 @@ source files
|
||||
[`docs/DATAMODEL.md`](docs/DATAMODEL.md#code) says which type serves which
|
||||
table.
|
||||
- `internal/storage`: the `Storer` interface and its three implementations, for
|
||||
S3, a local directory and rclone.
|
||||
S3, a local directory and [rclone](https://rclone.org/).
|
||||
- `internal/s3`: the S3 client that the S3 `Storer` uses.
|
||||
- `internal/config`: loads and checks the config file.
|
||||
- `internal/ui`: formats the status lines, warnings and errors a user sees.
|
||||
- `internal/log`: the logger. It writes to stderr.
|
||||
- `internal/pidlock`: the lock that keeps two data-changing commands from
|
||||
running at once.
|
||||
- `internal/pidlock`: the lock that `RunWithApp` takes for a command that
|
||||
changes data, so that only one runs at a time.
|
||||
- `internal/globals`: the version, commit and commit date, set when the binary
|
||||
is built.
|
||||
- `internal/types`: distinct types for IDs, hashes and paths.
|
||||
@@ -114,8 +114,12 @@ A command runs through `cli.RunOperation`. It works like this:
|
||||
waits for the function to return, so that its temporary files are removed.
|
||||
- The wait ends after 30 seconds at most.
|
||||
|
||||
The `config` commands, `database delete`, `completion` and `version` do not
|
||||
build the application.
|
||||
These commands do not build the application:
|
||||
|
||||
- the `config` commands
|
||||
- `database delete`
|
||||
- `completion`
|
||||
- `version`
|
||||
|
||||
## Backup
|
||||
|
||||
@@ -163,7 +167,8 @@ A new `Scanner` is created for each snapshot, from `ScannerFactory`. It owns a
|
||||
- `repairInterruptedBlobs` deletes the records of blobs whose upload never
|
||||
finished. Their data is then stored again.
|
||||
- `loadDatabaseState` loads the known files and chunks into memory.
|
||||
- `scanPhase` walks the path. It sorts each entry into changed or unchanged.
|
||||
- `scanPhase` walks the path. It sorts each regular file and symlink into
|
||||
changed or unchanged.
|
||||
- `detectDeletedFiles` counts the files that the previous snapshot of this name
|
||||
had and that are now gone.
|
||||
- `batchAddFilesToSnapshot` records the unchanged files in the new snapshot.
|
||||
@@ -184,8 +189,8 @@ Data is stored once at two levels.
|
||||
|
||||
- **Files.** A file is not read again when its record in the index still matches
|
||||
it.
|
||||
- **Chunks.** A chunk is named by the SHA-256 hash of its content. A chunk whose
|
||||
hash is already in the index is not stored again.
|
||||
- **Chunks.** A chunk is named by the SHA-256 hash of its content. A chunk that
|
||||
an uploaded blob already holds, according to the index, is not stored again.
|
||||
|
||||
A file's record matches when all of these are unchanged:
|
||||
|
||||
@@ -208,8 +213,8 @@ on the content, so an insertion early in a file moves only nearby chunks.
|
||||
this size. Only a file's last chunk can be smaller than a quarter of it. With
|
||||
the default of 10 MB, those limits are 40 MB and 2.5 MB.
|
||||
|
||||
`ChunkReaderStreaming` passes each chunk to a callback as it is found. The whole
|
||||
file is never held in memory.
|
||||
`ChunkReaderStreaming` passes each chunk to a callback as it is found. The
|
||||
chunker buffers at most twice the largest chunk size.
|
||||
|
||||
### Blobs
|
||||
|
||||
@@ -217,8 +222,9 @@ file is never held in memory.
|
||||
|
||||
- It gives the blob a UUID and creates a temporary file.
|
||||
- It adds each chunk's bytes to a running SHA-256 hash.
|
||||
- In the same pass it compresses the bytes with zstd and encrypts them with
|
||||
age. The result goes to the temporary file.
|
||||
- In the same pass it compresses the bytes with
|
||||
[zstd](https://github.com/facebook/zstd) and encrypts them with
|
||||
[age](https://age-encryption.org/). The result goes to the temporary file.
|
||||
- It records each chunk's offset and length within the blob.
|
||||
- It closes the blob when the next chunk would pass `blob_size_limit`, and at
|
||||
the end of each path.
|
||||
@@ -250,8 +256,8 @@ chunks is uploaded.
|
||||
- `MarkSnapshotComplete` marks the snapshot complete.
|
||||
|
||||
Completion is recorded last on purpose. A backup that stops during the upload
|
||||
leaves the snapshot unfinished, and the next backup removes it and stores its
|
||||
data again.
|
||||
leaves the snapshot unfinished, and the next backup removes it and backs up its
|
||||
files again.
|
||||
|
||||
`ExportSnapshotMetadata` works on a copy of the index, never on the index
|
||||
itself:
|
||||
@@ -282,11 +288,13 @@ therefore marks a backup that did not finish.
|
||||
temporary directory.
|
||||
- `getFilesToRestore` selects the files, from the paths given.
|
||||
- `restoreAllFiles` downloads blobs and writes the files.
|
||||
- With `--verify`, `verifyRestoredFiles` reads each restored file and checks its
|
||||
chunk hashes.
|
||||
- With `--verify`, `verifyRestoredFiles` reads each restored regular file and
|
||||
checks its chunk hashes.
|
||||
|
||||
The restore does not read the local index or the manifest. It finds each file's
|
||||
chunks and blobs through the downloaded database.
|
||||
Given a snapshot ID, the restore does not read the local index. Given a remote
|
||||
key, it reads the local index only to check the destination marker. It does not
|
||||
read the manifest. It finds each file's chunks and blobs through the downloaded
|
||||
database.
|
||||
|
||||
### Checks on the downloaded database
|
||||
|
||||
@@ -313,12 +321,13 @@ database is honest. The restore therefore checks it:
|
||||
- `restoreSweeper` deletes a cached blob once no remaining file needs it.
|
||||
|
||||
A regular file is created with mode `0600` and gets its stored mode after its
|
||||
content is written. A directory gets its stored mode, owner and modification
|
||||
time after the whole restore.
|
||||
content is written. A directory gets its stored mode and modification time after
|
||||
the whole restore, and its owner when the command runs as root.
|
||||
|
||||
## Verify
|
||||
|
||||
`Vaultik.VerifySnapshotWithOptions` checks a snapshot without the private key:
|
||||
Without `--deep`, `Vaultik.VerifySnapshotWithOptions` checks a snapshot without
|
||||
the private key:
|
||||
|
||||
- It downloads the manifest.
|
||||
- It checks that the snapshot's encrypted database is present.
|
||||
@@ -337,7 +346,7 @@ time after the whole restore.
|
||||
|
||||
## Prune
|
||||
|
||||
`Vaultik.Prune` does three steps:
|
||||
`Vaultik.Prune` calls `EnsureStorageBinding` and then does three steps:
|
||||
|
||||
- `CleanupLocalSnapshots` drops from the local index each snapshot whose
|
||||
manifest is gone from the destination.
|
||||
@@ -382,17 +391,18 @@ With no `storage_url`, it builds the S3 implementation from the `s3` settings.
|
||||
Two checks keep the local index and the destination in step.
|
||||
|
||||
- `checkDestinationMarker` looks for `vaultik-destination.txt`. It fails when
|
||||
the file is reported missing while the local index holds snapshots.
|
||||
the file is reported missing while the local index holds snapshots. It skips
|
||||
the check when the local index belongs to another destination.
|
||||
- `EnsureStorageBinding` records the configured destination in the local index
|
||||
on first use. Later it fails when the config names a different destination.
|
||||
|
||||
Every command that changes data calls `EnsureStorageBinding` first.
|
||||
Every command that takes the lock calls `EnsureStorageBinding` first.
|
||||
|
||||
## Concurrency
|
||||
|
||||
- Each open database uses one connection, so its queries run one at a time.
|
||||
- The index runs in SQLite's WAL mode. A second process can therefore read it
|
||||
while a backup writes to it.
|
||||
- The index runs in SQLite's write-ahead log (WAL) mode. A second process can
|
||||
therefore read it while a backup writes to it.
|
||||
- `Packer` guards its state with a mutex.
|
||||
- `Scanner` holds `packerMu` while a chunk is added to the packer and while a
|
||||
blob is closed.
|
||||
|
||||
Reference in new issue
Block a user