Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-alpha deploy branch #1061

Closed
wants to merge 18 commits into from
Closed

release-alpha deploy branch #1061

wants to merge 18 commits into from

Conversation

bfops
Copy link
Contributor

@bfops bfops commented Apr 9, 2024

This tracks the release-alpha branch, coordinated with the BitCraft alpha release.

I believe these are the important changesets included:

cd170887 Metrics were not measuring the time it took to commit the tx. I am now measuring that time. Commitlog appending is still not measured but should be measured separately in the future. (#1045)
bf53c346 Reject large SQL queries (#1037)
45a4ea84 Merge commit 'kim/commitlog2-integration' into release-alpha
11e4ddce Consistently use `--identity` `-i` for identity args to CLI (#1041)
53a5a42a Batch metrics with ctx (#916)
fbd479cd Detect unsatisfiable range queries; warn and short-circuit.

On top of this master revision:

commit 99b2fd426fc4061e5e5324b0e18278dacdccdde5
Author: Noa <coolreader18@gmail.com>
Date:   Tue Apr 2 20:54:53 2024 -0500

    Bump to reqwest 0.12 (uses hyper 1.0) (#1031)

gefjon and others added 11 commits April 1, 2024 11:56
This commit fixes a panic caused by unsatisfiable range bounds on an index query,
e.g. `WHERE x < 5 AND x > 5`.
These unsatisfiable bounds made Rust's `BTreeMap` angry
(See https://doc.rust-lang.org/src/alloc/collections/btree/search.rs.html#106-124),
and panicked.
They also represent probable bugs,
as it's silly to write a query which statically will return no rows.

With this commit, we detect statically unsatisfiable bounds in two cases:
- When compiling queries, we log a message at `WARN` containing the offending query.
- When evaluating queries, we silently construct an `EmptyRelOps`
  rather than a real query iterator.

This commit also adds a test that the offending queries can be compiled and executed
without panicking, and select no rows.
Defines traits intended to abstract over the kind of persistence a
database utilizes. The only implementation is (host-)local durability in
terms of the new commitlog crate.

The trait definitions may not be considered stable yet, but are in their
tentative form needed for further integration of the new commitlog.
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.
`TxData` is the representation of a transaction after it was committed,
and is passed around for evaluation of subscription queries and sending
the result to clients.

With the new commitlog, it can be represented more compactly, such that
copies for writing to the log can be avoided.

Note that this patch stops short of refactoring `DatabaseUpdate`, which
is another representation of the same information as sent to clients.
This means that `ProductValue`s need to be cloned from `TxData`, just as
before.
Disk usage reporting was left unimplemented in previous patches of the
series, as its semantics are slightly different from before.

Namely, inspecting the size of the commitlog now requires to `stat(2)`
the segment files, and is thus fallible.

Also, a size reporting function is only defined for local durability
(i.e. the commitlog). The behaviour when the database is in a follower
state is left unspecified.
* Commit iter metrics

* test: Make full-join benchmark async

* use Vec instead of Hash

* Revert "test: Make full-join benchmark async"

This reverts commit eed5aa8.

* rebase on master

* lint

* refector api

* remove lifetime from ctx (#1017)

* move ctx to higher level

* fix boilterplate

* batch more

---------

Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
* Consistently use `--identity` `-i` for identity args to CLI

Prior to this commit, some subcommands used `--as-identity` as the long,
some of them used `-I` as the short, and there was no clear pattern.

With this commit, we just use `--identity` as the long everywhere,
and `-i` as the short.

* Rename variables per John's review
This avoids a stack overflow when compiling deeply-nested `AND` and `OR` expressions.
@bfops bfops added the Do not merge Do not merge PRs with this label without coordinating further label Apr 9, 2024
@bfops bfops changed the title Release alpha release-alpha deploy branch Apr 9, 2024
…t the tx. I am now measuring that time. Commitlog appending is still not measured but should be measured separately in the future. (#1045)
@bfops bfops marked this pull request as ready for review April 9, 2024 19:08
@bfops bfops marked this pull request as draft April 9, 2024 19:26
bfops and others added 6 commits April 9, 2024 13:45
… module restart (#1053)

Fixes #1051.

Queue length metrics were not being reset in all cases for module restarts.
This patch:

(1) Disables unused WasmInstanceEnv metrics,
(2) Removes unused query compilation metrics,
(3) Removes unused RelationalDB metrics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do not merge Do not merge PRs with this label without coordinating further
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants