# Description of Changes
Closes
[#2686](https://github.com/clockworklabs/SpacetimeDB/issues/2686).
Add support for listening using the [PG wire
protocol](https://www.postgresql.org/docs/current/protocol.html) so `pg`
clients could be used against the database.
# API and ABI breaking changes
The output of `duration` is changed to `rfc3339`, instead of the way is
made with `sats` because is what is done in `pg`, see note below.
# Expected complexity level and risk
2
~~There is open questions that are in the [ticket
#2686](https://github.com/clockworklabs/SpacetimeDB/issues/2686). Also
the crate used here require `RustTls`, so it could be good idea to
decide if~~:
* ~~Rewrite a big chunk of code to use `OpenSSL`~~
* ~~Move to `RustTls`
https://github.com/clockworklabs/SpacetimeDB/pull/1700~~
* ~~Pay for the extra compilation cost~~.
I open another port(`5433`) to listen for `pg` connections using `ssl`.
Need to be decided if this is the way or instead try to multi-plex the
current port for both protocols.
# Testing
Only manual testing so far. Solving the above questions allow me to
implement some unit tests. Also, not yet integrated into cloud for the
same reasons.
- [x] Adding some test for the binary encoding of special and primitive
types
- [x] Smoke test using `psql` that connect to the db instance and run
some queries
- [x] Manually inspect using a UI database explorer how infer the types,
some of this tools generate special widgets when displaying `json,
duration, etc`
---------
Co-authored-by: Noa <coolreader18@gmail.com>
# Description of Changes
- Adds endpoint for for pretty printing migration plan.
- It also changes current `publish` endpoint to optionally provide
`MigrationToken` and `MigrationPolicy` to allow migration with breaking
clients.
# API and ABI breaking changes
Backward compatible change to existing API and new Api
# Expected complexity level and risk
2
# Testing
- Existing smoketest should cover changes for `publish` endpoint.
- For pretty print endpoint, smoketests can be written only after cli
changes.
---------
Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: James Gilles <jameshgilles@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
# Description of Changes
Implements [subscribing to durable
commits](https://github.com/clockworklabs/SpacetimeDBPrivate/issues/1594).
The setting works on a per-connection level, and essentially just delays
sending transaction updates until the transaction is reported as durable
by the database.
For connectionless SQL operations, the setting works per-request. No SQL
syntax is provided by this patch to toggle the configuration.
After some deliberation, I opted to obtain the offset when a transaction
commits (as opposed to when it starts). This creates some mild
inconvenience, because we prevent the transaction from committing until
the corresponding subscription updates are enqueued.
The strategy is, however, more correct should we ever support weaker
isolation levels, and it is easier to document.
Follow-ups include:
- Provide SQL syntax (`SET synchronous_commit = ON` or something)
- C# and TypeScript SDKs
- Reference docs?
# API and ABI breaking changes
Not breaking, but adds a parameter to the subscribe and sql endpoints.
# Expected complexity level and risk
4
To the author's understanding, ordering of outbound messages is not
changed by this patch, even if there are messages that don't have a
transaction offset (such as error messages). I.e. while waiting for the
transaction offset of a message to become durable, no message enqueued
after that message will be delivered. This may not be desirable in some
cases.
The patch may contain concurrency bugs, e.g. awaiting futures that may
never resolve.
# Testing
- [x] Implemented a new test in the `module_subscription_actor` module
- [x] Added unit tests for the core logic in `ClientConnectionReceiver`
It would be desirable to also have integration-level tests, but I'm
currently unsure how to write those without being able to control if and
when the database reports an offset as durable.
---------
Signed-off-by: Kim Altintop <kim@eagain.io>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
# Description of Changes
<!-- Please describe your change, mention any related tickets, and so on
here. -->
We stopped incrementing the incoming queue length metric. This patch
increments it again and adds a regression test.
# 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
<!--
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. -->
1
# 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] Regression test
# Description of Changes
Closes#3170 .
Commit messages:
### Increase the default incoming-queue-length limit
2048 turned out to be too low a value for BitCraft, as their world
upload process requests on the order of 6000 reducers very rapidly. We
still feel that having a limit is valuable to prevent malicious or
misguided clients from taking an arbitrarily large amount of host
memory, so we bump the value to give us a wide safety error for
BitCraft's needs but don't remove the limit entirely.
### Add log at `warn` when the host disconnects a client due to too many
requests
# API and ABI breaking changes
N/a
# Expected complexity level and risk
1
# Testing
- [x] @mamcx to run a BitCraft bot test.
The `Clone` impl for `ClientConnection` would create an independent
instance that could not observe module hotswapping. This would result in
methods called on a replaced `ModuleHost` to fail, because that host
exited already.
Fix by reading the `ModuleHost` from the watch channel directly, instead
of maintaining a redundant copy.
Also fix `watch_module_host` to properly mark the current module host as
seen.
# Expected complexity level and risk
2
# Testing
- [x] test suite passes
- [x] ran @bfops repro script
# Description of Changes
We recently merged several repos together. This PR clarifies the license
terms for several subdirectories, as well as the relationship between
the licenses.
The licenses in our subdirectories have become symbolic links to
licenses in our toplevel `licenses` directory. For any particular
subdirectory's license file in the diff, you can click `... -> View
file` and then click on the text that says "Symbolic Link" on that page.
This will take you to the license file that it links to.
I have also updated the `tools/upgrade-version` script to update the
change date in the new `licenses/BSL.txt` file.
# API and ABI breaking changes
None.
# Expected complexity level and risk
1
# Testing
None. Only changes to license files.
---------
Co-authored-by: Zeke Foppa <bfops@users.noreply.github.com>
# Description of Changes
Out-of-band discussions with the BitCraft team brought up questions
about whether it was possible for a rejected client connection to start
an expensive computation like a subscription before their connection was
killed, e.g. by sending a `Subscribe` message along the WebSocket before
`client_connected` had finished returning `Err`.
I don't believe this was actually possible, as `ClientConnection::spawn`
called and awaited `call_identity_connected` before invoking its `actor`
closure, and it was that `actor` which processed `Subscribe` messages.
But it was somewhat difficult to verify that behavior, and so I
re-organized the code so that the outer layer of the `subscribe` handler
obviously had that property without having to step into
`ClientConnection::spawn`.
I also added some additional logging to the subscribe route, including
the `X-Forwarded-For` header in more messages, as the BitCraft team
complained about having difficulty correlating IP addresses with
connections. The log levels remain the same as before, just with
additional information added:
- Successful connections are at `debug` level,
- Rejected connections are at `info` level (these are the ones BitCraft
cares about in this case).
- Failed connections are at `warn`.
As the levels are unchanged, this should not add undesirable log noise.
# API and ABI breaking changes
N/a
# Expected complexity level and risk
3? The `subscribe` route was complex and remains so. I believe this
change simplifies the code and makes the logic more obvious, but
reviewers should take care to verify that the behavior actually is
equivalent as I believe.
# Testing
- [ ] I would like a test deployment of BitCraft to staging or
something, or to include this patch in the next bot test that we do
anyways, just for sanity's sake.
Mario Montoya