Re: [Maria-developers] [Commits] 69704b6: MDEV-6887 Optimize sys_vars.sysvar_* tests
Hi, Elena! On Aug 06, elenst@montyprogram.com wrote:
revision-id: 69704b6c6f895fa706f83bcd3bcefe267aaf8e57 parent(s): afd59b575a75ebbc57f71ce2865fdff85e3e233b committer: Elena Stepanova branch nick: bb-10.1-elenst timestamp: 2015-08-06 02:55:43 +0300 message:
MDEV-6887 Optimize sys_vars.sysvar_* tests
diff --git a/mysql-test/suite/sys_vars/inc/sysvars_server.inc b/mysql-test/suite/sys_vars/inc/sysvars_server.inc index cb06b40..98bffd4 100644 --- a/mysql-test/suite/sys_vars/inc/sysvars_server.inc +++ b/mysql-test/suite/sys_vars/inc/sysvars_server.inc @@ -11,8 +12,9 @@ set sql_mode=ansi_quotes; # global_value_origin=SQL set global div_precision_increment=5;
---replace_regex /^\/\S+/PATH/ +--replace_regex /^\/\S+/PATH/ /^[a-zA-Z]:[\\\/]\S+/PATH/ /^BIGINT\b/<INT or BIGINT>/ /^INT\b/<INT or BIGINT>/ /^(?:18446744073709551615\b|4294967295\b)/<32-bit or 64-bit MAX UNSIGNED>/ /^(?:9223372036854775807\b|2147483647\b)/<32-bit or 64-bit MAX SIGNED>/ /^(?:18446744073709547520\b|4294963200\b)/<32-bit or 64-bit MAX UNSIGNED ADJUSTED>/ /^(?:9223372036853727232\b|2146435072\b)/<32-bit or 64-bit MAX SIGNED ADJUSTED>/
Aren't you removing too much with these replacements? It's kinda good to get rid of 32bit diff files (as they aren't easy to update), but I wouldn't replace every INT or BIGINT with <INT or BIGINT>. May be, move them to a separate select, like let type=BIGINT; if (32-bit) { let type=INT; } --replace $type <INT-or-BIGINT> select * from information_schema.system_variables where variable_name in (list of ulong variables); and something similar for other replacements. In fact, there's obscure feature of let $repl=/foo/bar/ /abc/def; replace_regex $repl; so you can have sysvar32bitrepl.inc that will set up $replace_regexes variable: let type=BIGINT; let max=18446744073709551615; if (32-bit) { let type=INT; let max=4294967295; } let replace_regexes=/$type/INT or BIGINT/ /$max/<32-bit or 64-bit MAX UNSIGNED>/; Then in every sysvar*.test file you can do, like --source sysvar32bitrepl --replace_regex $replace_regexes select * from information_schema.system_variables where variable_name in (list of ulong variables); Regards, Sergei
Hi Sergei, On 06.08.2015 11:21, Sergei Golubchik wrote:
Hi, Elena!
On Aug 06, elenst@montyprogram.com wrote:
revision-id: 69704b6c6f895fa706f83bcd3bcefe267aaf8e57 parent(s): afd59b575a75ebbc57f71ce2865fdff85e3e233b committer: Elena Stepanova branch nick: bb-10.1-elenst timestamp: 2015-08-06 02:55:43 +0300 message:
MDEV-6887 Optimize sys_vars.sysvar_* tests
diff --git a/mysql-test/suite/sys_vars/inc/sysvars_server.inc b/mysql-test/suite/sys_vars/inc/sysvars_server.inc index cb06b40..98bffd4 100644 --- a/mysql-test/suite/sys_vars/inc/sysvars_server.inc +++ b/mysql-test/suite/sys_vars/inc/sysvars_server.inc @@ -11,8 +12,9 @@ set sql_mode=ansi_quotes; # global_value_origin=SQL set global div_precision_increment=5;
---replace_regex /^\/\S+/PATH/ +--replace_regex /^\/\S+/PATH/ /^[a-zA-Z]:[\\\/]\S+/PATH/ /^BIGINT\b/<INT or BIGINT>/ /^INT\b/<INT or BIGINT>/ /^(?:18446744073709551615\b|4294967295\b)/<32-bit or 64-bit MAX UNSIGNED>/ /^(?:9223372036854775807\b|2147483647\b)/<32-bit or 64-bit MAX SIGNED>/ /^(?:18446744073709547520\b|4294963200\b)/<32-bit or 64-bit MAX UNSIGNED ADJUSTED>/ /^(?:9223372036853727232\b|2146435072\b)/<32-bit or 64-bit MAX SIGNED ADJUSTED>/
Aren't you removing too much with these replacements? It's kinda good to get rid of 32bit diff files (as they aren't easy to update), but I wouldn't replace every INT or BIGINT with <INT or BIGINT>.
May be, move them to a separate select, like
let type=BIGINT; if (32-bit) { let type=INT; } --replace $type <INT-or-BIGINT> select * from information_schema.system_variables where variable_name in (list of ulong variables);
and something similar for other replacements.
It's not quite as simple. First, you cannot of course do it via replace_result, it has to be replace_regex. Secondly, you cannot do it just for 32-bit, you also need to do it for Windows, any word size. But most importantly, you cannot do it this way at all. There are BIGINT variables that remain BIGINT on 32-bit (ULONGLONG, i assume?). # Test let $repl = /BIGINT/<INT-or-BIGINT>/; if ($MTR_word_size == 32) { let $repl = /^INT\b/<INT-or-BIGINT>/; } --replace_regex $repl query_vertical select variable_name, variable_type from information_schema.system_variables where variable_name in ( 'JOIN_BUFFER_SIZE', 'AUTO_INCREMENT_INCREMENT'); # End of test # Output on 64-bit variable_name JOIN_BUFFER_SIZE variable_type <INT-or-BIGINT> UNSIGNED variable_name AUTO_INCREMENT_INCREMENT variable_type <INT-or-BIGINT> UNSIGNED # Output on 32-bit variable_name JOIN_BUFFER_SIZE variable_type BIGINT UNSIGNED variable_name AUTO_INCREMENT_INCREMENT variable_type <INT-or-BIGINT> UNSIGNED That said, the idea of having more precise tests was of course very appealing. I did it already in my previous solution, I don't know if you followed it, but you can see it here: https://mariadb.atlassian.net/secure/attachment/38941/mdev6887-radical.patch I did the replacement on SQL level, but the point is the same: the hardcoded lists of variables were just ugly. And the more flexible we want it to be, the uglier it goes. if we want to have precise lists for volatile types and volatile values, it's already two lists, none of which is a subset of another, and even more separate SELECTs. If we want to keep precise lists for GLOBAL_VALUE, DEFAULT_VALUE and MAX_VALUE, the lists and SELECTs will multiply. And the lists for values are not just "all ULONGs", they are chaotic for my eye (maybe you know the system about values substitution, then please do share, I could only create the lists empirically). So I suspect it's not just ugly, it's also going to become another kind of maintenance hell, since people will have to try it on 32-bit, fail, remember what they need to do, be precise about updating lists... So, I wanted to try something different (but I stored the solution in case I decide to return to it afterwards). Then I remembered that you suggested mysqld--help as an example of good behavior in regard to rdiffs, in the sense that it almost always works. So, mysqld--help does exactly the same as this current patch does, it replaces unconditionally. It does not touch types since they are not in the output, but it does that for values: s/\b4294967295\b/18446744073709551615/; s/\b2146435072\b/9223372036853727232/; s/\b196608\b/262144/; s/\b4294963200\b/18446744073709547520/; I really, really don't like it, because not only is it radical, but it also creates explicitly wrong results, where you cannot tell which 18446744073709551615 you can trust and which you can't. So, I accepted its lack of selectiveness, but used stubs instead of real values. that's how the current solution was born. I do share your concern about the lack of precision and keep thinking about it.
In fact, there's obscure feature of
let $repl=/foo/bar/ /abc/def; replace_regex $repl;
so you can have sysvar32bitrepl.inc that will set up $replace_regexes variable:
Thanks for reminding! I already tried to introduce the inc file exactly for this purpose, and I remembered there was some way to use variables in replace_regex, but I failed to remember what it was. Now I recalled it -- in general, replace_regex /abc/def/ [...] $repl does not work, but replace_regex $repl [/abc/def/ ...] works. I will use it. Either way, none of these approaches solves all rdiff maintenance problems that I described in my lengthy comments in MDEV-6887. To check that, I took my last solution and tried to imitate the release development, from 10.1.1 to 10.1.6. Only on 10.1.5 an 10.1.6 the Windows and XtraDB rdiffs worked smoothly, on previous upgrades they had to be re-created after results files were updated. Sometimes it's inevitable, when code changes concern the exact variables covered by rdiffs (when variables are added, or removed, or modified). But more often than not it happens when the *neighbor* variables are changed, the rdiff context gets broken. I'm thinking what can be done about it. Regards, Elena
let type=BIGINT; let max=18446744073709551615; if (32-bit) { let type=INT; let max=4294967295; } let replace_regexes=/$type/INT or BIGINT/ /$max/<32-bit or 64-bit MAX UNSIGNED>/;
Then in every sysvar*.test file you can do, like
--source sysvar32bitrepl --replace_regex $replace_regexes select * from information_schema.system_variables where variable_name in (list of ulong variables);
Regards, Sergei
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Sorry, correction inline. On 06.08.2015 14:47, Elena Stepanova wrote:
Hi Sergei,
On 06.08.2015 11:21, Sergei Golubchik wrote:
Hi, Elena!
On Aug 06, elenst@montyprogram.com wrote:
revision-id: 69704b6c6f895fa706f83bcd3bcefe267aaf8e57 parent(s): afd59b575a75ebbc57f71ce2865fdff85e3e233b committer: Elena Stepanova branch nick: bb-10.1-elenst timestamp: 2015-08-06 02:55:43 +0300 message:
MDEV-6887 Optimize sys_vars.sysvar_* tests
diff --git a/mysql-test/suite/sys_vars/inc/sysvars_server.inc b/mysql-test/suite/sys_vars/inc/sysvars_server.inc index cb06b40..98bffd4 100644 --- a/mysql-test/suite/sys_vars/inc/sysvars_server.inc +++ b/mysql-test/suite/sys_vars/inc/sysvars_server.inc @@ -11,8 +12,9 @@ set sql_mode=ansi_quotes; # global_value_origin=SQL set global div_precision_increment=5;
---replace_regex /^\/\S+/PATH/ +--replace_regex /^\/\S+/PATH/ /^[a-zA-Z]:[\\\/]\S+/PATH/ /^BIGINT\b/<INT or BIGINT>/ /^INT\b/<INT or BIGINT>/ /^(?:18446744073709551615\b|4294967295\b)/<32-bit or 64-bit MAX UNSIGNED>/ /^(?:9223372036854775807\b|2147483647\b)/<32-bit or 64-bit MAX SIGNED>/ /^(?:18446744073709547520\b|4294963200\b)/<32-bit or 64-bit MAX UNSIGNED ADJUSTED>/ /^(?:9223372036853727232\b|2146435072\b)/<32-bit or 64-bit MAX SIGNED ADJUSTED>/
Aren't you removing too much with these replacements? It's kinda good to get rid of 32bit diff files (as they aren't easy to update), but I wouldn't replace every INT or BIGINT with <INT or BIGINT>.
May be, move them to a separate select, like
let type=BIGINT; if (32-bit) { let type=INT; } --replace $type <INT-or-BIGINT> select * from information_schema.system_variables where variable_name in (list of ulong variables);
and something similar for other replacements.
It's not quite as simple.
First, you cannot of course do it via replace_result, it has to be replace_regex.
Secondly, you cannot do it just for 32-bit, you also need to do it for Windows, any word size.
But most importantly, you cannot do it this way at all. There are BIGINT variables that remain BIGINT on 32-bit (ULONGLONG, i assume?).
My apologies, while writing this I forgot you suggested to do the substitution only for ulong variables, so please ignore the last point and the test case. The rest remains true (I think). /E
# Test
let $repl = /BIGINT/<INT-or-BIGINT>/; if ($MTR_word_size == 32) { let $repl = /^INT\b/<INT-or-BIGINT>/; } --replace_regex $repl
query_vertical select variable_name, variable_type from information_schema.system_variables where variable_name in ( 'JOIN_BUFFER_SIZE', 'AUTO_INCREMENT_INCREMENT');
# End of test
# Output on 64-bit
variable_name JOIN_BUFFER_SIZE variable_type <INT-or-BIGINT> UNSIGNED variable_name AUTO_INCREMENT_INCREMENT variable_type <INT-or-BIGINT> UNSIGNED
# Output on 32-bit
variable_name JOIN_BUFFER_SIZE variable_type BIGINT UNSIGNED variable_name AUTO_INCREMENT_INCREMENT variable_type <INT-or-BIGINT> UNSIGNED
That said, the idea of having more precise tests was of course very appealing. I did it already in my previous solution, I don't know if you followed it, but you can see it here: https://mariadb.atlassian.net/secure/attachment/38941/mdev6887-radical.patch
I did the replacement on SQL level, but the point is the same: the hardcoded lists of variables were just ugly. And the more flexible we want it to be, the uglier it goes. if we want to have precise lists for volatile types and volatile values, it's already two lists, none of which is a subset of another, and even more separate SELECTs. If we want to keep precise lists for GLOBAL_VALUE, DEFAULT_VALUE and MAX_VALUE, the lists and SELECTs will multiply.
And the lists for values are not just "all ULONGs", they are chaotic for my eye (maybe you know the system about values substitution, then please do share, I could only create the lists empirically). So I suspect it's not just ugly, it's also going to become another kind of maintenance hell, since people will have to try it on 32-bit, fail, remember what they need to do, be precise about updating lists...
So, I wanted to try something different (but I stored the solution in case I decide to return to it afterwards).
Then I remembered that you suggested mysqld--help as an example of good behavior in regard to rdiffs, in the sense that it almost always works. So, mysqld--help does exactly the same as this current patch does, it replaces unconditionally. It does not touch types since they are not in the output, but it does that for values:
s/\b4294967295\b/18446744073709551615/; s/\b2146435072\b/9223372036853727232/; s/\b196608\b/262144/; s/\b4294963200\b/18446744073709547520/;
I really, really don't like it, because not only is it radical, but it also creates explicitly wrong results, where you cannot tell which 18446744073709551615 you can trust and which you can't.
So, I accepted its lack of selectiveness, but used stubs instead of real values. that's how the current solution was born. I do share your concern about the lack of precision and keep thinking about it.
In fact, there's obscure feature of
let $repl=/foo/bar/ /abc/def; replace_regex $repl;
so you can have sysvar32bitrepl.inc that will set up $replace_regexes variable:
Thanks for reminding! I already tried to introduce the inc file exactly for this purpose, and I remembered there was some way to use variables in replace_regex, but I failed to remember what it was. Now I recalled it -- in general, replace_regex /abc/def/ [...] $repl does not work, but replace_regex $repl [/abc/def/ ...] works.
I will use it.
Either way, none of these approaches solves all rdiff maintenance problems that I described in my lengthy comments in MDEV-6887. To check that, I took my last solution and tried to imitate the release development, from 10.1.1 to 10.1.6. Only on 10.1.5 an 10.1.6 the Windows and XtraDB rdiffs worked smoothly, on previous upgrades they had to be re-created after results files were updated. Sometimes it's inevitable, when code changes concern the exact variables covered by rdiffs (when variables are added, or removed, or modified). But more often than not it happens when the *neighbor* variables are changed, the rdiff context gets broken. I'm thinking what can be done about it.
Regards, Elena
let type=BIGINT; let max=18446744073709551615; if (32-bit) { let type=INT; let max=4294967295; } let replace_regexes=/$type/INT or BIGINT/ /$max/<32-bit or 64-bit MAX UNSIGNED>/;
Then in every sysvar*.test file you can do, like
--source sysvar32bitrepl --replace_regex $replace_regexes select * from information_schema.system_variables where variable_name in (list of ulong variables);
Regards, Sergei
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
participants (2)
-
Elena Stepanova -
Sergei Golubchik