MDEV-39405: store the necessary plugin-engines optimizer costs#5077
Conversation
55e4d81 to
c0d48c6
Compare
There was a problem hiding this comment.
Code Review
This pull request enhances the optimizer context replay mechanism by capturing and storing storage engine-specific optimizer costs. It introduces logic to identify relevant storage engines for a query and generates the corresponding global configuration statements. The review feedback correctly identifies several potential memory leaks where hashes are not freed during early returns and suggests optimizations for the cost-storing logic to improve efficiency and safety.
c0d48c6 to
c77a130
Compare
| res= true; // OOM | ||
| } | ||
|
|
||
| if (!res && thd->variables.table_plugin) |
There was a problem hiding this comment.
I had to look up what is this. It seems to be the plugin for @@default_storage_engine.
Any reason why we need it in the context?
I assume we need plugins for the tables we're querying, plugin for in-memory temp.table, plugin for on-disk temp.table.
There was a problem hiding this comment.
Yes, this should be cleaned.
| if (!res && thd->variables.table_plugin) | ||
| { | ||
| const LEX_CSTRING *engine_name= plugin_name(thd->variables.table_plugin); | ||
| DB_NAME_KEY *engine_name_key; |
There was a problem hiding this comment.
What's the point of using DB_NAME_KEY here? It looks confusing.
Why not LEX_STRING?
There was a problem hiding this comment.
yes, used LEX_CSTRING. Also removed TABLE_NAME_KEY, and DB_NAME_KEY, and instead using LEX_CSTRING there as well.
|
When we're looking at the query's tables: We can get the plugin name, table's hton and hton->optimizer_costs. Question: why do we need to iterate plugins separately and compare name to see if the plugin was used, etc? |
create table t20 (a int, b int) engine=rocksdb partition by hash(a) partitions 2;
set optimizer_record_context=1;
select * from t20 where a=333;
select * from information_schema.optimizer_context\Gno ROCKSDB.OPTIMIZER_DISK_READ_COST here... create table t21 (a int, b int) engine=rocksdb;
set optimizer_record_context=1;
select * from t21 where a=333;
select * from information_schema.optimizer_context\GNow I can see: SET GLOBAL ROCKSDB.OPTIMIZER_DISK_READ_COST=10.24 Check handler->partition_ht(). |
c77a130 to
8d9199e
Compare
store the necessary plugin-engines optimizer costs that are used by the query into the context, so that the replay server uses the same while computing query cost
8d9199e to
f9cc1a9
Compare
Yes, using handler->partition_ht() is giving the underlying storage engine name for both partitioned and non-partitioned tables. |
store the necessary plugin-engines optimizer costs that are used by the query into the context, so that the replay server uses the same while computing query cost