* 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
* 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.
* Make `Page` always fully init
Per discussion on the snapshotting proposal,
this PR changes the type of `Page.row_data` to `[u8; _]`,
where previously it was `[MaybeUninit<u8>; _]`.
This turns out to be shockingly easy,
as our serialization codepaths never write padding bytes into a page.
The only place pages ever became `poison` was the initial allocation;
changing this to `alloc_zeroed` causes the `row_data` to always be valid at `[u8; _]`.
The majority of this diff is replacing `MaybeUninit`-specific operators
with their initialized equivalents,
and updating comments and documentation to reflect the new requirements.
This change also revealed a bug in the benchmarks
introduced when we swapped the order of sum tags and payloads
( https://github.com/clockworklabs/SpacetimeDB/pull/1063 ),
where benchmarks used a hardcoded offset for the tag which had not been updated.
* Update blake3
Blake3 only supports running under Miri as of 1.15.1, the latest version.
Prior versions hard-depended on SIMD intrinsics which Miri doesn't support.
* Address Mazdak's review.
Still pending his agreeing with me that `poison` is a better name than `uninit`.
* "Poison" -> "uninit"
Against my best wishes, for consistency with the broader Rust community's poor choices.
* Remove unnecessary `unsafe` blocks
* More unnecessary `unsafe`; remove forgotten SAFETY comments
2. Make `RowRef::row_hash` use the above.
3. Make `Table::insert` return a `RowRef`.
4. Use less unsafe because of 1-3.
5. Use `second-stack` to reuse temporary allocations in hashing and serialization.
- Arcing `TableSchema`, and this has benefits elsewhere too.
- Arc<[_]>ing the visitor program instructions.
The data behind the Arcs very rarely change,
which is the perfect case for an Arc.
* Swap the location of tags to go before variant data in the BFLATN encoding
* Fix a comment
* Apply suggestions from code review (@gefjon @Centril)
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
Signed-off-by: james gilles <jameshgilles@gmail.com>
* Implement memcpy consolidation for sums
* Vanquish clippy
---------
Signed-off-by: james gilles <jameshgilles@gmail.com>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
* Implement (but do not use) a fast path for BFLATN -> BSATN conversion
* fmt and clippy
* `u16` offset rather than `usize`
* Address Joshua's review
* Define methods on `RowRef` and `RelValue` which use the new serializer
* Comment in `align_to` about div-by-zero
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
* Add benchmark comparing BFLATN -> BSATN with and without the fast path
* Add benchmark on `u64_u64_u32`, which has less interior padding than `u32_u64_u64`
* Remove `to_len` from `to_bsatn_extend`
It turns out to be slower than just eating the `realloc`s.
* Remove unused `to_bsatn_slice`
I thought I would need it, but it ended up not being useful.
* Expand comment with example; `Box<[...]>` to reduce memory footprint
* Comments from Mazdak's review
---------
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
`AlgebraicTypeLayout` and friends already include full layout information,
including properly-aligned offsets for `ProductTypeElementLayout`s.
As such, there's no need to do any alignment computation
during `serialize_value` or `write_value`.
Instead, while traversing a `ProductTypeLayout`,
we can use each element's `offset` to update the `curr_offset`.
- Move it and friends from sats to vm.
- MemTable now stores a Vec<PV>.
- Other related improvements.
Co-authored-by: Phoebe Goldman <phoebe@goldman-tribe.org>
* optimize build_missing_tables; collect less + use read_col
* schema_for_table: don't go through PV
* table_id_from_name: use read_col
* relational_db tests: remove uses of to_product_value
* build_sequence_state: don't use to_pv
* build_indexes: don't use to_pv
* remove dead code: CommittedStateIter
* get_all_tables_tx: use read_col
* SystemTableQuery: don't use to_pv
* StModuleRow: don't use to_pv
* get rd of more to_product_value calls
* read_col / system_tables / mut_tx: cleanup + less work2
* This fixes replaying of the transaction log to no longer check constraints
* Fixed based on Phoebe and Kim's comments
* Cargo fmt
* Cargo fmt
* This fixes index updating for replaying deletes
---------
Signed-off-by: Tyler Cloutier <cloutiertyler@users.noreply.github.com>
* BTreeIndex: Specialize on primitive key types for great performance
Rewrite of Tyler's #771, because I thought this would be easier than rebasing.
This commit hoists branching on key types outside of comparisons and searches
in `BTreeIndex`,
so that in many cases we can use e.g. `u64::cmp`
instead of the much slower `AlgebraicValue::cmp`.
This design is kind of ugly, and will likely change in the future,
but for now it's good enough, and is a meaningful performance improvement.
* BTreeIndex: use `ReadColumn` instead of `ProductValue` fields
Prior to this commit, `BTreeIndex::insert` and `BTreeIndex::delete`
took a `&ProductValue`, the row to be inserted or deleted, as an argument,
and extracted the indexed column value(s) from it.
With this commit, we instead take a `RowRef`
referring to the row to be inserted or deleted,
and access the indexed column value(s) using `ReadColumn`.
For specialized indexes, we extract them directly as native types,
rather than as `AlgebraicValue`.
Making this change involved a nasty split borrow problem,
as we now need to have a `RowRef` to the inserted/deleted row
coexisting with a mutable borrow of the index to be modified.
This required introducing a `TableInner` struct,
which is the referent of `RowRef`.
* Lints: superfluous borrows
Rewrite of Tyler's #771, because I thought this would be easier than rebasing.
This commit hoists branching on key types outside of comparisons and searches
in `BTreeIndex`,
so that in many cases we can use e.g. `u64::cmp`
instead of the much slower `AlgebraicValue::cmp`.
This design is kind of ugly, and will likely change in the future,
but for now it's good enough, and is a meaningful performance improvement.
This commit defines `trait ReadColumn`,
which is implemented for types that can be stored in a table column
and can be read out of said table column.
Its interesting method is
`read_column(row: RowRef<'_>, idx: usize) -> Result<Self, TypeError>`,
which attempts to read the `idx`th column of `row` as a value of type `Self`,
returning a `TypeError` if the row in question does not have the appropriate types.
`ReadColumn` is implemented for Rust equivalents of all non-compound `AlgebraicType`s,
i.e. integers, floats, `bool` and `String`.
It is also implemented for all SATS value types,
including those which represent values of compound types,
i.e. `AlgebraicValue`, `ProductValue`, `SumValue`, `ArrayValue` and `MapValue`.
* datastore: impl mem arch using spacetimedb_table
* instance_env/iters: write bflatn -> bsatn directly
* Make tests build, fix 1 failure (of 4)
Commit message from SpacetimeDBPrivate 0ae26519d181cb4b13f748af53d036c322d4f642:
Bug fix in `delete_by_rel`: don't trigger unique constraints in dummy insert
Prior to this commit, `MutTxId::delete_by_rel` on a table with a unique constraint
would fail, because the `delete_by_rel` would attempt an insert of the row to be deleted,
with the intention of rolling back the insertion later.
This would trigger the unique constraint, aborting the `delete_by_rel`,
and the actual row would not be deleted.
With this commit, `MutTxId::delete_by_rel` properly uses
`Table::insert_internal_allow_duplicate` and `Table::delete_internal_skip_pointer_map`
to bypass unique constraints.
This requires exporting some methods from `Table` which were previously internal.
* Remove forgotten `dbg`
* `bflatn_to`: panic rather than UB on an ill-typed `ProductValue`
The docs in `bflatn_to.rs` claim that we'll panic
if the `ProductValue` passed to `write_row_to_pages`
does not conform to the type.
Prior to this commit, we would panic if some element of the `val`
had a different type constructor than the corresponding element of the `ty`,
but for `ProductValue`, we would not panic if the value had more or fewer elements
than the type.
When the value had more elements, this was merely a logic bug;
we'd drop the tail elements and go about our lives.
When the value had fewer elements, this invoked UB;
we'd leave the remainder of the row in the page uninit,
and later access uninitialized memory e.g. when hashing the row.
This commit adds a check that in `write_product`
that the `val` and `ty` have the same length.
If they do not, this commit panics.
This will need to be revised to return an `Err`, as spacetimedb-core has tests
that attempting to insert an ill-typed value returns an `Err` rather than panicking.
* Error handling, runtime schema mutations
This commit makes two sorts of changes:
- It begins filling in the error-handling story of the new mem-arch
by defining a few new error enums in the `spacetimedb-table` crate,
and converting them into `TableError` in `spacetimedb-core`.
- It implements `drop_table`, `rename_table` and friends to mutate a schema at runtime.
These operations are broken in master in that they violate transactionality,
and they're broken here too.
* Re-enable metrics
* Resolve some TODOs, incl. error type in `page.rs`
* Fixes for Tyler's comments
- Restrict some `pub` to `pub(crate)`
- Rename `MutTxId::delete_by_rel` to `delete_by_row_value`, since it deletes only one row.
- `relations` -> `relation`.
- Rename `ignore_duplicate_insert` -> `ignore_duplicate_insert_error` and add docs.
- nix `with_table_*` methods in favor of imperative `get_table_*`.
* Note correctness of `state_view::Iter` and add test in `relational_db`.
Tyler had concerns about the behavior of `state_view::Iter`
w.r.t. the edge case of a committed-deleted-inserted row,
since the new datastore maintains the invariant that
the committed and inserted tables are disjoint,
where the previous datastore did not.
This commit adds comments describing that invariant to the iterator,
and adds a test in `relational_db` that the external semantics are preserved.
* Add notes for future MVCC implementors re: disjoint committed and insert tables
Tyler convinced me via video call that, in MVCC, it is no longer valid
to treat the committed and insert tables as disjoint,
i.e. to elide inserts of already-present rows.
Doing so is still valid, and is more performant, in the current locking model,
so we retain that behavior here.
This commit adds comments to relevant code paths so that, when we move to MVCC,
we do not forget to remove the optimization.
---------
Co-authored-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Phoebe Goldman