# 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.
joshua-spacetime