readysettech / readyset

Readyset is a MySQL and Postgres wire-compatible caching layer that sits in front of existing databases to speed up queries and horizontally scale read throughput. Under the hood, ReadySet caches the results of cached select statements and incrementally updates these results over time as the underlying data changes.
https://readyset.io
Other
4.29k stars 118 forks source link

Bail if upstream URL is malformed #82

Open ayymart opened 1 year ago

ayymart commented 1 year ago

In at least one case (postgres missing db name), a malformed upstream URL results in errors that could be cleaner (indefinitely repeating replicator errors).

We should make this case a permanent failure with a user-friendly message, and investigate/fix possible others.

From SyncLinear.com | REA-2917

ayymart commented 1 year ago

Postgres can be configured to allow passwordless login as well. It does, however, always require a username, as far as I've read.

ayymart commented 1 year ago

In MySQL, you can create a superuser with no username or password:

CREATE USER ''@'%' IDENTIFIED BY '';
GRANT ALL PRIVILEGES ON *.* TO ''@'%';

When such a user exists, ReadySet will connect and replicate from a URL of the form:

mysql://<host>:<port>/<db>

So we don't want to require username + password for MySQL deployments.

ayymart commented 1 year ago

Postgres username missing:

Error { kind: Db, cause: Some(DbError { severity: "FATAL", parsed_severity: Some(Fatal), code: SqlState(E28000), message: "no PostgreSQL user name specified in startup packet", detail: None, hint: None, position: None, where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("postmaster.c"), line: Some(2268), routine: Some("ProcessStartupPacket") }) }

Postgres password missing:

Error { kind: Config, cause: Some("password missing") }

Postgres bad username/password:

Error { kind: Db, cause: Some(DbError { severity: "FATAL", parsed_severity: Some(Fatal), code: SqlState(E28P01), message: "password authentication failed for user \"postgress\"", detail: None, hint: None, position: None, where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("auth.c"), line: Some(335), routine: Some("auth_failed") }) }

Conclusions:

It might be possible to differentiate between recoverable and unrecoverable errors based on this information, but it would involve matching on strings and/or opaque error codes, which is brittle. We should just check for these fields manually.

ayymart commented 1 year ago

Blocked because I think the most robust way to handle these errors is to treat them as unrecoverable where they are now, rather than add logic elsewhere to try and catch them early.

ayymart commented 1 year ago

Omitting the username or password also leads to a replicator failure loop.

ayymart commented 1 year ago

https://gerrit.readyset.name/c/readyset/+/5197

ayymart commented 1 year ago

Some discussion on whether readyset might ever want to deal with postgres upstream URLs that don't contain a database name:

https://readyset-workspace.slack.com/archives/C01HF1WTP6Y/p1686944393015599