[MDEV-4581] QUERY CACHE - ADD more columns in QC_INFO plugin Created: 2013-05-25  Updated: 2015-05-21  Resolved: 2014-10-09

Status: Closed
Project: MariaDB Server
Component/s: Plugins
Fix Version/s: N/A

Type: Task Priority: Major
Reporter: roberto spadim Assignee: Sergei Golubchik
Resolution: Duplicate Votes: 0
Labels: plugins, qc_info, querycache

Attachments: Text File diff_10_0_3_qc_info.cc    
Issue Links:
Blocks
blocks MDEV-4584 QUERY CACHE - SQL Control Interface Open
PartOf
is part of MDEV-4682 QUERY CACHE - add STATISTICS per quer... Closed

 Description   

new fields (flags) to qc_info plugin



 Comments   
Comment by roberto spadim [ 2013-05-26 ]

now last version...
some fields maybe should be human readable but idon't know how to do it, like:
COLUMN_FLAGS_CHARACTER_SET_CLIENT_NUM,
COLUMN_FLAGS_CHARACTER_SET_RESULTS_NUM,
COLUMN_FLAGS_COLLATION_CONNECTION_NUM,
COLUMN_FLAGS_SQL_MODE,
COLUMN_FLAGS_DEFAULT_WEEK_FORMAT

and
just two flags that i don't understand how to show it : LC_TIME_NAMES and TIME_ZONE

but, work is done, please close this mdev, and if possiblem, submit patch to review =)

thanks!

Comment by roberto spadim [ 2013-05-26 ]

ops, i done tab->space conversion, patch was a little big, ops
now patch without tab->space, sorry many files upload i will learn how to use launchpad

Comment by roberto spadim [ 2013-05-26 ]

insert date information in unixtimestamp

Comment by roberto spadim [ 2013-05-26 ]

mariadb 10.0.2 patch

Comment by roberto spadim [ 2013-05-26 ]

sergei i don't know how to get the rows sent to user, on some select * from xxx order by xxx limit 1, the column "RESULT_FOUND_ROWS" sometimes return a very big number ?!
is the header->found_rows(limit_found_rows) wrong?
i will try an test case to reproduce it better

Comment by roberto spadim [ 2013-05-27 ]

hummm found...
result found rows = rows that should be returned if remove limit xxx part of select
well i add a new column with sent rows value

mariadb 10.0.2

Comment by roberto spadim [ 2013-05-28 ]

add some more columns
time expent in qc, hit last time, and changed miliseconds to microseconds

Comment by roberto spadim [ 2013-05-28 ]

SELECT
SUM((query_hits_total_time_us/query_hits))/1000000 qc_time,
SUM(SELECT_EXPEND_TIME_US-(query_hits_total_time_us/query_hits))*query_hits/1000000 as qc_won_time,
SUM(SELECT_EXPEND_TIME_US)/1000000 as expend_time
FROM query_cache_queries
WHERE query_hits>0

first 60 minutes of query cache, give me a faster response
qc_time=0,0242322356 seconds (1st column)
qc_win_time=6265,3803996478 seconds (2nd column)
expend_time=31,19528 seconds (3rd column)

now i know if qc is a good thing to me or not =]

now i will focus on MDEV-4582, and MDEV-4589, to get a better query cache usage (some fast disk queries don't need qc)
todo: nicer results for flags (sql_mode and others columns)
thanks guys

Comment by roberto spadim [ 2013-06-03 ]

5.5 patch is a bit old, must change it like 10.0.2

Comment by roberto spadim [ 2013-06-03 ]

the query cache time, should be changed to:
query cache found - start of query process
instead of
query cache found - start of query cache search function

Comment by roberto spadim [ 2013-06-05 ]

end =)
all fields are now human readable (only protocol that may be 0=text, 1=binary, but i don't found a function or anything like it to convert protocol number to protocol string)

Comment by roberto spadim [ 2013-06-05 ]

i removed the 5.5 version since 10.0.2 is newer and i'm using it without big problems in production machines =)

Comment by roberto spadim [ 2013-06-09 ]

could be release with 10.0.3?

Comment by roberto spadim [ 2013-06-11 ]

well, after 10.0.3 i will rewrite the patch

Comment by roberto spadim [ 2013-06-12 ]

mariadb 10.0.3 patch

diff -u mariadb-original/sql/sql_cache.cc mariadb-10.0.3/sql/sql_cache.cc > diff_10_0_3_sql_cache.cc

diff -u mariadb-original/sql/sql_cache.h mariadb-10.0.3/sql/sql_cache.h > diff_10_0_3_sql_cache.h

diff -u mariadb-original/plugin/qc_info/qc_info.cc mariadb-10.0.3/plugin/qc_info/qc_info.cc > diff_10_0_3_qc_info.cc

Comment by roberto spadim [ 2013-06-13 ]

i done a production use test with this patch and mariadb 10.0.3 (in production use too), and this mdev nice to be closed, could you (mariadb) test and close it?

Comment by roberto spadim [ 2013-06-17 ]

at this line in qc_info patch:
+ table->field[COLUMN_RESULT_TABLES_TYPE]>store(query_cache_query>tables_type(), 0); /* what is this ? */

what ->table_types() means? it's something like MyISAM/ARIA/INNODB?

Comment by roberto spadim [ 2013-06-17 ]

added table type to QUERY_CACHE_TABLES

Comment by roberto spadim [ 2013-06-17 ]

found at handler.h

/* Table caching type */
#define HA_CACHE_TBL_NONTRANSACT 0
#define HA_CACHE_TBL_NOCACHE 1
#define HA_CACHE_TBL_ASKTRANSACT 2
#define HA_CACHE_TBL_TRANSACT 4

and at sql_cache.cc
*tables_type|= HA_CACHE_TBL_NONTRANSACT; (view)
*tables_type|= tables_used->table->file->table_cache_type(); (non view)

my doubt now is about query tables_type, and not table_type of table
it's like a binary value, right?
0b0000abcd
a=HA_CACHE_TBL_TRANSACT (maybe query cache of one begin/commit transaction or xa transaction???)
b=HA_CACHE_TBL_ASKTRANSACT (innodb/xtradb an others)
c=HA_CACHE_TBL_NOCACHE (temporary, system or other non-cacheable table(s), (never used for query in cache))
d=HA_CACHE_TBL_NONTRANSACT (why 0 ??? it's always HA_CACHE_TBL_NONTRANSACT ??? myisam aria and others?)

in ha_partition.cc
/*
We need save underlying tables only for HA_CACHE_TBL_ASKTRANSACT:
HA_CACHE_TBL_NONTRANSACT - because all changes goes through partition table
HA_CACHE_TBL_NOCACHE - because will not be cached
HA_CACHE_TBL_TRANSACT - QC need to know that such type present
*/

Comment by roberto spadim [ 2013-06-17 ]

added tables_type, bug-overflow in max length with strncat

Comment by roberto spadim [ 2013-06-17 ]

add mean period between hits with a outlier of 1 minute (QUERY_CACHE_QC_INFO_PERIOD_OUTLIER_US), it's better to see if a query is being hitted or not than hit counter and last hit time

maybe in future add more granularity 10 seconds mean period, 30 seconds, 60 seconds, etc, example...
if(period<10) hit_10_seconds_period+; hit_10_seconds_time=period;
if(period<30) hit_30_seconds_period+; hit_30_seconds_time=period;
if(period<60) hit_30_seconds_period+; hit_60_seconds_time=period;
this make outliers more specific and easier to understand what kind of query it is (i'm thinking about how to prune and remove queries instead of LRU or other remove algorithm)

but i didn't tested if it's memory intensive (i'm using longlong...) just a <60 period is nice for now (i think)

1)does feedback plugin, give some information about max query hits? or something about query cache (just global STATUS)? maybe this could be a nice source of information to chose right memory type for qc_info informations
2)is the sql_cache.h file the better place to put functions? or i should put them in .cc file? for example... when i change sql_cache.h many files are recompiled, i'm thinking about putting qc_info functions inside .cc file
2.a) about compiler... when i'm using inline, shouldn't it include it inside sql_cache.cc? or compiler know that the file was changed and change all files that use sql_cache.h?

Comment by roberto spadim [ 2013-06-17 ]

hum... running 10 minutes in production server...
i found an interesting information... and i'm thinking reviewing the granularity... tell me what you think (or if better where i should ask this question, mariadb develop mail list?)

maybe should consider:
1) mean period between hits lower than expend time (cache have good hit rate)
2) mean period between hits higher than expend time (cache can be removed)
3) global mean period between hits (today implemented)

i'm seeing many queries that could be removed because the hit period is bigger than the expend time
in other words... if my query cache remove this query i will have near no user reporting 'system is slow now'

maybe when a query cache clean method could be developed (MDEV-4584 and MDEV-4583), we could consider a algorithm to remove queries with mean period between hits higher than expend time, and after others queries

Comment by Jean Weisbuch [ 2013-06-18 ]

(It might be a little off-topic but i dont think that it would deserve a MDEV on its own and you already opened enough trasks on the QC, feel free to tell me if i did put that in the wrong one)

Another idea would be to add a query cache settings (query_cache_type=3 for example) to only cache queries that involved filesort and non-indexed rows retreival.
And adding a new variable that would set a minimal number of rows examined to be put in cache, for example a "query_cache_min_examined_rows".

With these options, only long/expensive queries would be put in cache, it surely wouldnt solve the cache cleaning but it could limit the number of entries in the cache and ease the cleanup process.

Comment by roberto spadim [ 2013-06-18 ]

nice jean, i think MDEV-4582 is the right place to put your comment and my one i will put it in MDEV-4583 to don't forget about it

Comment by roberto spadim [ 2013-06-18 ]

well i think that's my last patch and we can close this MDEV

changed the outlier... now use the expend time, and a outlier coefient (today = 10) and a minimal period time (today 1second)
if period is more than query_expend time it is a high_hit, if it's less and query_expend time its a low_hit, if period is bigger than 1 second and period is bigger than 10*expend time, it's a outlier (not usefull statistic period), in other words, (period>1 seconds, and period>10*expend time) = outlier hit

with this we can know if a hit is being executed very fast (lower than expend time), if it's a normal hit.. (higher than expend time) or if it's a sometimes hit (outlier). normal hit when removed from cache normally don't give a user impression of "hey the system is slow now"

more than this, i think, only with a query log to know if a query is being hitted or not, and when where why, etc...

this is enought to know about query cache and start development of query cache control, prune and low mem procedures (check MDEV with 'querycache' label)

sql_cache.h is nice documented (with comments) - i don't know how to change the functions of qc_info plugin inside .h file to .cc file, this help a lot when compiling, since a change in sql_cache.h change many files in compile time...

sql_cache.cc have about 3 changes (start qc_info plugin information, save expend time, hit query) and some time variables (to get expend time)

qc_info.cc should be better documented... but source code is enought for now... (i think)
i added two tables, one with flags, and other without flags, it's easier to see
DOUBTS:
1)there's a way to send a parameter to fill table function, that remove or not remove some fields? instead of two functions i could have only one...
2)could we use WHERE condition to only return queries with table = XXX, intead of reading all queries and consuming a lot of memory and time?

inside qc_info.cc:
i checked some buffer overflow conditions in qc_info.cc and changed the strncat function to remove this bugs
i didn't found others bugs, so please if anyone have a nice feature to check about common bugs (valgrid do this?) in memory overflow or something like this could be nice check to be sure that we will have no (idiots) problems

thanks, that's for now =]

Comment by roberto spadim [ 2013-06-18 ]

using
select * from information_schema.query_cache_queries_no_flags

i think that _no_flags could be the default table, and a _with_flags could be a second table, i'm using query_cache_queries_no_flags more than with flags table

change order of QUERY_HITS_PERIOD_OUTLIER after QUERY_HITS
(this help know how many hits was outliers, and remove it easily if we can see both , without rewriting select (hits-outliers,, outliers/hits) )

change order of RESULT_FOUND_ROWS after QUERY_ROWS

maybe, change order of QUERY_HITS_TOTAL_TIME_US after QUERY_HITS_MEAN_PERIOD_HIGH_US, others users could help here saying, please put it here instead of here...

Comment by Sergei Golubchik [ 2013-06-19 ]

Roberto, under what license did you provide these patches to us?
I suppose, it's BSD, because you put PLUGIN_LICENSE_BSD for your new plugins.
But please say it explicitly, if that's the case.

Comment by Sergei Golubchik [ 2013-06-19 ]

Comments about the patch.
1. I'll apply it first without any columns that need patching the server. So, no sql_cache.* changes. May be later, as a second step, I'll do them too, but first let's have a clean plugin with no server changes.
2. Why do you want "ENTRY_POSITION_IN_CACHE" column? Why would it be useful to anyone? I'd rather remove it, if it's not useful for the user.
3. Detail, I don't particularly like "HA_CACHE_TBL_NONTRANSACT" etc strings in the user-visible output, I'll replace them with something less obscure.
4. I don't like the comma separated list of tables in the QUERY_CACHE_QUERIES table. Better to include some kind of the query id in both QUERIES and TABLES table (for example, ENTRY_POSITION_IN_CACHE of a query) and let the user join on that, if desired.
5. there's no need for a "NOFLAGS" version of the table, a user can select desired columns explicitly or create a view.

Otherwise it was pretty much fine.

Basically, I can apply your patch anytime, assuming you're ok with the above changes.

Comment by roberto spadim [ 2013-06-19 ]

hi sergei, after your work done, could you send me something about what's wrong or not? i will rewrite the qc_info.cc to use only one function to fill table
i will put 3 tables:
query_cache_queries - the default columns implementation
query_cache_queries_stats - with stats
query_cache_queries_flags - with stats + flags

Comment by roberto spadim [ 2013-06-19 ]

>Sergei Golubchik added a comment - Today 07:46
>Roberto, under what license did you provide these patches to us?
>I suppose, it's BSD, because you put PLUGIN_LICENSE_BSD for your new plugins.
>But please say it explicitly, if that's the case.
you can use anyone, let's use the same of Roland Bouman, the first plugin coder

>Comments about the patch.
>1. I'll apply it first without any columns that need patching the server. So, no sql_cache.* changes. May be later, as a second step, I'll do them too, but first let's have a clean plugin with no server changes.
nice, i will put a #IFDEF QUERY_CACHE_QC_PLUGIN inside my qc_plugin.cc to check if we have QC_PLUGIN inside qc_cache.cc/qc_cache.h ok?

>2. Why do you want "ENTRY_POSITION_IN_CACHE" column? Why would it be useful to anyone? I'd rather remove it, if it's not useful for the user.
well i'm thinking about this MDEV-4584, when we will DELETE the query inside query cache, we will use the ENTRY_POSITION
i didn't thinked about a small name, maybe just QC_ID could be nice to me...

>3. Detail, I don't particularly like "HA_CACHE_TBL_NONTRANSACT" etc strings in the user-visible output, I'll replace them with something less obscure.
yes, i don't like it too, i will put it only inside query cache with flags table, the NONTRANSACT is always displayed :/ maybe we could change it to 1, but it will change many things inside mysql code
something less obscure is nice, like "NON_TRANSACTIONAL", "TRANSACTIONAL", etc

>4. I don't like the comma separated list of tables in the QUERY_CACHE_QUERIES table. Better to include some kind of the query id in both QUERIES and TABLES table (for example, ENTRY_POSITION_IN_CACHE of a query) and let the user join on that, if desired.
hum the problem is the time between qc_table and qc_queries tables, maybe in the mean time a table could be removed, i put the table inside query cache table to easly show what table is bein used, maybe it should go only to stats table and flag table

>5. there's no need for a "NOFLAGS" version of the table, a user can select desired columns explicitly or create a view.
yes but, it's some much information, a less information version is nice and save more memory (think about a 100.000 rows table with 40columns, and a 100.000 rows with 10columns, this save a lot of time in temporary table creation)

>Otherwise it was pretty much fine.
thanks i really liked this qc_plugin, i like cache features

>Basically, I can apply your patch anytime, assuming you're ok with the above changes.
hum, i'm changing something i'm attaching the raw version for now about the news features (it have some errors to compile, but it can see the code)
when done i will upload again

thanks!

Comment by roberto spadim [ 2013-06-19 ]

uncomplete qc_info.cc with #ifdef to add (or not) the stats/flags table

Comment by Sergei Golubchik [ 2013-06-19 ]

Hi, Roberto!

I don't understand what you mean by that.

Right, but I mean, ENTRY_POSITION_IN_CACHE in the TABLES table.
Why would you need a table id like that?

Yes, this is a fundamental issue, that will go away when we have an
information_schema storage engine.

meanwhile one has to live with it, it's documented that different I_S
tables may not present a consistent view of the system at some specific
point in time.

anyway, the worse that can happen, when one joins QUERIES and TABLES
tables, one can get a query having no tables. not a crash or anything.

yes. and SQL way of solving it is to specify desired columns explicitly.
or to create a view. not to create a second table which is a copy of the
first, but with less columns.

Eh, fine. If you haven't finished working on it, I'll wait.

But when you say "please review", I assume that the code is final, and I
will not accept any new changes. If you will want to add more features
to this plugin after you said "please review", you will need to create a
new jira task with a new contribution. Ok?

So, now, I'll stop looking at this plugin and will wait until you're
done. And, again, when you say you're done - no more changes.

Regards,
Sergei

Comment by roberto spadim [ 2013-06-19 ]

>Hi, Roberto!
>I don't understand what you mean by that.
>Right, but I mean, ENTRY_POSITION_IN_CACHE in the TABLES table.
>Why would you need a table id like that?
i will remove it. i don't know why i put it there, maybe just copy and paste the code sorry

>Yes, this is a fundamental issue, that will go away when we have an
>information_schema storage engine.
>meanwhile one has to live with it, it's documented that different I_S
>tables may not present a consistent view of the system at some specific
>point in time.
nice, well with the LOCK QUERY CACHE, this will not be a problem... (when we have the query cache control MDEV-4584 done)

>anyway, the worse that can happen, when one joins QUERIES and TABLES
>tables, one can get a query having no tables. not a crash or anything.
hum.. maybe just a
#if false
#endif
could some the problem? i like the 'inline' table information, but no problem if remove it, but for now, we can at least use it
it's easy to search tables with LIKE operator, example:
select * from query_cache_queries where tables like "%`db name`.`table name`%"
maybe we will change it to something like
select * from query_cache_queries where tables IN (select query_id FROM query_cache_query_and_table where table = 'db_name.table_name')

the query_and_table is a table like:
query_id | db_name | table name

>yes. and SQL way of solving it is to specify desired columns explicitly.
>or to create a view. not to create a second table which is a copy of the
>first, but with less columns.
yes, you are right, well maybe again some #ifdef? i changed the code to have a 4th parameters (table type)
0 = single table, 1 = stats, 2 = flags
we could use always 2, but i don't know how to use less memory, maybe just leaving cols with NULL values?

>Eh, fine. If you haven't finished working on it, I'll wait.
yes, maybe we should wait more some days (today i think i will have the last version) and we can start review the code

>But when you say "please review", I assume that the code is final, and I
>will not accept any new changes. If you will want to add more features
>to this plugin after you said "please review", you will need to create a
>new jira task with a new contribution. Ok?
ok, sorry the rework :/

>So, now, I'll stop looking at this plugin and will wait until you're
>done. And, again, when you say you're done - no more changes.
nice! thanks and sorry about the rework again

>Regards,
>Sergei

Comment by roberto spadim [ 2013-06-19 ]

About the sql_cache.h sql_cache.cc, should i open other MDEV?

Comment by roberto spadim [ 2013-06-19 ]

created another mdev just for statistics MDEV-4581
i will send now the new diff

Comment by roberto spadim [ 2013-06-19 ]

new fields (flags) to qc_info plugin

Comment by roberto spadim [ 2013-06-20 ]

hi sergey, tell me what is better:
1) create another MDEV to change this one after aproved, to create the table
QUERY_CACHE_QUERY_TABLES
2) change this MDEV to include it before closing

it's easy to implement, but since last review, you told me to wait the review instead of changing code while you are working on it

Comment by Sergei Golubchik [ 2013-06-20 ]

I'm not working on it now, I'm waiting.
You can do either way.

Comment by roberto spadim [ 2013-06-20 ]

ok, i will change it, and post again, after i contact you to review when possible

Comment by roberto spadim [ 2013-06-20 ]

new table query/table
removed table field from queries table

Comment by roberto spadim [ 2013-06-20 ]

DONE!

Comment by roberto spadim [ 2013-06-21 ]

what about partitioning? could i get the partitions used in some query cached? isn't of only the table?

Comment by roberto spadim [ 2013-07-15 ]

hi sergey, the partition isn't implemented yet, i read the mysql/mariadb codes, no problem, maybe in a future we can implement...

i have a doubt about the
#ifdef QUERY_CACHE_QC_INFO_PLUGIN

should i use another name? should i remove?
i think the code is relative stable, if you want to review again, no problem , i think i will not change it until MDEV-4584 and MDEV-4682

any news please contact me, bye

Comment by roberto spadim [ 2013-08-09 ]

Hi serg, any news about adding this patch to current branch?

Comment by roberto spadim [ 2013-08-16 ]

hi Sergei, about this plugin, should i change something?

other doubt, there's a manual or doc about creating unit tests for mariadb?

Comment by Sergei Golubchik [ 2013-08-18 ]

unit tests — see unittest/ directory.
sql tests — see mysql-test/ directory.
for a plugin you would probably want to see the plugin suite, mysql-test/suite/plugin
or your own suite in plugin/qc_info/mysql-test/

See also https://kb.askmonty.org/en/mysql-test/

Comment by roberto spadim [ 2014-09-21 ]

please close this mdev, everything is inside mdev-4682

Comment by Daniel Black [ 2015-05-18 ]

https://github.com/MariaDB/server/pull/67/files contains just the flags addition.

Comment by Daniel Black [ 2015-05-21 ]

i assume by upping the priority you might look at this. If so opening the issue again is probably also required.

Comment by Sergei Golubchik [ 2015-05-21 ]

I'll look at MDEV-4682

Generated at Thu Feb 08 06:57:31 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.