Locally, I witnessed very slow tests. Profiling with Visual VM
revealed a lot of time spent in `wait`.
Digging in, we were trying to be clever, with a `wait(1000)/notify`
mechanism. However, there were never multiple threads in play, so the
waiter wasn't waiting when `notify` was invoked. That means we always
timed out. I think this never worked and using bare `wait()` would
have revealed that.
Anyway, `CountDownLatch` maintains the one bit of state (was I
notified) and generalizes smoothly to when we have threads.
Being able to derive partition map from partition definitions and current
state of the world (transactions), segmented by timelines, is useful
because it lets us not worry about keeping materialized partition maps
up-to-date - since there's no need for materialized partition maps at that point.
This comes in very handy when we start moving chunks of transactions off of our mainline.
Alternative to this work would look like materializing partition maps per timeline,
growing support for incremental "backwards update" of the materialized maps, etc.
Our core partitions are defined in 'known_parts' table during bootstrap,
and what used to be 'parts' table is a generated view that operates over
transactions to figure out partition index.
'parts' is defined for the main timeline. Querying parts for other timelines
or for particular timeline+tx combinations will look similar.
Normally we want to both materialize our changes (into 'datoms')
as well as commit source transactions into 'transactions' table.
However, when moving transactions from timeline to timeline
we don't want to persist artifacts (rewind assertions), just their
materializations.
This patch expands the 'db' interface to allow for this split,
and changes transactor's functions to take a crate-private 'action'
which defines desired behaviour.
This is necessary for the timelines work ahead. When schema is being
moved off of a main timeline, we need to be able to retract it cleanly.
Retractions are only processed if the whole defining attribute set
is being retracted at once (:db/ident, :db/valueType, :db/cardinality).
Help folks debugging by including symbols in our native libraries.
Yes, this makes the resulting AAR very large. The Android ecosystem
seems to be in flux around who is in charge of stripping native
binaries, but for now let's provide symbols and see how consumers
react.
Timelines work starts to perform modifications on the partitions
that go beyond simple allocations. This change pre-emptively protects
partition integrity by asserting that index modifications are legal.
Generally, I think that Mentat is using too many small traits rather
than wrapping types into newtypes. Wrapping into newtypes is cheap in
Rust, and it makes it easier to reason about the code.
* Part 1: Extract low-level test framework into mentat_db::debug for re-use.
* Part 2: Improve assert_matches!.
This corrects an incorrect pattern: a conversion method taking &self
but returning an owned value should be named like `to_FOO(&self) -> FOO`. (A
reference-to-reference conversion should be named like `as_FOO(&self)
-> &FOO`. A consuming conversion should be named like `into_FOO(self)
-> FOO`.)
In addition, this pushes the conversion via `to_edn` into the
`assert_matches!` macro, which lets consumers get a real data
structure (say, `Datoms`) and use it directly before or after
`assert_matches!`. (Currently, consumers get back `edn::Value`
instances, which aren't nearly as pleasant to use as real data
structures.)
Co-authored-by: Grisha Kruglov <gkruglov@mozilla.com>
* Part 3: Use mentat_db::debug framework in Tolstoy crate.
The advantage of this approach is that compiling Tolstoy (or anything
that's not db, really) can be quite a bit faster than compiling db.
* Add a top-level "syncable" feature.
Tested with:
cargo test --all
cargo test --all --no-default-features
cargo build --manifest-path tools/cli/Cargo.toml --no-default-features
cargo run --manifest-path tools/cli/Cargo.toml --no-default-features debugcli
Co-authored-by: Nick Alexander <nalexander@mozilla.com>
* Add 'syncable' feature to 'db' crate to conditionally derive serialization for Partition*
This is leading up to syncing with partition support.
These build on #778, and implement a variety of small fixes (related
parts are labelled as such), and one non-trivial part -- matching
tuple results with the `BindingTuple` trait. In practice, this is very
helpful, and greatly streamlined the logins API.
Right now, we write code like
```rust
match q_once(q, inputs)?.into_tuple()? {
Some(vs) => match (vs.len(), vs.get(0), vs.get(1)) {
(2, &Some(Binding::Scalar(TypedValue::Long(a))), &Some(Binding::Scalar(TypedValue::Instant(ref b)))) => Some((a, b.clone())),
_ => panic!(),
},
None => None,
}
```
to length-check tuples coming out of the database. It can also lead
to a lot of cloning because references are the easiest thing to hand.
This commit allows to write code like
```rust
match q_once(q, inputs)?.into_tuple()? {
Some((Binding::Scalar(TypedValue::Long(a)), Binding::Scalar(TypedValue::Instant(b)))) => Some((a, b)),
Some(_) => panic!(),
None => None,
}
```
which is generally much easier to reason about.
Perhaps we actually want to subdivide the top-level namespace so that
there is a `mentat::time` module, but I'd prefer to make part of the
process of fixing the public API as we get ready to christen version
1.0.
I think this is just oversight. Generally, we should anticipate what
our consumers need to do to interact with Mentat, and producing milli-
and micro-second timestamps is part of that need.
These are functions on `TermBuilder` itself to prevent mixing mutable
and immutable references in the most natural style. That is,
```
builder.add(e, a, builder.lookup_ref(...))
```
fails because `add` borrows `builder` mutably and `lookup_ref` borrows
`builder` immutably. There's nothing here that requires a specific
builder (since we're not interning lookup refs on the builder, like we
are tempids) so we don't need an instance.
There are a few tricky details to call out here. The first is the
`TransactableValueMarker` trait. This is strictly a marker (like
`Sized`, for example) to give some control over what types can be used
as value types in `Entity` instances. This expression is needed due
to the network of `Into` and `From` relations between the parts of
valid `Entity` instances. This allows to drop the `IntoThing`
work-around trait and use the established patterns. (Observe that
`KnownEntid` makes this a little harder, due to the cross-crate
consistency restrictions.)
The second is that we can get rid `{add,retract}_kw`, since the
network of relations expresses the coercions directly.
The third is that this commit doesn't change the name `TermBuilder`,
even though it is now building `Entity` instances. This is because
there's _already_ an `EntityBuilder` which fixes the `EntityPlace`.
It's not clear whether the existing entity building interface should
be removed or whether both should be renamed. That can be follow-up.
This is all part of moving the entity builder away from building term
instances and toward building entity instances. One of the nice
things that the existing term interface does is allow consumers to use
lightweight reference counted tempid handles; I don't want to lose
that, so we'll build it into the entity data structures directly.
