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

Revert "Revert "treewide: Bump rust version to nightly-2023-12-28"" #1244

Open
wants to merge 14 commits into
base: Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
Choose a base branch
from

Conversation

ethan-readyset
Copy link

This reverts commit bde05974330a69526f06f1fbcafab925064cd659.

@readysetbot readysetbot force-pushed the Ibbc34e0856ed22b475e75262e14f8d55995089d8 branch from 91f7145 to 93220d7 Compare April 26, 2024 15:14
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from 54932c6 to 05685f1 Compare April 26, 2024 15:15
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from 05685f1 to 3715a08 Compare May 6, 2024 19:07
@ethan-readyset ethan-readyset changed the base branch from Ibbc34e0856ed22b475e75262e14f8d55995089d8 to Ie5faa100158fc80c906d8ad5cb897d8a02a07be9 May 6, 2024 19:07
@readysetbot readysetbot force-pushed the Ie5faa100158fc80c906d8ad5cb897d8a02a07be9 branch from 2f1709d to e60957b Compare May 6, 2024 19:18
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from 3715a08 to a299900 Compare May 6, 2024 19:18
@readysetbot readysetbot force-pushed the Ie5faa100158fc80c906d8ad5cb897d8a02a07be9 branch from e60957b to 4394921 Compare May 7, 2024 15:26
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from a299900 to d1761d2 Compare May 7, 2024 15:26
altmannmarcelo and others added 2 commits May 8, 2024 13:02
As part of MySQL 8.4 release, terminology about Master/Slave has been
replaced. During snapshot, we issue SHOW MASTER STATUS to gather the
current binlog position. This command has been replaced with SHOW
BINARY LOG STATUS by mysql/mysql-server@6e2c577 . Unfortunately, the
new terminology is not available on 8.0 series, so we need to check
the server version and conditionally adjust the query we issue.

Also, adjusted the checksum query to use the new terminology of
source_binlog_checksum is compatible with 8.0 and 8.4.

Ref: REA-4374
Closes: #1253

Release-Note-Core: Adjusted replicators terminology to be compatible
   with MySQL 8.4.

Change-Id: I2a57d07ef1a4a426efce3e1989d2d0e3436b6d52
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7449
Tested-by: Buildkite CI
Reviewed-by: Jason Brown <[email protected]>
Since there is no `i24/u24` native to Rust, we use `u32` and do some
manual bounds checking when converting from other types.

We also introduce a MySQL-specific logictest subdirectory, for tests
(such as the one included here) which we never expect to run against
PostgreSQL. Likely some existing tests should be moved there.

Release-Note-Core: Added support for MySQL's `MEDIUMINT` column type.
Fixes: REA-4285
Change-Id: I4530093ea029957dc4c8b32ab6b56a47cce177ca
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7461
Tested-by: Buildkite CI
Reviewed-by: Jason Brown <[email protected]>
@readysetbot readysetbot force-pushed the Ie5faa100158fc80c906d8ad5cb897d8a02a07be9 branch from 4394921 to f9ceca3 Compare May 13, 2024 16:23
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from d1761d2 to a171a8e Compare May 13, 2024 16:23
jasobrown-rs and others added 10 commits May 13, 2024 21:19
Allow clustertests to set the `--enable-experimental-post-lookup`
flag.

Change-Id: Ia6b485922f6097154a1c4134f0aae827f4b5b0eb
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7415
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
If you have an empty directory under the `benches_data/<db>`, you will
get a confusing error message like this:

```
No schema for benchmark {name}
```

It ends up being that you don't have as schema file (suffixed with
`.sql`) in on of the subdirectories. The error message fails to insert
the bench test name (from it's folder name), and thus unclear. This CL
just fixes that error message a bit.

Change-Id: I2f85bc43d5ce986894233bff855392fc1345b9d6
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7469
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
Short circuit some post-lookup operations if the results contain only
a single key (possibly with multiple rows), or a single key with a
single row.

Change-Id: I6698188a7aeb2a1a575896a38387cc77225fe9e7
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7470
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
Allow parameterization of a WHERE IN clause when the SELECT
contains aggregations. The aggregation across the keys in from the
WHERE IN still must execute as a post-lookup operation.

Release-Note-Core: Allow parameterizing WHERE IN
  clauses when the query contains aggregations.
Change-Id: Iaf28fb394c4964e5d7e9869b3741fc2017c492d5
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7399
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
If we are no longer replicating a table, e.g. due to the presence of
`--replication-tables-ignore`, we will register it as a non-replicated
relation on the controller. We will then later try to drop the base
table, but will instead only drop the non-replicated relation
registration, leaving the base table around. Additionally, if the table
was only partially snapshotted, we could later error when trying to
retrieve its replication offset after snapshotting has supposedly
finished (but we've haven't been replicating that table). With this
change, we drop leftover tables before registering non-replicated
relations, so that attempting to drop the base table actually does so.

Fixes: REA-3770
Change-Id: Ie98eccd888f3c250d18b72a799d0dcfb5622a872
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7472
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
The MySQL min and max positions can be far apart. This happens on
unbalance workloads, where for example one of the tables do not
receive updates for a long period of time, like a config table that
might be static. This causes the MySQL min position to fall behind
the max position, in case of a restart, the replicator has to catch
up starting from the min position. This causes a lot on unnecessary
data to be re-streamed.
Currently we only update the min position when MySQL rotates the
binary log and we receive a EventType::ROTATE_EVENT.
Adjusting the position on all tables might be costly if the
installation has a lot of tables.

This commit adjust the replicator to adjust table position on a fixed
interval. This interval is hardcoded to 10 seconds.
The event we act on is either the EventType::QueryEvent when we
receive a "COMMIT" query, or the EventType::XidEvent. They are
virtually the same thing (a commit), but depending on the storage
engine MySQL reports a query COMMIT or an XID event.
We also report the position once we have finished the initial catch up
phase.

Ref: REA-4326
Closes: #1223

Release-Note-Core: Adjusted MySQL replicator to report table position
on a fixed interval(10 seconds). This makes the replicator to keep
distance short between min and max positions. This is useful when
Readyset restart, ensuring that we do not have to re-stream a lot of
binary logs to catch up.

Change-Id: I6dfaf523b8851597a6a0fd97f4d4627ca2f4ea80
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7363
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
Since MySQL binlogs `TRUNCATE` as a statement (`QUERY_EVENT`), but it's
not DDL and doesn't have a corresponding recipe `Change`, we were just
ignoring it. Now the MySQL replicator parses it, emitting the
`TableOperation::Truncate` we had already added for Postgres.

Fixes: REA-4325
Closes: #1221
Release-Note-Core: Added support for `TRUNCATE TABLE` statements for MySQL.
Change-Id: Ia40551e40fa70598973587f5b26e8662419e9853
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7488
Tested-by: Buildkite CI
Reviewed-by: Marcelo Altmann <[email protected]>
Similar to how we split up the postgres logictests, split up the mysql
logictests into numbered subfolders so we can run them in parallel to
reduce build times.

Change-Id: I4bb088b00f2f1e6f43c7791f9517d63e27c93a22
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7477
Reviewed-by: Michael Zink <[email protected]>
Tested-by: Buildkite CI
Now that `8c9bc345a6a42f071bf1b621047f840eb9b31379` is committed, most
of the logictests that were under the `out-of-scope/ENG-629` directory
can be moved out so they execute with everything else. There are still
some failing tests, but those are almost all related to `DISTINCT`,
which isn't supported for where in and aggregates.

Change-Id: I181694eacf94a3cc04c46e5d8a16e479004ac361
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7478
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
Reviewed-by: Michael Zink <[email protected]>
Lifts most of the restriction of
`ff819dc0d9cede2af458e17a30830f2a1e843821`, which prevented WHERE IN
and aggregates from being generated for the same query. After
`8c9bc345a6a42f071bf1b621047f840eb9b31379`, we can generate most
aggregations alongside `IN` clauses, but we still need to disallow
`DISTINCT`, either as a plain modifier on a column or as a modifier on
certain aggregation function (`sum`, `count`).

Change-Id: I76ac3573d864e39a8d3b91a6155fc32914332dce
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7479
Tested-by: Buildkite CI
Reviewed-by: Ethan Donowitz <[email protected]>
@readysetbot readysetbot force-pushed the Ie5faa100158fc80c906d8ad5cb897d8a02a07be9 branch from f9ceca3 to fb03811 Compare May 21, 2024 20:05
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from a171a8e to afba960 Compare May 21, 2024 20:05
Previously, the views synchronizer only checked the server for views for
queries that were in the "pending" state. This meant that if the
migration handler set a query's state to "dry run succeeded" before the
views synchronizer had a chance to check the server for a view, the query
would be stuck in the "dry run succeeded" state forever, even if a view
for the query did indeed exist already.

This commit fixes the issue by having the views synchronizer check the
server for views for queries in *either* the "pending" or "dry run
succeeded" states. In order to prevent the views synchronizer from
rechecking every query with status "dry run succeeded" over and over
again, a "cache" has been added to the views synchronizer to keep track
of which queries have already been checked.

While working on this, I also noticed that it was possible for the
following sequence of events to occur:

- Migration handler sees that a query is pending and kicks off a dry run
  migration
- Views synchronizer finds a view on the server for the same query and
  sets the status to "successful"
- Migration handler finishes the dry run migration for the query and
  overwrites the status as "dry run succeeded"

This could lead to a situation where a query that was previously
(correctly) labeled as "successful" is moved back to the "dry run
succeeded" state. To fix the issue, this commit updates the migration
handler to only write the "dry run succeeded" status if the query's
status is still "pending" after the dry run is completed.

Release-Note-Core: Fixed a bug where queries that already had caches
  were sometimes stuck in the `SHOW PROXIED QUERIES` list
Change-Id: Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7442
Tested-by: Buildkite CI
Reviewed-by: Jason Brown <[email protected]>
This reverts commit bde05974330a69526f06f1fbcafab925064cd659.

Change-Id: I56b71ed96e508ac617579ca1d0e181a1387671f1
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7396
Tested-by: Buildkite CI
Reviewed-by: Jason Brown <[email protected]>
@readysetbot readysetbot force-pushed the I56b71ed96e508ac617579ca1d0e181a1387671f1 branch from afba960 to 06cc8c7 Compare May 22, 2024 16:01
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ altmannmarcelo
❌ mvzink
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants