In MariaDB, `long_query_time`[1] and `max_statement_time`[2] have up to
microsecond granularity (6 digits to the right of the decimal).
Fixes an issue detected by proxysql in staging.
```
MySQL_Session.cpp:6567:handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo(): [ERROR] Unable to parse query. If correct, report it as a bug: SET long_query_time=3.9200000000000004
```
1. https://mariadb.com/kb/en/server-system-variables/#long_query_time
2. https://mariadb.com/kb/en/server-system-variables/#max_statement_time
---------
Co-authored-by: Aaron Gable <aaron@letsencrypt.org>
This change replaces [gorp] with [borp].
The changes consist of a mass renaming of the import and comments / doc
fixups, plus modifications of many call sites to provide a
context.Context everywhere, since gorp newly requires this (this was one
of the motivating factors for the borp fork).
This also refactors `github.com/letsencrypt/boulder/db.WrappedMap` and
`github.com/letsencrypt/boulder/db.Transaction` to not embed their
underlying gorp/borp objects, but to have them as plain fields. This
ensures that we can only call methods on them that are specifically
implemented in `github.com/letsencrypt/boulder/db`, so we don't miss
wrapping any. This required introducing a `NewWrappedMap` method along
with accessors `SQLDb()` and `BorpDB()` to get at the internal fields
during metrics and logging setup.
Fixes#6944
Previously, we had three chained calls initializing a database:
- InitWrappedDb calls NewDbMap
- NewDbMap calls NewDbMapFromConfig
Since all three are exporetd, this left me wondering when to call one vs
the others.
It turns out that NewDbMap is only called from tests, so I renamed it to
DBMapForTest to make that clear.
NewDbMapFromConfig is only called internally to the SA, so I made it
unexported it as newDbMapFromMysqlConfig.
Also, I copied the ParseDSN call into InitWrappedDb, so it doesn't need
to call DBMapForTest. Now InitWrappedDb and DBMapForTest both
independently call newDbMapFromMysqlConfig.
I also noticed that InitDBMetrics was only called internally so I
unexported it.
Adds a new function to the `//sa` to ensure that a MariaDB config passed
in via SA `setDefault` or via DSN perform the following validations:
1. Correct quoting for strings and string enums to prevent future
problems such as PR #6683 from occurring.
2. Each system variable we care to use is scoped as SESSION, rather than
strictly GLOBAL.
3. Detect system variables passed in that are not in a curated list of
variables we care about.
4. Validate that values for booleans, floats, integers, and strings at
least pass basic a regex.
This change is in a bit of a weird place. The ideal place for this
change would be `go-sql-driver/mysql`, but since that driver handles the
general case of MySQL-compatible connections to the database, we're
implementing this validation in Boulder instead. We're confident about
the specific versions of MariaDB running in staging/prod and that the
database vendor won't change underneath us, which is why I decided to
take this approach. However, this change will bind us tighter to MariaDB
than MySQL due to the specific variables we're checking. An up-to-date
list of MariaDB system variables can be found
[here.](https://mariadb.com/kb/en/server-system-variables/)
Fixes https://github.com/letsencrypt/boulder/issues/6687.
Add an upstream ProxySQL container to our docker-compose. Configure
ProxySQL to manage database connections for our unit and integration
tests.
Fixes#5873
When sql_mode is set as part of a multi-variable SET command (which
happens in go-sql-driver/mysql 1.6.0+), ProxySQL can mis-parse parts of
the SET command that come after it. For instance, if we run:
SET sql_mode=STRICT_ALL_TABLES,log_queries_not_using_indexes=ON;
Then ProxySQL would mis-parse that and pass along to its upstream:
SET sql_mode=STRICT_ALL_TABLES,log_queries_not_using_indexes;
Adding quotes around sql_mode (a string-valued variables) causes
ProxySQL to parse this correctly.
- Replace `-1` in return values with `0`. No callers were depending on
`-1`.
- Replace `count(` with `COUNT(` for the sake of readability.
- Replace `COUNT(1)` with `COUNT(*)` (https://mariadb.com/kb/en/count).
Both
versions provide identical outputs but let's standardize on the docs.
Fixes#6494
Historically the only database/sql driver setting exposed via JSON
config was maxDBConns. This change adds support for maxIdleConns,
connMaxLifetime, connMaxIdleTime, and renames maxDBConns to
maxOpenConns. The addition of these settings will give our SRE team a
convenient method for tuning the reuse/closure of database connections.
A new struct, DBSettings, has been added to SA. The struct, and each of
it's fields has been commented.
All new fields have been plumbed through to the relevant Boulder
components and exported as Prometheus metrics. Tests have been
added/modified to ensure that the fields are being set. There should be
no loss in coverage
Deployability concerns for the migration from maxDBConns to maxOpenConns
have been addressed with the temporary addition of the helper method
cmd.DBConfig.GetMaxOpenConns(). This method can be removed once
test/config is defaulted to using maxOpenConns. Relevant sections of the
code have TODOs added that link back to an newly opened issue.
Fixes#5199
This copies over a number of features flags and other settings from
test/config-next that have been applied in prod.
Also, remove the config-next gate on various tests.
* SA: add unit test for auto_increment schemas.
`TestAutoIncrementSchema` uses a root user connection to the
`information_schema` MariaDB database to try and find table columns from
the Boulder schemas that are both `auto_increment` and not `int64`.
* SA: rename _db-next RemoveOCSPResponses.sql migration.
Based on the order that we apply migrations the
`RemoveOCSPResponses.sql` migration with its old prefix
(`20181101105733`) was never being applied. That in turn caused the new
`TestAutoIncrementSchema` unit test to fail because the old
`ocspResponses` table has an `id` field that is `auto_increment` but
`sized `int(11)`.
Renaming the migration with a newer prefix solves the problem. The
`ocspResponses` table ends up dropped when `config-next` is used.
Afterwards the `TestAutoIncrementSchema` unit test passes again.
This uses the mysql driver library's capability to use `SET` to set the system
variables that prefixdb previously was.
Unfortunately, the library doesn't sort the params when making the string, so we
have to do a little munging to TestNewDbMap.
Ran it in a checkout of the repo since godeps now doesn't include the test files (which is great!).
```
MYSQL_TEST_ADDR=127.0.0.1:3306 go test .
ok github.com/go-sql-driver/mysql 46.099s
```
If the feature flag "ReusePendingAuthz" is enabled, a request to create a new authorization object from an account that already has a pending authorization object for the same identifier will return the already-existing authorization object. This should make it less common for people to get stuck in the "too many pending authorizations" state, and reduce DB storage growth.
Fixes#2768
This means that we get a more useful log message for slow queries, and don't
need to close the MySQL connection. It also means that the query is actually
killed on the MySQL side, rather than just timing out and returning on the
client side.
We set the max_statement_time to 95% of the `readTimeout`.
Also set the `long_query_time` to 80% of the `readTimeout`, so that queries that are
close to timing out will be logged by MySQL's slow query logging.
- Move dbMap construction and type converter into individual files in the sa package.
- Add DB configuration for the OCSP tool to the boulder config:
- left to the user if they want to use different boulder-config.json files
for different purposes.
- Added updater to Makefile
- Fix trailing ',' in the Boulder config, add more panic logging
- Ignore .pem files produced by the integration test
- Change RPC to use per-instance named reply-to queues.
- Finish OCSP Updater logic
- Rework RPC for OCSP to use a transfer object (due to serialization problems of x509.Certificate)
Phil Porada
Jacob Hoffman-Andrews
Samantha
Jacob Hoffman-Andrews
Daniel McCarney
Roland Bracewell Shoemaker
Jeff Hodges
Igor Bukanov
Kane York
Richard Barnes
J.C. Jones