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

we should add extra info to inform that a query plan is from plan cache #16127

Closed
ChenPeng2013 opened this issue Apr 7, 2020 · 10 comments · Fixed by #16321
Closed

we should add extra info to inform that a query plan is from plan cache #16127

ChenPeng2013 opened this issue Apr 7, 2020 · 10 comments · Fixed by #16321
Assignees
Labels
epic/plan-cache feature/accepted This feature request is accepted by product managers sig/planner SIG: Planner type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@ChenPeng2013
Copy link
Contributor

ChenPeng2013 commented Apr 7, 2020

Feature Request

Is your feature request related to a problem? Please describe:

prepared-plan-cache.enabled=true;

use test;
create table t(a int);
prepare stmt from "select * from t";
select connection_id();
execute stmt;
# explain for connection id;

I don't know whether the last sql's plan is from plan cache.

Describe the feature you'd like:

mysql> explain for connection 1;
+-----------------------+---------+-----------+---------------+--------------------------------+
| id | estRows | task | access object | operator info |
+-----------------------+---------+-----------+---------------+--------------------------------+
| TableReader_5 | 3.00 | root | | data:TableFullScan_4, plan cache: true |
| └─TableFullScan_4 | 3.00 | cop[tikv] | table:t | keep order:false, stats:pseudo |
+-----------------------+---------+-----------+---------------+--------------------------------+
2 rows in set (0.01 sec)

@ChenPeng2013 ChenPeng2013 added the type/feature-request Categorizes issue or PR as related to a new feature. label Apr 7, 2020
@zz-jason
Copy link
Member

zz-jason commented Apr 7, 2020

It's a little tricky to add plan cache: true to the result of explain. How about adding a NOTE information after execute stmt;?

Through this method, users can tell whether the executed statement benefits from the plan cache via show warnings.

It's something like this:

mysql> execute stmt;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> SHOW WARNINGS;
+-------+------+------------------------------------+
| Level | Code | Message                            |
+-------+------+------------------------------------+
| Note  | 1051 | Hit plan cache: True               |
+-------+------+------------------------------------+
1 row in set (0.00 sec)

@wwar
Copy link

wwar commented Apr 8, 2020

It's a little tricky to add plan cache: true to the result of explain. How about adding a NOTE information after execute stmt;?

I think that this will lead to applications ignoring warnings because 1051 is a generic error. i.e. they can not check by error code safely, and it is usually unwise to check by the Message string. I also don't know of a better error code.

If it can't be shown in EXPLAIN, could it be added to optimizer trace? #9243

@zz-jason
Copy link
Member

zz-jason commented Apr 8, 2020

I think that this will lead to applications ignoring warnings because 1051 is a generic error. i.e. they can not check by error code safely, and it is usually unwise to check by the Message string. I also don't know of a better error code.

How about using another unused error code reserved for the planner package?

Typically, when the application meets the performance issue, they may need to check the plan cache hit ratio. For now, we can do this via the grafana monitor. Or we can record whether a SQL used the plan from the cache in the slow log.

For a single query executed by a user, he/she may be curious about whether this statement hits the cache. I prefer a "Note" level warning comparted with modifying the execution plan returned by explain for connection.

Another advantage of this way is it's easy for the test framework. We can simply check the warning message, no need to parse the execution plan returned by explain for connection.

could it be added to optimizer trace? #9243

Yes, I think we should consider plan cache in optimizer trace. cc @winoros

@wwar
Copy link

wwar commented Apr 9, 2020

How about using another unused error code reserved for the planner package?

I think it's an improvement, but could still break compatibility for applications that are not expecting this. This is a MySQL protocol problem. There are warnings and errors, but there is not really an elegant way to send purely debug 'notices' to applications.

I have a couple of other suggestions:

  1. Keep this behavior, but hide it behind a "tidb_enable_*" flag.
  2. Populate a session variable to indicate if the planner cache was used. There is some prior art in MySQL with this, for example error_count and last_insert_id.

Typically, when the application meets the performance issue, they may need to check the plan cache hit ratio. For now, we can do this via the grafana monitor. Or we can record whether a SQL used the plan from the cache in the slow log.

Yeah, I agree! Please also include a counter in information_schema.statements_summary (to be renamed this in #16188 ). This will help with observing the ratio in real time. Maybe it could also show when the plan was last prepared?

@zz-jason
Copy link
Member

zz-jason commented Apr 9, 2020

keep this behavior, but hide it behind a "tidb_enable_*" flag.

I'm sorry, I can not understand the detailed behavior of this method. What does hide it behind a "tidb_enable_*" flag mean?

Populate a session variable to indicate if the planner cache was used. There is some prior art in MySQL with this, for example error_count and last_insert_id.

This way is better than the warning message 👍. Do you have any suggestions about the variable name? How about hit_plan_cache?

Please also include a counter in information_schema.statements_summary (to be renamed this in #16188 ). This will help with observing the ratio in real time.

Sure, it's indeed necessary to record the hit counter or ratio in information_schema.statements_summary table. I've recorded this suggestion in #16217.

Maybe it could also show when the plan was last prepared?

Do you mean to show whether the prepared statement is put into the plan cache?

@zz-jason
Copy link
Member

zz-jason commented Apr 9, 2020

SQL Plan Binding may also have the same issue: how to intuitively observe whether a SQL used the session or global level binding? For now, users have to check whether the execution plan is matched with the hints in the binding. But this method requires the user to be very similar with the optimizer and SQL hint behaviors.

Considered that the binding creator and the binding user maybe is not the same person. For example, binding creator may be the database administrator while the user may be the application developer.

Like plan cache, it's better to display this information in:

  • Grafana monitor, already have
  • statement summary table
  • slow log

@wwar
Copy link

wwar commented Apr 9, 2020

keep this behavior, but hide it behind a "tidb_enable_*" flag.

I'm sorry, I can not understand the detailed behavior of this method. What does hide it behind a "tidb_enable_*" flag mean?

I meant add another one of these:

mysql> SHOW SESSION VARIABLES LIKE 'tidb_enable%';
+-----------------------------------+-------+
| Variable_name                     | Value |
+-----------------------------------+-------+
| tidb_enable_cascades_planner      | 0     |
| tidb_enable_chunk_rpc             | 1     |
| tidb_enable_fast_analyze          | 0     |
| tidb_enable_index_merge           | 0     |
| tidb_enable_noop_functions        | 0     |
| tidb_enable_radix_join            | 0     |
| tidb_enable_slow_log              | 1     |
| tidb_enable_stmt_summary          |       |
| tidb_enable_streaming             | 0     |
| tidb_enable_table_partition       | auto  |
| tidb_enable_vectorized_expression | 1     |
| tidb_enable_window_function       | 1     |
+-----------------------------------+-------+
12 rows in set (0.00 sec)

Populate a session variable to indicate if the planner cache was used. There is some prior art in MySQL with this, for example error_count and last_insert_id.

This way is better than the warning message . Do you have any suggestions about the variable name? How about hit_plan_cache?

May I suggest last_statement_found_in_plan_cache?

I like the wording "last_statement". It is similar to "last_insert_id", but also "last_statement" is the name that MySQL sys schema uses in views like sys.session.

Maybe it could also show when the plan was last prepared?

Do you mean to show whether the prepared statement is put into the plan cache?

I will comment in #16217

@zz-jason
Copy link
Member

zz-jason commented Apr 9, 2020

May I suggest last_statement_found_in_plan_cache?

I like the wording "last_statement". It is similar to "last_insert_id", but also "last_statement" is the name that MySQL sys schema uses in views like sys.session.

This method and the variable name LGTM. @eurekaka PTAL

@danmay319
Copy link
Contributor

danmay319 commented Apr 15, 2020

Shall we raise a warning when users try to execute set @@last_statement_found_in_plan_cache = 1? The set operator used for most system variables seems totally useless here and may cause some puzzles for users. @wwar

@ChenPeng2013
Copy link
Contributor Author

added many automation ticases about last_plan_from_cache
release-4.0: v4.0.0-rc.1-64-gf77e0ced7 verified by TICASE-1929
master: v4.0.0-beta.2-441-g36a29da52 verified by TICASE-1929

@zz-jason zz-jason added the feature/accepted This feature request is accepted by product managers label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic/plan-cache feature/accepted This feature request is accepted by product managers sig/planner SIG: Planner type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
4 participants