Re: 448e82f9f0d: MDEV-35720 Add query_time to statistics
Hi, Monty, Only a couple of comments about the test case:
commit 448e82f9f0d Author: Michael Widenius <monty@mariadb.org> Date: Fri Dec 27 19:26:55 2024 +0200
diff --git a/mysql-test/main/log_state.test b/mysql-test/main/log_state.test --- a/mysql-test/main/log_state.test +++ b/mysql-test/main/log_state.test @@ -321,6 +321,48 @@ SET GLOBAL general_log_file = @old_general_log_file;
--enable_ps_protocol
+--echo # +--echo # MDEV-35720 Add query_time to statistics +--echo # + +connect (con2,localhost,root,,); +let $s1=`show status like "query_time"`; +select sleep(3) into @a; +let $s2=`show status like "query_time"`; +let $s3=`show global status like "query_time"`; +--disable_query_log +--eval SET @s1= SUBSTRING_INDEX('$s1', '\t', -1); +--eval SET @s2= SUBSTRING_INDEX('$s2', '\t', -1); +--eval SET @s3= SUBSTRING_INDEX('$s3', '\t', -1);
Eh, that's not how one is supposed to do it. I mean, it works, but the easier approach is let $s1=query_get_value(show status like "query_time", Value); or purely in SQL set @s1=(select variable_value from information_schema.global_status where variable_name='query_time');
+select @s1 >= 0.00 and @s1 <= 2.00 as "should be true"; +--eval select @s2 >= 2.00 and @s2 < 10.00 as "should be true"; +--eval select @s3 >= 3.00 as "should be true";
1. you don't need eval here 2. I expect this test, like almost all other timing tests will be horribly unstable in buildbot and will be eventually disabled. No, I don't have a recipe for a stable timing test.
+--enable_query_log + +# select @s1,@s2,@s3; +disconnect con2; +connection default; + +delimiter |; +create procedure p1() +begin +select sleep(1) into @a; +select sleep(2) into @a; +end| +delimiter ;| + +connect (con2,localhost,root,,); +call p1(); +let $s1=`show status like "query_time"`; +--disable_query_log +--eval SET @s1= SUBSTRING_INDEX('$s1', '\t', -1); +--eval select @s1 > 2.00 and @s1 < 10.00 as "should be true"; +--enable_query_log + +# select @s1; +disconnect con2; +connection default; + # # Cleanup #
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
Hi! On Sun, Dec 29, 2024 at 1:52 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Monty,
Only a couple of comments about the test case:
commit 448e82f9f0d Author: Michael Widenius <monty@mariadb.org> Date: Fri Dec 27 19:26:55 2024 +0200
diff --git a/mysql-test/main/log_state.test b/mysql-test/main/log_state.test --- a/mysql-test/main/log_state.test +++ b/mysql-test/main/log_state.test @@ -321,6 +321,48 @@ SET GLOBAL general_log_file = @old_general_log_file;
--enable_ps_protocol
+--echo # +--echo # MDEV-35720 Add query_time to statistics +--echo # + +connect (con2,localhost,root,,); +let $s1=`show status like "query_time"`; +select sleep(3) into @a; +let $s2=`show status like "query_time"`; +let $s3=`show global status like "query_time"`; +--disable_query_log +--eval SET @s1= SUBSTRING_INDEX('$s1', '\t', -1); +--eval SET @s2= SUBSTRING_INDEX('$s2', '\t', -1); +--eval SET @s3= SUBSTRING_INDEX('$s3', '\t', -1);
Eh, that's not how one is supposed to do it. I mean, it works, but the easier approach is
let $s1=query_get_value(show status like "query_time", Value);
or purely in SQL
set @s1=(select variable_value from information_schema.global_status where variable_name='query_time');
I use the above substring approach as this was used in some other tests. We should probably fix the other tests to use the simpler approach to not confuse other developers. I still hope that we can soon implement @@status.query_time as it will make things much simpler to use the status variables directly in expressions.
+select @s1 >= 0.00 and @s1 <= 2.00 as "should be true"; +--eval select @s2 >= 2.00 and @s2 < 10.00 as "should be true"; +--eval select @s3 >= 3.00 as "should be true";
1. you don't need eval here
This comes from me first having $s1 in the above, but later replaced with @s1.
2. I expect this test, like almost all other timing tests will be horribly unstable in buildbot and will be eventually disabled. No, I don't have a recipe for a stable timing test.
I do not expect the above to be stable as most of the test are >= of someting that is guaranteed to always be true. The only' unsafe' thing ius the < 10.00, but that can easily be increased if that is wrong. This is just there to ensure we do not get any totally ridiculous value for query_time. Thanks for the review, everything is now fixed (except the >= 10 test) Regards, Monty
participants (2)
-
Michael Widenius -
Sergei Golubchik