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

DB: Truncate timestamps to microseconds #7075

Closed
wants to merge 2 commits into from
Closed

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Sep 8, 2023

Historically, go-mysql-driver rounded all timestamps to the nearest microsecond before forwarding them to the database connector. This is because MySQL only supports microsecond accuracy for timestamps, so the driver did the rounding first, to avoid sending unnecessary bits on the wire.

However, this led to a bug regarding double-rounding, detailed in go-sql-driver/mysql#1121. The fix for this bug was for go-mysql-driver to stop rounding timestamps entirely, favoring correctness over optimization, and allowing the database itself to handle rounding and truncation however it is configured to do so. This fix was landed in go-sql-driver/mysql#1172, which was included in the v1.6.0 release of that package (https://github.com/go-sql-driver/mysql/releases/tag/v1.6.0).

When we attempted to upgrade our version of go-mysql-driver to v1.6.0, we ran into significant performance problems which we failed to diagnose. The same happened over a year later, when we attempted to upgrade to v1.7.1. In the course of these investigations we found and fixed numerous other bugs, including upstreaming fixes to ProxySQL, but none of those seemed to alleviate the original issue. See the following bugs and PRs for the full timeline and context:

Finally, with the help of new deployment infrastructure, we bisected the whole set of go-mysql-driver commits between v1.5.0 and v1.7.1 to try to find the culprit. This revealed the timestamp-rounding commit described above as the first commit to cause performance issues, and finally made the explanation clear.

Our database has a significant quantity of timestamp data, all of which has been written to the database with truncated timestamps, thanks to go-sql-driver's historical behavior. When switching to a newer version, all of our SELECT queries which used time ranges were now querying for those time ranges with untruncated precision, and were therefore not using our existing indexes.

This obviously led to significant query performance issues, particularly on the fqdnSets, orderFqdnSets, and authz2 tables. These tables are very heavily queried with time ranges both for order and authorization reuse, and for rate limits. The fact that this issue only arises in the presence of significant historical data also explains why we could never reproduce it in the development environment or integration tests.

This change causes Boulder to do the time truncation that go-sql-driver used to do. This should allow our queries to continue using the existing indexes, and prevent performance regressions.

@aarongable
Copy link
Contributor Author

Upon further research, it doesn't appear that this approach is effective. The TypeConverter.ToDb method is only called by gorp/borp during Insert, Update, and Delete operations. It is not called to process the arbitrary arguments that may be supplied to Select or Exec statements.

To take this approach, we'd need a way to intercept and modify all args ...interface{} arguments supplied to any exported borp method. There's not a good place to do that, as some of the methods (such as QueryContext) quickly fall through to the underlying go stdlib database/sql methods with minimal processing of inputs or outputs.

@aarongable
Copy link
Contributor Author

Closing this PR as this doesn't seem to be a viable approach.

@aarongable aarongable closed this Sep 28, 2023
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.

None yet

1 participant