[MDEV-6887] Optimize sys_vars.sysvar_* tests Created: 2014-10-17  Updated: 2021-11-25

Status: Stalled
Project: MariaDB Server
Component/s: Tests
Fix Version/s: N/A

Type: Task Priority: Minor
Reporter: Elena Stepanova Assignee: Elena Stepanova
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File mdev6887-2.patch     File mdev6887-radical.patch    
Issue Links:
Relates
relates to MDEV-8500 sys_vars.sysvars_innodb '32bit,xtradb... Closed

 Description   

<serg> in 10.1 there are now sys_vars.sysvar_* tests they list all system variables and their metadata
<serg> this is, obviously, pretty fragile, I've split them in debug/non-debug, embedded, and created rdiffs for xtradb/innodb, 64bit/32bit, etc
<serg> and simply disabled some of them for valgrind or non-debug builds
<serg> still they require a lot of maintenance. Like after bug changes I need to build 32-bit version to update 32-bit rdiffs, and often do the same on windows etc
<serg> I wonder whether you can manage to optimize them somehow (what should rdiff, what should be .result, where they should be simply disabled, etc)
<serg> so that they won't break that often or that rdiffs apply cleanly?
<serg> like mysqld--help test, rdiff for windows almost always just works, it's enough to update the result on linux
<serg> that's not particularly urgent, I'll keep updating tests manually anyway
<serg> but I've spent too much time on it already and mostly out of ideas now
<serg> that's one of the new features in 10.1 that I've recently pushed
<serg> INFORMATION_SCHEMA.SYSTEM_VARIABLES table: https://mariadb.com/kb/eninformation-schema-system_variables-table/
<elenst> Do you expect it to be so useful that it's worth the trouble?
<serg> the feature - yes, absolutely. tests? I don't know, really, perhaps disabling them isn't that bad
<serg> I'd like to have this feature tested, but it doesn't need to list all variables, I agree

 Comments   
Comment by Elena Stepanova [ 2015-07-02 ]

See http://buildbot.askmonty.org/buildbot/builders/kvm-bintar-centos5-x86/builds/3011/steps/test/logs/stdio

misordered hunks! output would be garbled
 
1 out of 134 hunks FAILED -- saving rejects to file -
 
main.pool_of_threads                     w1 [ pass ]  17268
 
worker[1] > Restart [mysqld.1 - pid: 26118, winpid: 26118] - using different config file
 
sys_vars.sysvars_server_notembedded '32bit' w4 [ fail ]
 
        Test ended at 2015-07-02 15:13:30
 
 
 
CURRENT_TEST: sys_vars.sysvars_server_notembedded
 
--- /usr/local/mariadb-10.1.6-linux-i686/mysql-test/suite/sys_vars/r/sysvars_server_notembedded,32bit.result~	2015-07-02 15:13:29.000000000 +0200
 
+++ /usr/local/mariadb-10.1.6-linux-i686/mysql-test/suite/sys_vars/r/sysvars_server_notembedded,32bit.reject	2015-07-02 15:13:29.000000000 +0200
 
@@ -4930,7 +4930,7 @@
 
 VARIABLE_TYPE	BIGINT UNSIGNED
 
 VARIABLE_COMMENT	Sets the internal state of the RAND() generator for replication purposes
 
 NUMERIC_MIN_VALUE	0
 
-NUMERIC_MAX_VALUE	18446744073709551615
 
+NUMERIC_MAX_VALUE	4294967295
 
 NUMERIC_BLOCK_SIZE	1
 
 ENUM_VALUE_LIST	NULL
 
 READ_ONLY	NO
 
 
 
mysqltest: Result length mismatch

It's not MTR, it's the patch, reproducible on the same centos 5 32-bit vm, but not on my wheezy 64-bit machine:

[buildbot@localhost ~]$ patch -p 3  < sysvars_server_notembedded,32bit.rdiff
 
patching file sysvars_server_notembedded.result
 
Hunk #7 succeeded at 256 (offset 14 lines).
 
...
 
Hunk #131 succeeded at 4907 (offset 14 lines).
 
misordered hunks! output would be garbled
 
Hunk #133 FAILED at 4920.
 
Hunk #134 succeeded at 5011 (offset 10 lines).
 
1 out of 134 hunks FAILED -- saving rejects to file sysvars_server_notembedded.result.rej
 

[buildbot@localhost ~]$ patch -v
 
patch 2.5.4

Comment by Elena Stepanova [ 2015-07-23 ]

On Windows sysvar tests also fail (both on 32-bit and 64-bit).

Comment by Elena Stepanova [ 2015-08-02 ]

The first solution I implemented was this:

  • get rid of wordsize-dependent rdiffs by making the tests return unified results for 64-bit and 32-bit. For this,
    • in each affected test, keep lists of variables for which either types or max/default/etc values depend on the word size (separately for types and for values);
    • propagate the word size from suite.pm (which detects it anyway) to an environment variable, so it could be changed from inside a test;
    • conditionally substitute the values for these variables while querying the I_S, replacing them with generic placeholders, identical for 64-bit and 32-bit;
    • create generic result files and get rid of 32bit rdiffs
  • create rdiff files for Windows (they do not exist now, and the tests fail on Windows, it's just not obvious because they don't run on WIndows in buildbot);
  • for frequently changing variables, such as INNODB_VERSION, read fewer fields so that the actual version number does not have to be fixed on every release.

The patch is here: https://github.com/MariaDB/server/commit/77f6c082c5f8ea9ddc457d7a4683f457d95b82ba , and in case I somehow manage to overwrite it later, I will also attach it as mdev6887-radical.patch.

In fact, it could have been even more radical: Windows/non-Windows differences could also be suppressed (or minimized) on the test level.

However, I stopped at this point because the solution turns out to be extremely ugly as it is, I really don't like it. So, I'm going to save it, return to the beginning, and try another approach, less radical. Whichever turns out to be more efficient at the end (in terms of making the maintenance easier) will win.

Comment by Elena Stepanova [ 2015-08-04 ]

So, I did some research to see why sysvar* rdiffs have to be changed so often, rather than apply smoothly on top of modified result files. For that, I took all result files from each version, and attempted to apply rdiffs from a previous version (to see when hunks start failing), and also reviewed the contents of rdiffs and changes in them (to see when the changes were necessary due to objective reasons).

There are several reasons why rdiffs have to be changed.

  • in every version, both xtradb rdiffs break because of the version change. The rdiffs replace InnoDB plugin version with the Percona version; so, the InnoDB plugin version has to be in the rdiff file; and when it changes, which happens with every release, the hunk cannot be applied anymore.
    It is very easy to fix by not printing the actual version number in the output. It will also reduce the amount of maintenance for result files, especially in mature releases, when only the version changes.

Other reasons mostly concern early releases, when functionality changes often. For them, I don't see any good solution apart from the radical one:

  • if a variable is added, it can break a context in the rdiff file. It concerns both xtradb rdiff and 32-bit rdiffs, and will concern windows rdiffs as well, if they are added. Most generic case is this:

    Existing rdiff

    		 
    var1 context
    		 
    + combination-specific variable
    		 
    var2 context

    Then, a new variable is added in the general result file and alphabetically is placed after var1. The combination-specific variable should go after it, but the hunk cannot be applied.

  • a variable type is changed. It affects 32-bit rdiffs – when rdiff replaces BIGINT with INT, and BIGINT changes to e.g. DOUBLE, obviously the hunk is not applicable anymore.
  • a variable is removed. If rdiff either affected it directly, or had it in the context, it will also fail.

Finally, there was one case for which I don't have a good explanation, apart from maybe patch failure or limitation. When 10.1.3 rdiffs for server_embedded and server_notembedded are applied to corresponding result files from 10.1.4, the following happens:

$ patch < sysvars_server_embedded,32bit.rdiff 
patching file sysvars_server_embedded.result
 
Hunk #1 succeeded at 57 (offset 2 lines).
 
Hunk #2 succeeded at 71 (offset 2 lines).
 
Hunk #3 succeeded at 85 (offset 2 lines).
 
Hunk #4 succeeded at 144 (offset 2 lines).
 
Hunk #5 succeeded at 169 (offset 2 lines).
 
Hunk #6 succeeded at 183 (offset 2 lines).
 
Hunk #7 succeeded at 242 (offset 2 lines).
 
Hunk #8 succeeded at 256 (offset 2 lines).
 
Hunk #9 succeeded at 449 (offset 2 lines).
 
Hunk #10 succeeded at 505 (offset 2 lines).
 
Hunk #11 succeeded at 519 (offset 2 lines).
 
Hunk #12 succeeded at 533 (offset 2 lines).
 
Hunk #13 succeeded at 547 (offset 2 lines).
 
Hunk #14 succeeded at 603 (offset 2 lines).
 
Hunk #15 succeeded at 617 (offset 2 lines).
 
Hunk #16 succeeded at 631 (offset 2 lines).
 
Hunk #17 succeeded at 645 (offset 2 lines).
 
Hunk #18 succeeded at 673 (offset 2 lines).
 
Hunk #19 succeeded at 743 (offset 30 lines).
 
Hunk #20 succeeded at 771 (offset 30 lines).
 
Hunk #21 succeeded at 813 (offset 30 lines).
 
Hunk #22 succeeded at 855 (offset 30 lines).
 
Hunk #23 succeeded at 869 (offset 30 lines).
 
Hunk #24 succeeded at 883 (offset 30 lines).
 
Hunk #25 succeeded at 942 (offset 30 lines).
 
Hunk #26 succeeded at 1093 (offset 30 lines).
 
Hunk #27 succeeded at 1121 (offset 30 lines).
 
Hunk #28 succeeded at 1233 (offset 30 lines).
 
Hunk #29 succeeded at 1264 (offset 30 lines).
 
Hunk #30 succeeded at 1289 (offset 30 lines).
 
Hunk #31 succeeded at 1527 (offset 30 lines).
 
Hunk #32 succeeded at 1625 (offset 30 lines).
 
Hunk #33 succeeded at 1653 (offset 30 lines).
 
Hunk #34 succeeded at 1695 (offset 30 lines).
 
Hunk #35 succeeded at 1705 (offset 30 lines).
 
Hunk #36 succeeded at 1723 (offset 30 lines).
 
Hunk #37 succeeded at 1733 (offset 30 lines).
 
Hunk #38 succeeded at 1751 (offset 30 lines).
 
Hunk #39 succeeded at 1765 (offset 30 lines).
 
Hunk #40 succeeded at 1779 (offset 30 lines).
 
Hunk #41 succeeded at 1793 (offset 30 lines).
 
Hunk #42 succeeded at 1810 (offset 30 lines).
 
Hunk #43 succeeded at 1821 (offset 30 lines).
 
Hunk #44 succeeded at 1849 (offset 30 lines).
 
Hunk #45 succeeded at 1863 (offset 30 lines).
 
Hunk #46 succeeded at 1877 (offset 30 lines).
 
Hunk #47 succeeded at 1891 (offset 30 lines).
 
Hunk #48 succeeded at 1905 (offset 30 lines).
 
Hunk #49 succeeded at 1919 (offset 30 lines).
 
Hunk #50 succeeded at 1947 (offset 30 lines).
 
Hunk #51 succeeded at 1975 (offset 30 lines).
 
Hunk #52 FAILED at 1959.
 
Hunk #53 FAILED at 1973.
 
Hunk #54 succeeded at 2017 (offset 30 lines).
 
Hunk #55 succeeded at 2031 (offset 30 lines).
 
Hunk #56 succeeded at 2045 (offset 30 lines).
 
Hunk #57 succeeded at 2059 (offset 30 lines).
 
Hunk #58 succeeded at 2073 (offset 30 lines).
 
Hunk #59 succeeded at 2083 (offset 30 lines).
 
Hunk #60 succeeded at 2097 (offset 30 lines).
 
Hunk #61 succeeded at 2129 (offset 30 lines).
 
Hunk #62 succeeded at 2146 (offset 30 lines).
 
Hunk #63 succeeded at 2199 (offset 30 lines).
 
Hunk #64 succeeded at 2213 (offset 30 lines).
 
Hunk #65 succeeded at 2227 (offset 30 lines).
 
Hunk #66 succeeded at 2241 (offset 30 lines).
 
Hunk #67 succeeded at 2311 (offset 30 lines).
 
Hunk #68 succeeded at 2325 (offset 30 lines).
 
Hunk #69 succeeded at 2339 (offset 30 lines).
 
Hunk #70 succeeded at 2367 (offset 30 lines).
 
Hunk #71 succeeded at 2395 (offset 30 lines).
 
Hunk #72 succeeded at 2409 (offset 30 lines).
 
Hunk #73 succeeded at 2423 (offset 30 lines).
 
Hunk #74 succeeded at 2437 (offset 30 lines).
 
Hunk #75 succeeded at 2451 (offset 30 lines).
 
Hunk #76 succeeded at 2465 (offset 30 lines).
 
Hunk #77 succeeded at 2479 (offset 30 lines).
 
Hunk #78 succeeded at 2493 (offset 30 lines).
 
Hunk #79 succeeded at 2507 (offset 30 lines).
 
Hunk #80 succeeded at 2521 (offset 30 lines).
 
Hunk #81 succeeded at 2535 (offset 30 lines).
 
Hunk #82 succeeded at 2549 (offset 30 lines).
 
Hunk #83 succeeded at 2563 (offset 30 lines).
 
Hunk #84 succeeded at 2577 (offset 30 lines).
 
Hunk #85 succeeded at 2591 (offset 30 lines).
 
Hunk #86 succeeded at 2605 (offset 30 lines).
 
Hunk #87 succeeded at 2619 (offset 30 lines).
 
Hunk #88 succeeded at 2633 (offset 30 lines).
 
Hunk #89 succeeded at 2647 (offset 30 lines).
 
Hunk #90 succeeded at 2661 (offset 30 lines).
 
Hunk #91 succeeded at 2675 (offset 30 lines).
 
Hunk #92 succeeded at 2689 (offset 30 lines).
 
Hunk #93 succeeded at 2703 (offset 30 lines).
 
Hunk #94 succeeded at 2717 (offset 30 lines).
 
Hunk #95 succeeded at 2731 (offset 30 lines).
 
Hunk #96 succeeded at 2745 (offset 30 lines).
 
Hunk #97 succeeded at 2759 (offset 30 lines).
 
Hunk #98 succeeded at 2773 (offset 30 lines).
 
Hunk #99 succeeded at 2787 (offset 30 lines).
 
Hunk #100 succeeded at 2801 (offset 30 lines).
 
Hunk #101 succeeded at 2871 (offset 30 lines).
 
Hunk #102 succeeded at 2899 (offset 30 lines).
 
Hunk #103 succeeded at 2913 (offset 30 lines).
 
Hunk #104 succeeded at 2969 (offset 30 lines).
 
Hunk #105 succeeded at 2983 (offset 30 lines).
 
Hunk #106 succeeded at 2997 (offset 30 lines).
 
Hunk #107 succeeded at 3011 (offset 30 lines).
 
Hunk #108 succeeded at 3028 (offset 30 lines).
 
Hunk #109 succeeded at 3081 (offset 30 lines).
 
Hunk #110 succeeded at 4108 (offset 1040 lines).
 
Hunk #111 succeeded at 4118 (offset 1036 lines).
 
Hunk #112 FAILED at 3093.
 
Hunk #113 FAILED at 3107.
 
Hunk #114 FAILED at 3135.
 
Hunk #115 FAILED at 3149.
 
Hunk #116 FAILED at 3191.
 
Hunk #117 FAILED at 3289.
 
Hunk #118 FAILED at 3303.
 
Hunk #119 FAILED at 3362.
 
Hunk #120 FAILED at 3639.
 
Hunk #121 FAILED at 3709.
 
Hunk #122 FAILED at 3723.
 
Hunk #123 FAILED at 3737.
 
Hunk #124 FAILED at 3751.
 
Hunk #125 FAILED at 3866.
 
Hunk #126 FAILED at 3877.
 
Hunk #127 FAILED at 3891.
 
Hunk #128 FAILED at 3989.
 
Hunk #129 FAILED at 4091.
 
Hunk #130 succeeded at 4203 (offset 34 lines).
 
20 out of 130 hunks FAILED -- saving rejects to file sysvars_server_embedded.result.rej
 
elenst@wheezy-64:~/tmp/10.1.3$ patch < sysvars_server_embedded.result.rej 
patching file sysvars_server_embedded.result
 
Hunk #1 FAILED at 1959.
 
Hunk #2 FAILED at 1973.
 
Hunk #3 succeeded at 3095 (offset 2 lines).
 
Hunk #4 succeeded at 3109 (offset 2 lines).
 
Hunk #5 succeeded at 3137 (offset 2 lines).
 
Hunk #6 succeeded at 3151 (offset 2 lines).
 
Hunk #7 succeeded at 3193 (offset 2 lines).
 
Hunk #8 succeeded at 3291 (offset 2 lines).
 
Hunk #9 succeeded at 3305 (offset 2 lines).
 
Hunk #10 succeeded at 3364 (offset 2 lines).
 
Hunk #11 succeeded at 3641 with fuzz 2 (offset 2 lines).
 
Hunk #12 succeeded at 3711 (offset 2 lines).
 
Hunk #13 succeeded at 3725 (offset 2 lines).
 
Hunk #14 succeeded at 3739 (offset 2 lines).
 
Hunk #15 succeeded at 3753 (offset 2 lines).
 
Hunk #16 succeeded at 3868 (offset 2 lines).
 
Hunk #17 succeeded at 3879 (offset 2 lines).
 
Hunk #18 succeeded at 3893 (offset 2 lines).
 
Hunk #19 succeeded at 3991 (offset 2 lines).
 
Hunk #20 succeeded at 4095 (offset 4 lines).
 
2 out of 20 hunks FAILED -- saving rejects to file sysvars_server_embedded.result.rej

That is, at some point all hunks start failing. But if the failed hunks are applied again, they work all right. The 2 that still fail, do it on the reasons listed earlier; and they are not responsible for the other failures, because even removing them does not help.
Anyway, it happened only once so far, I suppose it can be ignored for now.

Comment by Elena Stepanova [ 2015-08-04 ]

Now, back to comparison with mysqld--help, which mostly works smoothly with its windows rdiff.

It turns out mysqld--help does exactly the same that I did in my "radical" solution. Only it does it much thicker, it just unconditionally replaces all "32-bit-looking" values with the corresponding "64-bit-looking" values, and also suppresses some inconvenient variables completely. Still, it has to have the windows rdiff because for Windows it's not just about different values, there are some variables that only exist on Windows, and some that do not exist on Windows. So, again, it correlates with my existing ugly solution, where I get rid of 32-bit rdiffs, but introduce win rdiffs instead. And of course, xtradb rdiff will have to stay as well.

So, my conclusion is this: we cannot target only mature versions with these tests, and maintaining them on early releases is real pain (I had a chance to feel it while working on this). So, the radical solution wins, I am going to get back to it, maybe will be able to make it somewhat less ugly.

But even then, adding or removing variables can break the context of remaining rdiffs. It happens with mysqld--help as well (for example, see 55e99b29339dae833448ded6d0ca3d31f673d02a); only, without plugin variables it happens rarely. We are going to have this problem with our XtraDB rdiff much more often. When other problems are solved, I will need to get back to this one.

Comment by Elena Stepanova [ 2015-08-06 ]

Second solution (attached as mdev6887-2.patch) does type and value replacements unconditionally. It's mysqld--help style, which is of course not very precise.
Additional logic for InnoDB version number.
32-bit rdiffs are removed, Windows rdiffs are introduced.

I tried 10.1.1=>10.1.2=>10.1.3=>10.1.4=>10.1.5=>10.1.6 upgrade to see how much maintenance problems it solves. 

10.1.1 => 10.1.2
 
Linux:
 
server, innodb, xtradb fail
 
after updating results => xtradb fails (mismatches, rdiff applies smoothly)
 
Windows (after fixing Linux tests):
 
server fails (1 hunk, broken context), debug does not build, can't check the rest
 
 
 
10.1.2 => 10.1.3
 
 
 
Linux:
 
all fail (additions, changes)
 
after updating results => xtradb fails (1 hunk, broken context)
 
 
 
Windows:
 
debug and innodb fail (broken context, maybe in the previous version too)
 
 
 
10.1.3 => 10.1.4
 
 
 
Linux:
 
aria passes, other fail 
after updating results => xtradb fails, 2 hunks, broken context
 
 
 
Windows: 
debug and xtradb fail (broken context)
 
 
 
10.1.4 => 10.1.5
 
 
 
Linux:
 
server and wsrep fail
 
 
 
Windows:
 
okay
 
 
 
10.1.5 => 10.1.6
 
 
 
Linux:
 
wsrep, server, innodb, xtradb failed (additions, changes)
 
after updating result files: okay
 
 
 
WIndows:
 
okay

So, only in the last 2 versions all rdiffs worked smoothly, previously they needed to be re-created. Sometimes it's inevitable, when changes affect variables directly covered by rdiffs; but more often it's about the context, neighboring variables get changed, so the hunks cannot be applied.

Comment by Sergei Golubchik [ 2015-08-06 ]

If it's often about the context, this can be fixed, I guess. We can make sure the context never changes. Either by having less context (diff -U1 for example), or by providing constant context:

select '' as before1, '' as before2, '' as before3, *, '' as after1, '' as after2, '' as after3 from

Comment by Elena Stepanova [ 2015-08-06 ]

Yes, I've been thinking about both possibilities.
I don't think we can afford -U1. I don't know all dark magic of patch, but it appears to me that -U1 might do more harm than good. With the current rdiffs, I'm almost certain it would. Here is a typical hunk:

@@ -144,7 +144,7 @@
 
 VARIABLE_TYPE  BIGINT UNSIGNED
 
 VARIABLE_COMMENT       The size of the transactional cache for updates to transactional engines for the binary log. If you often use transactions containing many statements, you can increase this to get more performance
 
 NUMERIC_MIN_VALUE      4096
 
-NUMERIC_MAX_VALUE      18446744073709551615
 
+NUMERIC_MAX_VALUE      4294967295
 
 NUMERIC_BLOCK_SIZE     4096
 
 ENUM_VALUE_LIST        NULL
 
 READ_ONLY      NO

So, 1-line context would make it completely indistinguishable; and line numbers alone cannot be enough, because they change even more often than the context.

If we get rid of 32-bit rdiffs, there will be less examples like that, but they still exist:

@@ -1021,7 +1021,7 @@
 
 COMMAND_LINE_ARGUMENT  NULL
 
 VARIABLE_NAME  HAVE_CRYPT
 
 SESSION_VALUE  NULL
 
-GLOBAL_VALUE   YES
 
+GLOBAL_VALUE   NO
 
 GLOBAL_VALUE_ORIGIN    COMPILE-TIME
 
 DEFAULT_VALUE  NULL
 
 VARIABLE_SCOPE GLOBAL

For the second solution, again my lack of expertise in patch magic does not allow me to be sure, but I suspect patches which add variables will fail miserably, because they won't know where exactly to add them.

I was going to experiment with both, but I start having a suspicion that some context problems might stay unavoidable.

Comment by Elena Stepanova [ 2015-08-06 ]

On a separate note, performance_schema variables should probably be extracted into their own test, because as it is now, the test should fail if the server is compiled without P_S.

Generated at Thu Feb 08 07:15:20 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.