← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 69704b6: MDEV-6887 Optimize sys_vars.sysvar_* tests

 

Hi Sergei,

On 06.08.2015 11:21, Sergei Golubchik wrote:
Hi, Elena!

On Aug 06, elenst@xxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp



Follow ups

References