← Back to team overview

maria-developers team mailing list archive

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

 

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@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?).

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@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


_______________________________________________
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


References