# Description of Changes
Adds a Prometheus counter for websocket clients that are disconnected
after the server considers them idle:
-
`spacetime_worker_ws_clients_idle_timed_out_total{database_identity=...}`
The counter is incremented at the existing websocket idle-timeout
disconnect path, next to the `Client ... timed out` log.
# API and ABI breaking changes
None.
# Expected complexity level and risk
1. This is instrumentation-only and does not change websocket behavior.
# Testing
- [x] `rustup run 1.93.0 rustfmt
crates/client-api/src/routes/subscribe.rs
crates/core/src/worker_metrics/mod.rs`
- [x] `git diff --check`
- [x] `LC_ALL=en_US.UTF-8 LC_CTYPE=en_US.UTF-8 LANG=en_US.UTF-8
RUSTC=/Users/clockworklabs/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc
rustup run 1.93.0 cargo check -p spacetimedb-client-api -p
spacetimedb-core`
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
Co-authored-by: Tyler Cloutier <cloutiertyler@users.noreply.github.com>
# Description of Changes
#5300 changed the backing table schema of views. While this technically
isn't a breaking change because these tables are ephemeral, the schema
is not ephemeral, so it does require us to update the schema by dropping
and re-adding views once on initial startup.
# API and ABI breaking changes
None
# Expected complexity level and risk
1.5
# Testing
Auto-migration unit tests
# Description of Changes
Module launch now exposes a completion handle so callers can wait until
`st_module` is durable before cleaning up controldb bootstrap state.
# API and ABI breaking changes
None
# Expected complexity level and risk
2
# Testing
Tests added in the corresponding private PR that deletes bootstrap state
from the controldb
# Description of Changes
Uses `jsonwebtoken v10.4.0` instead. Important changes include:
**1. Token serialization**
Old tokens with `"exp": null` are still accepted, but new no-expiry
tokens now omit `exp` instead of serializing it as `"exp": null`.
**2. OIDC/JWKS validation**
Issuer extraction now uses `jsonwebtoken::dangerous::insecure_decode`
for key discovery only, not validation. And the old `spacetimedb-jwks`
crate required every JWK to have a `kid`, but this patch does not
preserve that limitation.
# API and ABI breaking changes
I don't believe this is considered breaking, but it bears repeating that
new no-expiry tokens now serialize without `exp` instead of `"exp":
null`.
# Expected complexity level and risk
2
# Testing
- [x] Verify a legacy no-expiry token serialized as `"exp": null` still
validates.
- [x] Verify a token with an expired `exp` is still rejected.
- [x] Verify OIDC/JWKS validation works when the JWKS keys omit the
optional `kid` field.
# Description of Changes
In order to be able to suspend databases based on memory usage.
Reports memory usage for wasm linear memory as well as the v8 heap. Also
reports page-level memory for tables.
It does not report memory usage for indexes.
# API and ABI breaking changes
None
# Expected complexity level and risk
1.5
# Testing
Testing added in the private counterpart which handles enforcement of
memory limits.
# Description of Changes
Adds a Prometheus counter for client disconnects caused by the outgoing
WebSocket message queue reaching capacity.
The new metric is
`spacetime_client_outgoing_queue_disconnects_total{db=...}`. It
increments only on the `TrySendError::Full` path that kicks a client
after its bounded outgoing queue fills.
# API and ABI breaking changes
None.
# Expected complexity level and risk
1. This is a narrow observability change: one metric definition and one
increment at the existing kick site.
# Testing
- [x] `cargo fmt --all`
- [x] `LC_ALL=en_US.UTF-8 LC_CTYPE=en_US.UTF-8 LANG=en_US.UTF-8 cargo
check -p spacetimedb-core`
- [x] `git diff --check`
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
# Description of Changes
Moves `RelationalDB` and related database code into a new
`spacetimedb-engine` crate.
The main motivation is to tighten dependency control around the engine
layer and isolate `RelationalDB`
behind a crate boundary.
- Majority of this PR is code-motion.
- Removes direct production dependence on `tokio` from
`spacetimedb-engine`.
- Keeps `tokio` only as a dev-dependency for test-only code in
`spacetimedb-engine`.
- This is intended to be a structural refactor only and should not
result in any functional change in
production.
- Adds a CI check to ensure `spacetimedb-engine` continues to compile in
simulation mode
# API and ABI breaking changes
NA
# Expected complexity level and risk
1.5.
# Testing
Existing tests should be enough.
---------
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
# Description of Changes
Review #5287 first.
This patch updates view backing table schemas to have a single private
column `arg_hash`. Previously sender-scoped views had a `sender` column
for the calling identity, however now that's been replaced with a single
unified `arg_hash` column that encodes the calling identity within it.
When we add parameterized views, the view args will also be encoded in
this hash and stored in this column.
This column exists for both anonymous and sender scoped views meaning
that the backing tables for all views now have the same number of
private columns - one.
This hash is now used as a runtime variable that the query engine uses
to evaluate view table scans.
In order to keep the diff small, this patch does update view read sets
with this new hash value. That change has a larger blast radius and will
be done in the next set of changes.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
2
# Testing
Existing coverage.
# Description of Changes
Makes runtime parameters explicit in query plans (prerequisite for
parameterized views).
As part of this change, `sender` is no longer baked directly into query
plans as a literal value. Instead it is represented as a parameter in
the query plan. Values are supplied at runtime via a variable
environment called `ExecutionParams`.
Note, parameterized plans are still not shared across subscriptions yet.
That will be done in a follow up.
This is mostly a mechanical change. The majority of the diff is just
threading runtime params/variables through various call sites.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
...
# Testing
Existing coverage
# Description of Changes
So as not to race with the manual snapshots in these tests.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
1
# Testing
Fixes a flaky test
# Description of Changes
Required before we add formal parameters to query plans which in turn is
required to support parameterized views.
Before this change, we cached two types of query representations for a
subscription - a physical plan and what we call a pipelined executor.
This was mainly an artifact of built up technical debt, so this change
removes the physical plan and only caches the actual executor for the
subscription, in preparation for adding bind variables that are
substituted by the executor at runtime.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
...
# Testing
Existing coverage
# Description of Changes
Fixes two bugs around dropping event tables.
**Bug 1:** a deterministic commitlog replay failure introduced in #5269
that permanently prevents a database from restarting after an event
table is dropped. Observed in production on 2.5.0:
```
ERROR ... Failed to open database: DatastoreError: Error deleting row ProductValue { elements: [U32(4134), U16(0), String("account_id"), Array([13])] } from table "st_column" during transaction 26002 playback: ... TableError: Table with ID `4134` not found in `st_table`.
```
## Root cause
Dropping an event table (a `RemoveTable` automigration) deletes its
`st_table`, `st_column`, and `st_event_table` rows in a single
transaction. `prepare_tx_data_for_durability` sorts delete ops by
ascending table id, so replay applies them in this order:
1. The `st_table` (table id 1) delete is replayed first. The table is
recorded in `replay_table_dropped` and its `st_table` row is removed.
2. The `st_column` (table id 2) deletes are replayed next. The handling
added in #5269 calls `is_event_table_for_replay`, which still finds the
table's `st_event_table` (table id 17) row, since that delete has not
been replayed yet.
3. `st_column_changed` is invoked to refresh the table's layout. It
calls `find_st_table_row`, which fails because the `st_table` row is
already gone.
Production replay runs with `ErrorBehavior::FailFast`, so the replica
fails to launch on every restart. The commitlog itself is intact:
databases bricked by this bug recover with no data loss once opened with
this fix.
#5269 tested in-place event table reschemas (which keep the `st_table`
row alive) but not drops.
## Fix
Skip the layout refresh when the referenced table was dropped earlier in
the same transaction, as tracked by `replay_table_dropped`. That set is
the existing mechanism for exactly this replay ordering hazard, and the
`st_table` delete is always replayed before the `st_column` deletes due
to the table id sort, so the guard is reliable.
**Bug 2:** `drop_table` never deletes the table's `st_event_table` row.
It cleans up `st_table`, `st_column`, `st_index`, `st_sequence`,
`st_constraint`, the accessor tables, and `st_scheduled`, but
`st_event_table` was missed when event tables were introduced. Dropping
an event table therefore leaves an orphaned `st_event_table` row,
verified empirically:
```
> SELECT * FROM st_event_table -- after dropping the only event table
table_id
----------
4097
> SELECT table_id FROM st_table WHERE table_id = 4097
(no rows)
```
The orphan is latent today (`st_event_table` is only read via point
lookups by live table ids), but it is the precondition that exposed bug
1, and it is incorrect catalog state. `drop_table` now deletes the row.
Note that both fixes are required independently: the replay guard is
needed even with the `drop_table` fix, because the `st_event_table`
delete replays after the `st_column` deletes (table id 17 vs. 2), and
also because commitlogs already written by unfixed versions contain drop
transactions with no `st_event_table` delete at all. Databases which
already dropped an event table on an unfixed version keep their orphaned
row; cleaning those up retroactively is left for a follow-up (e.g. a
fixup in `rebuild_state_after_replay`, like
`fixup_delete_duplicate_system_sequence_rows`).
# API and ABI breaking changes
None.
# Expected complexity level and risk
1. A guard on an existing replay invariant, plus tests.
# Testing
- [x] New unit regression tests `replay_event_table_drop_no_snapshot`
and `replay_event_table_drop_after_snapshot` in
`crates/core/src/db/update.rs`, following the structure of the #5269
replay tests (create event table, write an event row, drop the table via
`RemoveTable` automigration, reopen to force replay). Verified both fail
with the production error signature before the fix and pass after:
```
---- db::update::test::replay_event_table_drop_no_snapshot stdout ----
Error: DatastoreError: Error deleting row ProductValue { elements:
[U32(4096), U16(0), String("payload"), Array([13])] } from table
"st_column" during transaction 3 playback
...
1: TableError: Table with ID `4096` not found in `st_table`.
```
- [x] The pre-existing #5269 replay tests
(`replay_event_table_schema_change_*`) still pass.
- [x] New smoketest
`automigrate_drop_event_table_replays_after_restart`: publishes a module
with an event table, emits an event, drops the table via automigration,
restarts the server, and verifies the database replays and serves reads
and writes. Passes locally.
- [x] Reproduced the production failure on a stock 2.5.0 standalone
(publish module with event table, republish without it, restart,
identical error), then opened the same bricked data directory with a
standalone built from this branch and verified the database replays and
serves queries with all data intact.
# Description of Changes
Refactors index probes so that multi-column index keys are represented
as a single product value expression `PhysicalExpr::Product` instead of
being split across various fields and structs. As a result, this patch
also simplifies the index-scan and index-join query executor variants
now that index scans/probes share one physical shape.
This change is in preparation for adding formal parameters to query
plans. Since index probe values now have single unified representation
as a `PhysicalExpr`, a future parameterized plan can represent an index
scan or index join as follows:
```rust
IndexProbe::Point(PhysicalExpr::Product(vec![
PhysicalExpr::Param(sender_slot),
PhysicalExpr::Value(other_const),
]))
PhysicalExpr::Product(vec![
PhysicalExpr::Param(sender_slot),
PhysicalExpr::Field(lhs_join_field),
])
```
This will avoid having to duplicate parameter handling in a bunch of
different places or rules.
Note, a very nice consequence of this refactor is that it
removes/consolidates a significant amount of code as the diff shows.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
...
# Testing
Existing coverage
# Description of Changes
This fixes a scheduler crash where a panic from a scheduled JS
reducer/procedure could unwind out of `SchedulerActor::handle_queued`.
The fix keeps the existing panic semantics through `ModuleHost`, so
`defer_on_unwind` still runs and poisons the failed module host, but the
scheduler now catches the unwind after that boundary, logs a warning,
and returns without rescheduling that queue item.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
1
# Testing
...
# Description of Changes
This commit adds special handling in automigrations to accept a broad
set of schema- and layout-altering automigrations on event tables,
including changes that we'd reject for non-event tables like removing or
reordering columns, or layout-incompatibly changing the types of
existing columns.
This is due to a change we want to make to the ControlDB schema, where
we have a product type which is used both as a table type, and as the
column type of an event table, and we want to add an element to that
product type.
I've added a simple smoketest of the new behavior,
`automigrate_reschema_event_table_arbitrarily`. Note that I have not
tested commitlog replay of a database which has undergone one of these
migrations, which has been a common place that bugs have appeared in
similar changes in the past.
I have done a pretty lazy job with the migration plan formatter for the
new step, just printing the name of the event table which is changing,
not any information about the specific changes or the columns. This is
in an effort to save time, as we'd like to release the ControlDB change
blocked by this.
# API and ABI breaking changes
N/a
# Expected complexity level and risk
3 at least: new categories of automigrations have a high risk to
introduce commitlog replay bugs, and it's also possible I have
misunderstood or mis-remembered some of the safety invariants of the
`table` crate.
# Testing
- [x] New (minimal) smoketest.
- [x] Manually tested replay:
- Copied event table from the before version new smoketest into
module-test.
- Published module-test.
- Replaced event table with after version, published, observed and
approved client-breaking automigration.
- Restarted SpacetimeDB.
- Called the procedure `return_value` in the migrated module to force
commitlog replay.
- [ ] Test the ControlDB change in staging, incl. restarting the control
node.
---------
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
# Description of Changes
Prior to this commit, the metric `wasm_memory_bytes` had several
problems:
1. Despite its name, it was used for both Wasmtime and V8 modules. For
V8 modules, it was the same value as `v8_used_heap_size_bytes`.
2. It stored only the value for a single instance at any given time, so
it under-reported a database's memory usage.
3. The same row (set of label values) was written concurrently by all
instances of a particular database, with each one clobbering the
previously written value.
In this commit, we change the metric so that:
1. It is recorded only for Wasmtime instances, not V8 instances. For V8
instances, instead directly check `v8_used_heap_size_bytes`, or one of
the other V8 heap metrics. This change involved moving the recording of
this metric from `module_host_actor.rs` to
`wasmtime/wasm_instance_env.rs`
2. Similar to the V8 heap metrics, all the instances cooperatively share
the metric entry, updating it by incrementing and decrementing rather
than `set`ting.
Note that this metric is used for billing, and so we will need to update
our billing code (elsewhere) to account for the change. In particular,
our billing code should now charge for the sum of `wasm_memory_bytes`
and `v8_used_heap_size_bytes`. We also should expect with this change
for each database's recorded usage to increase, as we are now accurately
recording the usage for all instances, not just one.
# API and ABI breaking changes
Billing metric semantics changed.
# Expected complexity level and risk
3: billing metric semantics changed.
# Testing
I do not know how to test metrics.
# Description of Changes
Previously `assert_after_durable` assumed that `Poll::Pending` meant
confirmed reads was blocked on durability. That was not necessarily
true, because subscription messages pass through the subscription send
worker before reaching the client receiver. A `Poll::Pending` could also
mean the send worker had not delivered the message yet, so a single
immediate poll after marking durability was not guaranteed to be
`Poll::Ready`.
This patch removes `test_confirmed_reads` entirely since the guarantee
that client receivers do not receive messages before they are durable is
already covered by the `ClientConnectionReceiver` tests, where `Pending`
is unambiguous.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
1
# Testing
N/A
# Description of Changes.
Introduces deterministic runtime crate.
Integrate it with RelationalDB.
I think best steps to review:
- Read the
[README](https://github.com/clockworklabs/SpacetimeDB/blob/shub/sim/crates/runtime/README.md)
of runtime crate.
- Look at the integration with existing crates - `durability`, `core`,
`snapshot`, etc.
- Read runtime crate's code.
Draft branch to Test code -
https://github.com/clockworklabs/SpacetimeDB/pull/5019
# API and ABI breaking changes
NA
# Expected complexity level and risk
Does not intend to change any production functionality, but it's big
code.
# Testing
- new crate contains unit and integration tests.
- Existing tests should work for production.
---------
Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
# Description of Changes
Merged `SingleThreadedExecutor` into `SingleCoreExecutor`.
Before this change, we would allocate two database worker threads for
WASM modules - one for reducers and one for procedures.
With this change, `procedures` run on a Tokio `LocalSet`, while reducers
run inline on the OS thread. The goal is to avoid spawning more than one
thread.
I don’t think this meaningfully changes runtime behavior (or we can call
it bit of an optimisaion), and it still avoids the `async` overhead for
reducer calls. It also better aligns with the “thread per core”
principle and allows the single-threaded DST executor to perform close
to production simulation.
# API and ABI breaking changes
NA
# Expected complexity level and risk
2, simple runtime behaviour change but it could have hidden implications
if done wrong.
# Testing
I think existing tests should be enough, considering we also have
benchmark tests in CI.
# Description of Changes
A sort of followup to/unrevert of #4884, an alternative to #4927.
In #3832, we made FunctionBudget a different unit than EnergyQuanta, but
there were still many places where we treated them the same and directly
converted between them. I got confused by that in #4884, and now this PR
properly separates them and corrects the v8 energy calculation.
Also, since we now benchmark the same module in rust and typescript in
the form of the keynote benchmark, this patch adds a new assertion to
that test that the fuel usage of both modules is within 2X of each
other.
# Expected complexity level and risk
3 - touches energy calculation without reverting the problematic #4884,
but I'm confident it's correct this time.
# Testing
- [x] Verified that the conversion factors make sense for wasmtime and
for v8.
- [x] Assert that typescript and rust keynote bench runs have similar
cpu usage
---------
Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
# Description of Changes
I believe this to fix#4947.
Previously, committed read sets stored module-local indexes used for
view dispatch. These indexes are not stable across module updates, but
they were not removed from the committed state, so that later when a
write triggered a view refresh, the runtime could dispatch the wrong
view function or attempt to materialize into the wrong backing table
which would result in a fatal error.
This patch removes these unstable indexes from read sets. The core
behavior change is that committed read sets no longer depend on stale
per-module layout details. They identify the logical view call, and
refresh resolves the current module metadata when needed.
# API and ABI breaking changes
N/A
# Expected complexity level and risk
2
# Testing
- [x] Auto-migrate smoketests
# Description of Changes
Adds support to Rust modules and the SpacetimeDB host for defining HTTP
handlers and registering them to routes.
## User-facing API
In a Rust module, users can annotate functions with the new macro
`#[spacetimedb::http::handler]`. A function annotated this way must
accept exactly two arguments, of types `&mut
spacetimedb::http::HandlerContext` and `spacetimedb::http::Request`
(which is a type alias for `http::Request<spacetimedb::http::Body>`. It
must also return `spacetimedb::http::Response` (which is a type alias
for `http::Response<spacetimedb::http::Body>`).
Once the user has defined an HTTP handler, they can register it to a
route by annotating a function with `#[spacetimedb::http::router]`. Such
a function must take no arguments and return a
`spacetimedb::http::Router`. (The original design put this annotation on
a `static` variable rather than a function, but that turned out to be
undesirable because it required that constructing a `Router` be
`const`.) `Router` exposes various methods for registering handlers to
routes.
All of a database's user-defined routes are exposed under
`/v1/database/:name_or_identity/route/{*path}`.
## Example
See [the new
smoketest](https://github.com/clockworklabs/SpacetimeDB/blob/phoebe/http-handlers-webhooks/crates/smoketests/tests/smoketests/http_routes.rs)
for a more exhaustive example.
A simpler example, which stores arbitrary byte data in a table via a
`POST` request, returns an ID, and then retrieves that same data via a
`GET` request with a query parameter:
```rust
#[spacetimedb::table(accessor = data)]
struct Data {
#[primary_key]
#[auto_inc]
id: u64,
body: Vec<u8>,
}
#[spacetimedb::http::handler]
fn insert(ctx: &mut HandlerContext, request: Request) -> Response {
let body: Vec<u8> = request.into_body().into_bytes().into();
let id = ctx.with_tx(|tx| tx.db.data().insert(Data { id: 0, body: body.clone() }).id);
Response::new(Body::from_bytes(format!("{id}")))
}
#[spacetimedb::http::handler]
fn retrieve(ctx: &mut HandlerContext, request: Request) -> Response {
let id = request
.uri()
.query()
.and_then(|query| query.strip_prefix("id="))
.and_then(|id| u64::from_str(id).ok())
.unwrap();
let body = ctx.with_tx(|tx| tx.db.data().id().find(id).map(|data| data.body));
if let Some(body) = body {
Response::new(Body::from_bytes(body))
} else {
Response::builder().status(404).body(Body::empty()).unwrap()
}
}
#[spacetimedb::http::router]
fn router() -> Router {
Router::new().post("/insert", insert).get("/retrieve", retrieve)
}
```
## Design and implementation notes
- As mentioned above, the router is registered via a function, not a
`static` or `const` item. This is because `static` or `const`
initializers must be `const`, and it turns out to be a pain to make all
of the `Router` constructors be `const fn`s.
- The `#[handler]` macro clobbers the original function name with a
`const` variable of type `HttpHandler`. This is unfortunate, but AFAICT
necessary, 'cause we need to pass the string identifier for the handler
to the `Router`, not the function pointer, and Rust allows no (stable
and reliable) way to get a unique string identifier out of a function
item/value, nor to attach data or implement traits for function
items/values. The alternative(s) would involve changing the signature of
the `Router` methods to have uglier and more complex callsites, e.g.
like `.get("/retrieve", retrieve::handler())`, `.get("/retrieve",
handler!(retrieve))` or `.get::<retrieve>("/retrieve")`. I believe that
registering handlers will be much more common than calling their
functions, so I've chosen to make it so that registering them gets the
convenient syntax, even though the inability to call them directly will
be somewhat surprising.
- I haven't wired up energy handling or timing metrics for handler
execution to anywhere. Procedures are still in the same boat.
- HTTP requests to user-defined routes bypass the usual SpacetimeDB auth
middleware, meaning that the host does not validate (or inspect in any
way) `Authorization` headers in requests before invoking the
user-defined handler. This is required to allow arbitrary
user-programmable handling of `Authorization` headers, including those
in formations which SpacetimeDB would reject. As a result of this,
`HandlerContext` doesn't expose a `sender` or `sender_connection_id`.
- HTTP route paths may consist only of a very restrictive set of
characters. I've chosen this set to keep our options open in the future
to add additional syntax to routes, like for registering wildcard
segments and path parameters:
- ASCII digits.
- ASCII letters.
- `-_~/`.
- The internal data structure that represents a `Router` is currently a
`Vec<Route>`, meaning that resolving a request to a route is
`O(num_routes)`. Registering a route checks against each previous route
for uniqueness, meaning that constructing a router is `O(num_routes ^
2)`. There are TODO comments to use a trie, but I think this can wait,
as I expect most databases to register few routes.
- Commit 999a7c317 contains a fix to a mostly-unrelated bug where a few
bindings introduced by the SATS derive macros were unhygienic and not in
a reserved namespace, leading to name conflicts. I discovered this
'cause I tried writing an HTTP handler named `index` to serve the
index/root of a website and it broke.
## Still TODO
- [x] Resolve various TODO comments in the diff.
- [x] Documentation.
- [x] C# bindings support.
- [x] C++ bindings support.
- [x] V8 host support.
- [x] TypeScript bindings support.
# API and ABI breaking changes
New APIs, currently flagged as `unstable`, which will eventually need
stability guarantees. No (intentional) breaking changes, or changes to
existing APIs at all.
# Expected complexity level and risk
3? Changes to our HTTP routing to support the user-defined routes.
# Testing
<!-- Describe any testing you've done, and any testing you'd like your
reviewers to do,
so that you're confident that all the changes work as expected! -->
- [x] New smoketest of the behavior!
- [x] I dunno, maybe try hosting a simple webpage and see how it works?
- [x] Build a test app with Stripe integration.
- @aasoni did this.
---------
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
Co-authored-by: Jason Larabie <jason@clockworklabs.io>
Those were introduced at info level to debug apparent deadlocking
issues, which turned out to be unrelated. They generate quite a bit of
noise.
Removing them instead of lowering the level as we already have
sufficient logging around view cleanup.
# Expected complexity level and risk
1
# Testing
Logging only.
# Description of Changes
Before this change, we used a single async-enabled wasm runtime for all
requests, even though procedures are the only operation that can yield.
Now each module gets two separate runtimes. We continue to use the same
async runtime for procedures, but now reducers are executed against a
synchronous wasm runtime, backed by a single OS-thread instead of a
Tokio runtime.
The purpose of this change is to remove from the critical path the
overhead associated with async calls that really aren't async at all.
Also includes the following fix from #5135:
> After #4973, WASM procedures can execute concurrently with later
operations on the same WebSocket.
> Before this change, the C# regression testsuite queued several
procedures, then immediately queued `UnsubscribeThen`. After #4973, the
unsubscribe could be applied before the `SubscriptionEventOffset`
procedure callback ran, clearing `MyTable` from the local subscribed
cache. The callback then failed while asserting that the `offset-test:`
row was present.
> This change treats unsubscribe as a separate phase. It is scheduled
after the main work is queued, but only starts once `waiting == 0`, so
all callbacks that inspect subscribed state run before the cache is
cleared.
# API and ABI breaking changes
None
# Expected complexity level and risk
2.5
# Testing
Pure refactor. Relies on current test coverage. #5078 will ensure the
performance is on par with V8.
# Description of Changes
<!-- Please describe your change, mention any related tickets, and so on
here. -->
This reverts commit 458eac8c85.
This rolls back a pricing change that we made to V8 (TypeScript) based
modules. This will be applied as a hotfix to 2.2.0.
# API and ABI breaking changes
<!-- If this is an API or ABI breaking change, please apply the
corresponding GitHub label. -->
None
# Expected complexity level and risk
1 - Just a pricing change.
<!--
How complicated do you think these changes are? Grade on a scale from 1
to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex
change.
This complexity rating applies not only to the complexity apparent in
the diff,
but also to its interactions with existing and future code.
If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning
ways. -->
# Testing
<!-- Describe any testing you've done, and any testing you'd like your
reviewers to do,
so that you're confident that all the changes work as expected! -->
- [x] Tested on staging - I didn't verify that the energy used went down
because I don't think I can do that on staging but at least I know this
doesn't appear to break anything.
Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
# Description of Changes
Prior to this PR, the `V8HeapMetrics` were tracked only for the "main"
instance of a database, i.e. the reducer worker. This meant that we had
little to no visibility into memory usage by procedures.
In this PR, we also track values for the procedure workers. We
considered tracking each instance's usage separately with a unique
integer `instance_id` label, but were concerned about cardinality (see
discussion), so decided instead to track only two sets of label values
per database: `JsWorkerKind::Main` and `JsWorkerKind::Procedure`. The
entries for `JsWorkerKind::Procedure` store the sum of the values for
all procedure workers for that database.
I also moved the logic for calling `remove_label_values` into an
associated function on `V8HeapMetrics`, rather than listing them all in
`remove_database_gauges`. This hides the fact that we have label values
for both `JsWorkerKind` variants.
# API and ABI breaking changes
We don't use any of these metrics for billing, and otherwise do not
consider our metrics a stable API.
# Expected complexity level and risk
2: it would be unfortunate if we reported incorrect values for these
metrics, though (as mentioned above) they are not used for billing, only
diagnostics.
# Testing
I do not know how to test metrics.
# Description of Changes
In keeping with the "pipeline everything" approach of SpacetimeDB, this
patch serializes multiple client updates in a single websocket message
using the v3 protocol.
# API and ABI breaking changes
None. `spacetime subscribe` was updated to use the v3 websocket api, but
it falls back to v1 if protocol negotiation fails.
# Expected complexity level and risk
2
# Testing
This patch updates `spacetime subscribe` to use the v3 websocket
protocol by default in order to get adequate coverage via the
smoketests.
# Description of Changes
Adds the commitlog knobs `max_segment_size`, `write_buffer_size`, and
`preallocate_segments` to the server's `config.toml`. There are other
seemingly more advanced knobs that I did not add at this time.
This patch also increases the `DEFAULT_WRITE_BUFFER_SIZE` from `8KiB` to
`128KiB` to optimize high throughput workloads like the keynote-2
benchmark.
# API and ABI breaking changes
None
# Expected complexity level and risk
1
# Testing
Manual
# Description of Changes
In keeping with the "pipeline everything" ethos, I've replaced `recv`
with `recv_many` on `ClientConnectionReceiver` so that a client
connection's websocket send path works on a batch of messages at a time,
if possible.
# API and ABI breaking changes
None
# Expected complexity level and risk
1.5
# Testing
Refactor
# Description of Changes
- Introduce `SnapshotRepo` as the snapshot backend trait and decouple
`SnapshotWorker` from the
filesystem-backed implementation.
- Dynamic dispatch-ing is not needed but done to avoid propagating
generics all over the place.
- Snapshotting is also not on the hot path, and durability also follows
the same `dyn` pattern.
- Make commitlog `durability::Local` generic over `Repo`, so the same
durability implementation can be
reused with in-memory and fault-injected repositories.
- Add a few small API exposure changes under `cfg(test)` for DST
integration.
# API and ABI breaking changes
NA.
# Expected complexity level and risk
2
# Testing
Existing tests should be enough.
---------
Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
# Description of Changes
This reverts commit ca1b45f6c2 since after
https://github.com/clockworklabs/SpacetimeDB/pull/4981 we no longer hold
the lock while compressing.
# API and ABI breaking changes
None
# Expected complexity level and risk
1
# Testing
N/A
# Description of Changes
This was originally introduced in #4302; essentially, we stopped
unconditionally setting `HookFunctions.recv` to undefined and started
setting it to the value stored in
`ctx.get_embedder_data(RECV_SLOT_INDEX)`. However, in the code path for
v1 js modules, we never actually set that embedder data slot, and so
recv was a garbage value.
# Expected complexity level and risk
1: concentrated fix
# Testing
- [x] Repro no longer segfaults.
Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
# Description of Changes
Defers commitlog segment compression until write load has been idle for
a short window, while still forcing progress when the uncompressed
segment backlog grows too large.
This tries to avoid creating a durability backlog that eventually blocks
the main execution thread.
# API and ABI breaking changes
None
# Expected complexity level and risk
2
# Testing
TODO
# Description of Changes
The core motivation for this change is simple: avoid cross-thread
handoffs and synchronization on the main execution path.
Before this change, the ingress task for each websocket connection would
wait for a completion response on each request before submitting the
next request to the database. This was mainly used to guarantee that we
delivered message responses in receive-order per connection. However it
also meant that for every request, we notified a waiting Tokio task,
which potentially incurred kernel-assisted wakeup and scheduler
overhead.
Note this design existed mainly for historical reasons. Before the
database had a dedicated job thread, requests were not serialized
through a single queue. The module instance was gated behind a semaphore
which guaranteed mutual exclusion, but it did not guarantee FIFO
ordering. Awaiting the completion of each request in `ws_recv_task` was
therefore the mechanism that enforced per-connection receive-order
semantics. However it now serves primarily as a source of overhead.
Procedures are the important exception. They are not serialized through
the main worker queue. Instead they use their own instance pool so as to
be able to run concurrently with other requests. However procedures may
be composed of multiple transactions and they may effectively yield
between transactions. This means that before this change, if a procedure
were to yield, it would effectively block all subsequent requests from
that client until it returned which is quite undesirable.
So with this change, procedures may execute out of order with other
operations received on the same WebSocket. Hence if this is not a
desirable property, clients must enforce ordering themselves by waiting
for a response before submitting the next request.
## What changed?
### 1. Different instance managers for procedures and everything else
Procedures use a bounded instance pool where each instance is backed by
an isolate running in a thread. Reducers and all other operations are
serialized through an mpsc queue that feeds a single isolate running in
a thread.
Trapped isolates are replaced inline. Only a fatal error within one of
the instance threads results in the `ModuleHost` and all its connections
being dropped. The host controller will recreate a new `ModuleHost`
lazily on the next request.
### 2. New enqueue-only `ModuleHost` interface
`ClientConnection` now calls enqueue-only methods on `ModuleHost` which
return immediately after enqueuing on the main instance lane or in the
case of a procedure, checking out an available instance and starting the
operation.
### 3. Separate `ModuleHost` interfaces for scheduled reducers and
scheduled procedures
Scheduled reducers now target the main js instance/worker, while
scheduled procedures go through the pool. The scheduler now
distinguishes between reducers and procedures and calls the appropriate
method.
Note, the scheduler does not pipeline its operations. It waits for each
one to complete before scheduling the next operation. This means that a
long running procedure will block all other operations from being
scheduled. This will need to be fixed at some point, but this patch
doesn't change the current behavior.
### 4. Misc
This patch also names the main js worker thread for better diagnostics.
It also disables core pinning by default and makes it an explicit
opt-in.
This last one is pretty important. The current architecture reduces
thread and context switching significantly such that naive core pinning
may perform worse than just deferring to the OS scheduler on certain
platforms. As it stands, the main motivation which led us to our
original core pinning strategy no longer exists, so we should probably
just defer to the OS until we've designed a proper scheduler that suits
our needs.
# API and ABI breaking changes
As mentioned above, with this change, procedures may execute out of
order with other operations received on the same WebSocket. Hence if
this is not a desirable property, clients must enforce ordering
themselves by waiting for a response before submitting the next request.
# Expected complexity level and risk
4
# Testing
This is mainly a performance oriented refactor, so no additional
correctness tests were added. However this patch does touch a lot of
code that could probably use more coverage in general. Benchmarks were
run to verify expected performance characteristics.
---------
Signed-off-by: joshua-spacetime <josh@clockworklabs.io>
Co-authored-by: Noa <coolreader18@gmail.com>
When creating or compressing a snapshot, `fsync` all files and
directories, so as to ensure that the snapshot is durable on the local
disk.
This obviously amounts to a large number of `fsync` calls, which may
negatively impact performance of taking a snapshot -- since we hold a
transaction lock while taking a snapshot, this is not to be taken
lightly.
# Expected complexity level and risk
3 -- performance impact
# Testing
I haven't quantified the performance impact.
# Description of Changes
See the corresponding commits/PRs for descriptions.
# API and ABI breaking changes
None
# Expected complexity level and risk
1 -- individual PRs already reviewed.
# Testing
No semantic changes.
# Description of Changes
`MutTxId::add_columns_to_table` creates new table but only copies
sequences to in-memory state, which causes `autoinc` columns to reset on
module restart.
The existing implementation relies on `create_table_and_update_seq`
helper, which only updates the sequence state in memory. This change
ensures that the `allocation` is also persisted to the system table,
keeping it consistent across restarts.
# API and ABI breaking changes
NA
# Expected complexity level and risk
2
# Testing
Added a test, which migrate table and checks for `autoinc` column value
without and with restart.
---------
Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
# Description of Changes
1. This adds some basic tests of `LockedFile` (which would generally
have also passed before these changes).
2. `LockedFile` can now have contents, which are added to the errors if
we can't aquire the lock. The default metadata has the pid and a
timestamp.
I now see that there is a separate `Lockfile` that doesn't clean up
locks on a crash, so that should probably also be cleaned up (but in a
separate PR).
# Expected complexity level and risk
1.
# Testing
This has unit tests.
# Description of Changes
Revert the following PRs that have caused some breakage:
```
a32cffa76 Finish refactoring out replay (#4850)
d639be0af Replay: some code motion & reuse `ReplayCommittedState` (#4849)
78d6b6f7d Update NativeAOT-LLVM infrastructure to current ABI (#4515)
d5c1738c1 Better module backtraces for panics and whatnot (#577)
6f23b19f3 Wait for database update to become durable (#4846)
81c9eab86 Add `spacetime lock/unlock` to prevent accidental database deletion (#4502)
809aebd7c Move field `replay_table_updated` to `ReplayCommittedState` (#4807)
21b58ef99 Update axum (#2713)
b5cadff7a Extract replay stuff out of `CommittedState`, part 1 (#4804)
```
I also updated the Python smoketests for breakage introduced in
https://github.com/clockworklabs/SpacetimeDB/pull/4502. Reverting that
PR caused conflicts, so this fix is more straightforward.
# API and ABI breaking changes
Maybe kind of, but we haven't released any of these.
# Expected complexity level and risk
1
# Testing
Ask @bfops about testing
---------
Co-authored-by: Zeke Foppa <bfops@users.noreply.github.com>
# Description of Changes
Add support for btree indices where the keys are encoded byte strings
for e.g., multi-column indices of no-unbounded-types (arrays and
strings) that aren't floats.
The main interesting stuff in this PR is in `bytes_key.rs` which defines
`RangeCompatBytesKey`, a type that is derived from `BytesKey`, by
converting little-endian encoded integers to big-endian. Signed integers
are now also supported, but floats are not. `table_index/mod.rs` also
includes a bunch of interesting stuff.
# API and ABI breaking changes
Technically this fixes pre-existing bugs in the handling of `Excluded`
ranges for multi-col indices.
# Expected complexity level and risk
2?
# Testing
- A proptest `order_in_bsatn_is_preserved` is now adjusted and enabled
to exercise the ordering of `RangeCompatBytesKey`.
- A proptest `btree_multi_col_range_scans_work` is added to check the
behavior of range scans on multi-col indices.
---------
Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
# Description of Changes
Uses `Isolate::add_near_heap_limit_callback` to prevent unbounded heap
growth. Upon nearing the heap limit, we will now:
1. Call `Isolate::terminate_execution`.
2. Request that v8 double the heap limit.
Then, upon finishing a function call, we lower the heap limit back down.
This should hopefully fix the issue where v8 hits the heap limit and
crashes the whole process.
Also improves the way termination requests are checked for and
processed.
# Expected complexity level and risk
2
# Testing
- [ ] Manual testing with memory-leaky modules
# Description of Changes
First two commits are code motion.
The second commit fixes a mistake I made in a previous PR that made us
use potentially several `ReplayCommittedState`s per
`datastore.replay(..)`.
More to come in terms of PRs; stay tuned.
# API and ABI breaking changes
None
# Expected complexity level and risk
3
# Description of Changes
Similar to https://github.com/clockworklabs/SpacetimeDB/pull/4801, after
we evaluate subscriptions on the main database thread, we send the
results to a worker whose job it is to fan out the updates for the
relevant clients. Hence we want to make sure this worker is not
constantly parked on `recv()` as each `send` on the main thread will
incur overhead waking the task.
To avoid this I've added a utility that wraps an `mpsc`
`UnboundedReceiver` with an adaptive "linger" policy. On each message
`recv`, the worker will now "linger" for a period of time and wait for
any more messages before parking itself on the `recv()` again.
# API and ABI breaking changes
None
# Expected complexity level and risk
1
# Testing
Manual performance testing for now. Automation to follow.
Confirmed reads applies only to subscription clients, calls to the the
HTTP API publish endpoint return a success response before the operation
is confirmed.
While we await scheduling of a new database, updates require to wait for
the update transaction to be confirmed. To allow this, the
`TransactionOffset` channel and the database's `DurableOffset` need to
be returned all the way up to the request handler.
Note that waiting for confirmation is almost always the right choice, so
can't be opted out of at the time of submission of this patch. Callers
may, however, extend the timeout after which waiting for confirmation is
cancelled.
# Description of Changes
While working on #1111 I realised that we have a bug with subscriptions
not being unique when multiple clients with the same identity are
connected. I fixed the bug and only then realised it was already fixed
yesterday in #1121. When working on my changes I created a test for the
issue, so I guess it doesn't hurt to at least submit it.
# Expected complexity level and risk
1
---------
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
clockwork-labs-bot