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

errno, infoschema, executor, server: add client error infoschema tables #20785

Closed
wants to merge 7 commits into from
Closed

errno, infoschema, executor, server: add client error infoschema tables #20785

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 3, 2020

What problem does this PR solve?

Issue Number: Fixes #14433

Problem Summary:

This adds support for information_schema.client_errors_summary_* tables. It addresses a gap in metrics where it might be a client that is causing an error. Potential issues that this could help find are:

  • Out of date client library.
  • Out of date application (forgot to upgrade one server).
  • Change in configuration (client was not expecting strict sql mode).
  • Connectivity issues (client of one particular host receiving errors).

Client errors are different from server errors. The server should typically log when it encounters something unexpected or weird about its operation. But in this case, the server is probably operating as expected, it's the client that is behaving strange.

What is changed and how it works?

Three new information schema tables have been introduced:

  • client_errors_summary_global
  • client_errors_summary_by_host
  • client_errors_summary_by_user

The design is modeled loosely based on what MySQL 8.0 has in performance_schema. But there are the following differences to be aware of:

  • In MySQL 8.0, reseting the stats is done with a TRUNCATE TABLE command. But since these are in infoschema in TiDB, a command FLUSH CLIENT_ERRORS_SUMMARY is added.
  • In MySQL there is always a row for each error code (regardless if any errors or warnings have been generated). I thought could be misleading, since if it showed all errors in the errno package - some are known not to be generated. Also, it creates a very large table if there are a lot of users or hosts.
  • In TiDB it shows the ERROR_MESSAGE (in sprintf format), not the ERROR_NAME. This is a current limitation based on what is stored in the errno` package. I think it's fine.
  • There is no ERROR_RAISED / ERROR_HANDLED counts, and the columns are just renamed to ERROR_COUNT. TiDB does not have stored procs, and thus no error handling.

How it Works:

The errors and warnings could be generated anywhere in code. I capture them not at generate time, but as they are sent to the client. This means that internal sql execution that triggers warnings etc. is not logged.

Related changes

Check List

Tests

  • Unit test

Side effects

  • Should be minimal performance impact since it only needs to acquire the mutex briefly when there is a statement that has caused an error or warning. I checked with tiup tpcc bench to see how many client errors are generated, and it is only 1 during prepare.

Release note

  • A set of client_errors_summary tables has been added to Information Schema. This helps keep track of which errors have been sent to clients.

@github-actions github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution labels Nov 3, 2020
@ghost ghost marked this pull request as ready for review November 9, 2020 19:44
@ghost ghost requested review from a team as code owners November 9, 2020 19:44
@ghost ghost requested review from lzmhhh123 and removed request for a team November 9, 2020 19:44
@ghost
Copy link
Author

ghost commented Nov 9, 2020

The PR currently includes the changes in #20917 - once this merges (and the parser PR merges) the changeset will look a little cleaner. Fixed.

@ghost ghost requested a review from djshow832 November 9, 2020 19:58
@ghost ghost added the status/PTAL label Nov 9, 2020
@ghost ghost requested a review from breezewish November 9, 2020 19:59
@ti-srebot
Copy link
Contributor

@breeswish, @lzmhhh123, @djshow832, PTAL.

2 similar comments
@ti-srebot
Copy link
Contributor

@breeswish, @lzmhhh123, @djshow832, PTAL.

@ti-srebot
Copy link
Contributor

@breeswish, @lzmhhh123, @djshow832, PTAL.

@ghost ghost mentioned this pull request Nov 19, 2020
@ghost
Copy link
Author

ghost commented Dec 15, 2020

We can revisit this for sprint 2.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect and expose client errors/warnings as a table
1 participant