Make it so `HostController` manages both the module host (wasm
machinery) and the database (`RelationalDB` / `DatabaseInstanceContext`)
of spacetime databases deployed to a server.
The `DatabaseInstanceContextController` (DBIC) is removed in the
process.
This allows to make database accesses panic-safe, in that uncaught
panics will cause all resouces to be released and the database to be
restarted on subsequent access. This is a prerequisite for #985.
It also allows to move towards storage of the module binary directly in
the database / commitlog. This patch, however, makes some contortions in
order to **not** introduce a breaking change just yet.
* Create a lockfile when opening config files
In the past, we've had issues where multiple concurrent CLI processes
would race to read and write the CLI config file,
leading to data loss.
We considered using `flock`/`LockFileEx` and blocking until the file became available,
but unfortunately it's not possible to atomically create and lock a nonexistent file,
which we need to do in the case where the configuration doesn't yet exist.
Instead, we opt for a classic lockfile-based scheme:
Before opening a config file `foo.conf`, attempt to exclusively create `foo.lock`,
and panic if the exclusive creation fails.
Once it becomes clear that we will not write the config any more,
i.e. in `Config::drop`,
delete the lockfile, allowing another process to operate.
This means that attempting to run multiple concurrent Spacetime CLI processes
with the same config file is now a hard error.
* Fix CI failures
This commit fixes two CI failures:
- `spacetime start`, and a few other CLI subcommands, do not access their `Config` at all,
but the CLI constructs it unconditionally in `main`,
which made it an error to run any CLI command while `spacetime start` was running.
This is fixed by having subcommands which don't need a `Config`
drop it before doing anything.
- Contrary to my assumption,
the test configuration created by `Config::new_with_localhost` does get `drop`ped,
because the test harness `clone`s is and passes an owned version to the CLI.
This was causing it to attempt to delete the empty path, which failed.
This is fixed by having the home configuration be `Option`al,
and setting it to `None` in tests.
* Clap before config because they suppress destructors
Perform Clap argument parsing as the very first thing in a CLI process,
before locking the config,
because Clap calls `exit` directly on error rather than panicing
(presumably to have more control over error output),
which prevents destructors from running,
leaving stale lockfiles.
* Encapsulate lockfile logic in a type
Also deduplicate logic for finding config file paths.
* Define `create_parent_dir` helper with comments
* Replace `drop` calls with more explicit `Config::release_lock`.
This patch attempts to integrate the new commitlog with the minimum
changes.
Most of the diff comes from deletions of the legacy log and the need to
adjust tests due to the requirement for a tokio runtime when a durable
database is used in tests.
The "meat" of the patch are the `RelationalDB` constructors,
`RelationalDB::commit_tx`, and the replay logic in
`locking_tx_datastore`.
While `DataKey` is gone, there is still some redundant data being passed
around, which will be addressed in the follow-up patch.
This error turns out to have been a legitimate, albeit rare, race condition,
which could have (but apparently didn't) manifested in user code.
The cause, ultimately, was holding locks for too-short scopes
while initializing the connection.
It was possible for a new connection to receive and completely process
its `IdentityToken` message before calling `maybe_set_credentials`,
which would then clobber the received identity.
This is now fixed (hopefully) by holding the `CredentialStore` lock
for the duration of `BackgroundDbConnection::connect`.
Two other locks, on `ClientCache` and `ReducerCallbacks`,
are also newly held for the duration to avoid similar issues.
In addition, this commit adds a call to `disconnect` to the beginning of `connect`.
This will prevent pathological user code
which connects twice in a row without an intervening disconnect
from encountering bizarre errors
caused by the first connection's background workers staying active.
This commit also bumps the `RUST_LOG` env variable for the SDK test clients to `trace`,
and adds several trace-level logs to the SDK,
which were instrumental in pinning down the failure.
This does mean that log output of failed SDK tests will now be much noisier.
We've had continual issues with test isolation when developing breaking changes.
This commit doesn't fully address those, but is a step in the right direction:
the SDK tests now create a tempdir as their `STDB_PATH`,
rather than using `~/.spacetime`.
* Add `address: Address` to `ReducerContext`
Initial support for identifying connections via an `Address`,
passed as the caller address in `ReducerContext`.
Notable design choices:
- The same type `Address` is used for clients and dbs,
as opposed to having distinct `DbAddress` and `ClientAddress` types.
- For automatic reducers (init, scheduled, &c), the passed `Address`
is the database's address, not the address of the client which called `publish`.
- Clients may specify an `Address` as a query parameter
to the `subscribe` and `call` endpoints.
If they do not, an `Address` will be randomly generated for them.
Still to do:
- Send the client's `Address` alongside their `Identity` and token
upon WebSocket init in `IdentityToken`,
so the client can save its `Address` for re-connection.
- SDK support.
- C# module support.
- Documentation.
- Refuse new connections if an existing connection
with the same `(Identity, Address)` pair exists.
- Ensure we can store `Address` in tables,
so modules can track connected clients,
and ser/de `Address`, so those tables can be synced to clients.
* Use `None` in `ReducerContext` for HTTP calls without client address
* Fix typo in trait argument name
* `Address` representation amenable to SDK usage
Similar to `Identity`, make `Address` a product type with a double-underscored field name.
This will allow SDKs to distinguish `Address` from `Vec<u8>`,
but does necessitate some ugly `AddressForUrl` hackery
to get ser/de working correctly in different contexts.
This commit does not yet include SDK support for `Address`,
only the server-side machinery necessary for it.
* Add client_address parameter to /publish; pass to init and update reducers
Per discussion with Kim, it's useful for SpacetimeDB-cloud
for the client_address passed to the init reducer
to be that of the caller of /database/publish,
rather than the database's address.
This commit implements that behavior. Now:
- `init` and `update` take the client_address passed to publish, if any,
or `None` if no client_address was supplied.
- Scheduled reducers never receive a client_address.
- Reducers invoked by HTTP /call take the client_address passed to /call, if any,
or `None` if no client_address was supplied.
- Reducers invoked by WebSocket always receive the address of the WebSocket connection.
* `Address` support in Rust client SDK
* Add test that addresses are stable within a client process
* Run rustfmt against merge changes
* Not sure why my local `cargo fmt` didn't get this...
* Add caller_address argument to reducer arguments in Rust SDK
* rustfmt again...
* Add caller address to `EventJson` message
* Python codegen for client addresses
* C# module support for client addresses
* Fix scoping error
* Add `Address`-related stuff to C# codegen
- Emit `SpacetimeDB.Address` for address types
- Add `callerAddress` field to `ReducerEvent`
* TypeScript codegen changes
* Fix merge conflict with new test
* Run rustfmt
---------
Co-authored-by: Ingvar Stepanyan <me@rreverser.com>
* Client API changes to accomodate new cloud architecture
The exact trait interfaces for `client-api` is TBD
* Ensure we're not blocking when accessing the filesystem
* Derive Clone for SendGridController
* Add YOLO error variant to InsertDomainResult
* Rebase
* fixup! Rebase
* Fix SpacetimeType for Address
* Temporarily disable message / frame size limits for SDK WS
* Remove get_database_instance_state from API trait
It's an internal (worker db) thing, which does not need to be satisfied
by impls.
* Update indexes when updating a database
It turns out that the order of the index definitions in the proposed
schema may differ from those returned from the catalog, causing valid
(i.e. no-op) updates to be rejected. While at it, allow updating table
indexes so as long as the (column) schema remains unchanged.
* Update indexes when updating a database
It turns out that the order of the index definitions in the proposed
schema may differ from those returned from the catalog, causing valid
(i.e. no-op) updates to be rejected. While at it, allow updating table
indexes so as long as the (column) schema remains unchanged.
* Fix -S instead of -s for update-module smoke test
-s now means "server", -S "skip clippy", changed in:
a1e9984 (Multiple server configurations for CLI (#214), 2023-09-01)
* Invalidate schema cache when committing a tx
* Use long options in update-module.sh, fix unused warning
* Add test asserting schema_for_table reflects index updates
This switches tests from invoking a spacetimedb CLI found on PATH to always using up-to-date version by depending on the CLI crate as a library.
---------
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
* Simple SDK test harness; SDK test module
Yet to come: actual SDK tests.
* Quiet clippy lint about "too many arguments"
* Lints are named with underscores, not dashes...
* Will this make clippy shut up?
* Go nuclear on disabling `too_many_attributes` lint. Sigh.
* WIP SDK test client, and fix bugs in Rust codegen
Compiling the module_bindings for the sdk-test module revealed two bugs:
- Enums holding structs generated incorrectly,
unpacking the struct into the enum's payload.
- Recursive types would cause the codegen to attempt to recursively import
the current module into itself.
* One (1) actual runnable test in the sdk crate
* Exclude test-client from CI
The sdk tests already build this crate (though they don't clippy or fmt it).
Attempting to build, test, fmt or clippy it as-is will fail
because the module_bindings are not committed.
This is intentional, as the SDK test suite wants to generate the module_bindings
during its run.
* Rustfmt ignore generated module_bindings
It turns out `cargo fmt` doesn't actually support the `--exclude` option
the way `cargo clippy` does.
Instead, just `#[rustfmt::skip]` the `mod module_bindings;` decl.
* Actually commit test file...
God, I'm so bad at remembering to commit new files.
Anyway, add a test for deleting rows with primitive unique fields.
* Make CLI tool available in tests workflow
The SDK tests need to run `spacetime start`, `spacetime generate` and `spacetime publish`.
* Test update events with primitive pk types; split test-client into files
* Tests with `Identity` fields in tables
* Tests for reducer callbacks, both successful and failing
* Tests with vecs of stuff, with structs, with enums
* Test that should fail, test that uses a large table with many columns
* Test for resubscribe functionality
* Test of reauth; fix major bug in `TestCounter`
I misread `Condvar::wait_timeout_while` as `Condvar::wait_timeout_until`,
and flipped my predicate.
This led to false negatives (i.e. tests that passed that shouldn't have).
* A fistful of doc comments
* Avoid race condition running multiple tests with same client project
This commit fixes a race condition which sometimes caused the SDK tests to fail
because multiple `spacetime generate` processes running concurrently
would clobber each others' output,
potentially deleting it while a `cargo build` or `cargo run` process was running.
Now, the test harness will only run `spacetime generate` at most once
for any given directory.
* Add env_logger to SDK test client
* RUST_LOG=trace when running test clients
* quieter logs in test client: only warn-level and higher
* Install from source instructions
* Improved comment
* Separate the global location of files per OS & create a configuration trait to read them
---------
Co-authored-by: Boppy <no-reply@boppygames.gg>
Noa