Also use `pretty_assertions` in some tests,
because I was having trouble debugging small differences in large structures.
Notably does not use `pretty_assertions` in our whole test suite,
only in the tests broken by the previous commits in this PR.
Prior to this commit, we pre-allocated 4096 values for each sequence during bootstrap.
For user sequences, these were 0..=4096; for system sequences, they were 4097..=8192.
This did not play nicely with restoring after a restart;
we would either incorrectly re-use values starting from 4097 after restart,
or would spuriously allocate after bootstrap without a restart
and begin with values starting from 8193.
With this commit, we do not pre-allocate sequence values during bootstrap.
Each user table sequence starts with `value: 1, allocation: 0`,
and each system sequence with `value: 4097, allocation: 4096`.
This means that the first access to a sequence, either after bootstrap or after a restart,
will perform an allocation.
This is in contrast to previously, where accesses after restart would allocate,
but accesses after bootstrap would not.
Additionally, the logic for determining whether an allocation was necessary
in `MutTxId::get_next_sequence_value` contained an off-by-one value
which caused the last value before an allocation to be skipped.
This commit fixes that off-by-one error, making it so that yielding value `4096`
when `allocation == 4096` is possible, though it does result in a new allocation.
Previously, we would yield 4095 without allocation, skip 4096,
then allocate and yield 4097.
* Add the `snapshot` crate, which implements snapshotting at a low level
- Requires making `BlobHash` be `Serialize` and `Deserialize`.
For arcane macro-ology reasons, this requires writing `BlobHash::SIZE`
instead of `Self::SIZE` (it gets embedded in a visitor struct or something).
- Requires adding two new operators to `BlobStore`.
- Adds a return value to `Page::save_content_hash`, for convenience.
- Impls `DerefMut` for `Pages`.
- **Scary change:** adds `Table::pages_mut`.
I think possibly this operator should be `unsafe`,
since write access to the `Pages` allows an undisciplined caller
to violate the `Table`'s assumptions by corrupting a `Page`.
It seems like an anti-pattern to mark a method `unsafe` on the grounds that
misusing its return value can cause UB,
but I don't see a plausible alternative
without making most methods on `Page` unsafe.
Open to feedback on this one!
* Nix `Table::pages_mut`
* Address Mazdak's feedback
* Use `thiserror` rather than `anyhow` for better error hygiene
* commitlog: Make bitflip test a proptest
The test sometimes fails. As a proptest, we'll be able to seed it with
failing inputs.
Fixes: #1167
* commitlog: Fix the bitflip test
Turns out we sometimes flipped a bit in the CRC32 itself, which makes
things go wrong in not the expected way.
* Make both TX offset counters agree
Prior to this commit, `CommittedState::next_tx_offset` got out of sync
with the commitlog/durability's notion of the tx offset,
because the former counted all committed TXes,
while the latter excluded certain empty TXes
(namely, empty TXes
which were not `__identity_connected__` or `__identity_disconnected__` reducers).
With this commit, the skipping logic is moved earlier into `CommittedState`,
so that it can maintain a counter consistent with that used by the commitlog.
* Remove duplicated increment
* Create new crate `fs-utils`; move `Lockfile` and `create_parent_dir`
The snapshot crate will need to create lockfiles.
Rather than duplicating code to do so, we choose to move our definition of `Lockfile`
into a crate that can be depended on by both `cli` and `snapshot`.
No existing crate seems like an obvious choice for this
-- a `Lockfile` is not really a data structure, so `data-structures` seems wrong --
so we add a new crate, `fs-utils`.
Currently this contains only `Lockfile` and `create_parent_dir`,
but a follow-up PR will add `DirTrie`, a Git-like on-disk object store.
* Deduplicate `map_err` closure
* Zeke's nit: simplify control flow
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
---------
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
* Impl `Serialize`, `Deserialize` for `Page`
Snapshotting needs to write `Page`s to files and read them back again.
To that effect, this commit implements `Serialize` and `Deserialize` for `Page`.
* Address Mazdak's review
- Fix soundness in `FixedBitSet` by moving an assert.
- Add commentary to test.
- Add commentary to `spacetimedb-lib` dependency.
Mario Alejandro Montoya Cortés