|
The test crashes for me already in 10.3. The SYSTEM VERSIONING syntax was introduced in MariaDB Server 10.3.4.
|
10.3 25ecf8ed4b4cbca69a9fa09c27bbd4e5c83fafe3
|
mysqltest: At line 10: query 'DELETE HISTORY FROM t1 BEFORE SYSTEM_TIME NOW() ' failed: 2013: Lost connection to MySQL server during query
|
…
|
2021-02-28 11:54:55 9 [ERROR] InnoDB: Record in index `FTS_DOC_ID_INDEX` of table `test`.`t1` was not found on update: TUPLE (info_bits=0, 2 fields): {[8] (0x0000000000000003),[6] (0x000000000202)} at: COMPACT RECORD(info_bits=0, 2 fields): {[8] (0x0000000000000002),[6] (0x000000000201)}
|
mysqld: /mariadb/10.3/storage/innobase/row/row0upd.cc:2435: dberr_t row_upd_sec_index_entry(upd_node_t *, que_thr_t *): Assertion `0' failed.
|
|
|
Please review bb-10.3-midenok
|
|
I think that we have to ensure that there is no performance regression when inserting or updating indexed columns into non-versioned tables.
|
|
Answered. The branch is the same.
|
|
Please make the fix available as a single commit based on the latest 10.3 branch. All I could find was a branch that is 27 commits behind 10.3 and includes also other changes.
|
|
Please review bb-10.3-midenok-MDEV-25004
|
|
Reviewed. I don’t expect performance issues anymore.
|
|
RQG testing on origin/bb-10.3-midenok-MDEV-25004 3dfd2fb5afdc488786447b6df6c73244abcee3c5 2021-09-28T13:43:02+03:00
|
|
I hit frequent an assert looking like https://jira.mariadb.org/browse/MDEV-20640.
|
But here Versioning is involved.
|
|
--source include/have_innodb.inc
|
--disable_abort_on_error
|
SET GLOBAL default_storage_engine = 'InnoDB' /* RQG runner */;
|
|
# DROP TABLE /*! IF EXISTS*/ table200_innodb_int_autoinc ;
|
CREATE TABLE `table200_innodb_int_autoinc` (
|
`c10` varchar(255) CHARACTER SET latin1 not null,
|
`c4` varchar(355) CHARACTER SET latin1 not null,
|
`c15` char(255) CHARACTER SET utf8 default null,
|
`c0` varchar(355) CHARACTER SET latin1,
|
`c16` char(255) CHARACTER SET latin1 not null,
|
pk integer auto_increment,
|
`c2` varchar(355) CHARACTER SET latin1 default null,
|
`c9` varchar(255) CHARACTER SET utf8 default null,
|
`c13` char(255) CHARACTER SET utf8,
|
`c7` varchar(255) CHARACTER SET utf8,
|
`c5` varchar(355) CHARACTER SET utf8 not null,
|
`c3` varchar(355) CHARACTER SET utf8 default null,
|
`c11` varchar(255) CHARACTER SET utf8 not null,
|
`c1` varchar(355) CHARACTER SET utf8,
|
`c12` char(255) CHARACTER SET latin1,
|
`c17` char(255) CHARACTER SET utf8 not null,
|
`c6` varchar(255) CHARACTER SET latin1,
|
`c14` char(255) CHARACTER SET latin1 default null,
|
`c8` varchar(255) CHARACTER SET latin1 default null,
|
/*Indices*/
|
fulltext key (`c10` ),
|
fulltext key (`c4` ),
|
fulltext key (`c15` ),
|
fulltext key (`c0` ),
|
fulltext key (`c16` ),
|
primary key (pk),
|
fulltext key (`c2` ),
|
fulltext key (`c9` ),
|
fulltext key (`c13` ),
|
fulltext key (`c7` ),
|
fulltext key (`c5` ),
|
fulltext key (`c3` ),
|
fulltext key (`c11` ),
|
fulltext key (`c1` ),
|
fulltext key (`c12` ),
|
fulltext key (`c17` ),
|
fulltext key (`c6` ),
|
fulltext key (`c14` ),
|
fulltext key (`c8` )) ENGINE=innodb ;
|
INSERT INTO table200_innodb_int_autoinc VALUES
|
('use', 'strip', 'muslim', 'define', 'overwhelm', NULL, 'politics', 'copy',
|
'colleague', 'artist', 'blind', 'top', 'admission', 'demonstration', 'mineral',
|
'boom', 'eager', 'stake', 'clothes') ;
|
|
ALTER TABLE `table200_innodb_int_autoinc` ADD SYSTEM VERSIONING ;
|
DELETE FROM `table200_innodb_int_autoinc` LIMIT 3 ;
|
DELETE HISTORY FROM `table200_innodb_int_autoinc` BEFORE SYSTEM_TIME NOW(6) ;
|
|
# Give the DB some time for crashing.
|
sleep 3;
|
|
CREATE TABLE t13 (col1 INT);
|
DROP TABLE t13;
|
|
I am sure that the test could be more simplified.
|
|
Some output from the RQG run
|
=========================
|
# 2021-10-01T15:24:35 [100062] | [rr 101367 444717]2021-10-01 15:23:43 3 [ERROR] InnoDB: tried to purge non-delete-marked record in index `FTS_DOC_ID_INDEX` of table `test`.`table100_innodb_int_autoinc`: tuple: TUPLE (info_bits=0, 3 fields): {[8] (0x000000000000000A),[4] (0x8000000A),[7]aW' U(0x615727FE031255)}, record: COMPACT RECORD(info_bits=0, 3 fields): {[8] (0x000000000000000A),[4] (0x8000000A),[7]aW' U(0x615727FE031255)}
|
# 2021-10-01T15:24:35 [100062] | [rr 101367 444720]mysqld: /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:597: bool row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, const dtuple_t*): Assertion `0' failed.
|
# 2021-10-01T15:24:35 [100062] | [rr 101367 444740]211001 15:23:43 [rr 101367 444743][ERROR] mysqld got signal 6 ;
|
# 2021-10-01T15:24:35 [100062] | Thread 3 (Thread 101367.101632):
|
# 2021-10-01T15:24:35 [100062] | #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
|
# 2021-10-01T15:24:35 [100062] | #1 0x0000730f53160859 in __GI_abort () at abort.c:79
|
# 2021-10-01T15:24:35 [100062] | #2 0x0000730f53160729 in __assert_fail_base (fmt=0x730f532f6588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x555aaacb48e0 "0", file=0x555aaacb6b40 "/data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc", line=597, function=<optimized out>) at assert.c:92
|
# 2021-10-01T15:24:35 [100062] | #3 0x0000730f53171f36 in __GI___assert_fail (assertion=0x555aaacb48e0 "0", file=0x555aaacb6b40 "/data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc", line=597, function=0x555aaacb7680 "bool row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, const dtuple_t*)") at assert.c:101
|
# 2021-10-01T15:24:35 [100062] | #4 0x0000555aa9b9cf4d in row_purge_remove_sec_if_poss_leaf (node=0x61a000002d90, index=0x6180003b3908, entry=0x61900059b108) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:597
|
# 2021-10-01T15:24:35 [100062] | #5 0x0000555aa9b9d597 in row_purge_remove_sec_if_poss (node=0x61a000002d90, index=0x6180003b3908, entry=0x61900059b108) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:693
|
# 2021-10-01T15:24:35 [100062] | #6 0x0000555aa9b9d9ea in row_purge_del_mark (node=0x61a000002d90) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:767
|
# 2021-10-01T15:24:35 [100062] | #7 0x0000555aa9ba12fe in row_purge_record_func (node=0x61a000002d90, undo_rec=0x625001702e48 "", thr=0x61a000002cc8, updated_extern=false) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:1193
|
# 2021-10-01T15:24:35 [100062] | #8 0x0000555aa9ba1a14 in row_purge (node=0x61a000002d90, undo_rec=0x625001702e48 "", thr=0x61a000002cc8) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:1259
|
# 2021-10-01T15:24:35 [100062] | #9 0x0000555aa9ba1ff9 in row_purge_step (thr=0x61a000002cc8) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0purge.cc:1337
|
# 2021-10-01T15:24:35 [100062] | #10 0x0000555aa9a841d8 in que_thr_step (thr=0x61a000002cc8) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/que/que0que.cc:1038
|
# 2021-10-01T15:24:35 [100062] | #11 0x0000555aa9a84644 in que_run_threads_low (thr=0x61a000002cc8) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/que/que0que.cc:1100
|
# 2021-10-01T15:24:35 [100062] | #12 0x0000555aa9a84aa3 in que_run_threads (thr=0x61a000002cc8) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/que/que0que.cc:1140
|
# 2021-10-01T15:24:35 [100062] | #13 0x0000555aa9c47d2c in srv_task_execute (slot=0x555aab851110 <srv_sys+976>) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/srv/srv0srv.cc:2485
|
# 2021-10-01T15:24:35 [100062] | #14 0x0000555aa9c480c3 in srv_worker_thread (arg=0x0) at /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/srv/srv0srv.cc:2540
|
# 2021-10-01T15:24:35 [100062] | #15 0x00003e56327b3609 in start_thread (arg=<optimized out>) at pthread_create.c:477
|
# 2021-10-01T15:24:35 [100062] | #16 0x0000730f5325d293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
|
|
pluto:/data/Results/1633101779/MDEV-25004-MDEV-20640/dev/shm/vardir/1633101779/21/1/rr
|
|
There were also several cases where data dumps before and after Shutdown+Restart differ.
|
These failures need deeper investigation because RQG might have failed.
|
|
|
Immediately before the assertion failed, the following message had been output:
2021-10-01 15:23:43 3 [ERROR] InnoDB: tried to purge non-delete-marked record in index `FTS_DOC_ID_INDEX` of table `test`.`table100_innodb_int_autoinc`: tuple: TUPLE (info_bits=0, 3 fields): {[8] (0x000000000000000A),[4] (0x8000000A),[7]aW' U(0x615727FE031255)}, record: COMPACT RECORD(info_bits=0, 3 fields): {[8] (0x000000000000000A),[4] (0x8000000A),[7]aW' U(0x615727FE031255)}
|
I now realize that the patch will avoid inserting a record into FTS_DOC_ID_INDEX under some circumstances. That breaks the consistency between indexes and could break not only purge but also any MVCC reads and checks for implicit record locks. I would also expect CHECK TABLE to report row number mismatch.
|
|
The output of the following shows that some of my earlier review comments were addressed:
git diff 3dfd2fb5afdc488786447b6df6c73244abcee3c5..d62151d7b2f7a92fa31c113504e72c28e25f9411
|
However, as far as I can see, nothing was done to fix the problem that was found during testing.
I do not think that it is ever acceptable to have a secondary index that contains fewer records than the clustered index. Normally it can only temporarily happen when a row operation is in progress. (We first write an undo log record, then update the clustered index, then the secondary indexes, one per mini-transaction.)
I think that it would be very tricky and error-prone to implement special cases to MVCC and locking if we allowed the FTS_DOC_ID_INDEX to omit some rows.
By the way, was fulltext search ever tested on system versioned tables?
|
I now realize that the patch will avoid inserting a record into FTS_DOC_ID_INDEX under some circumstances.
marko Actually this patch does not do that, MDEV-21138 does. But you have approved that, so please suggest another solution if you think that one is not viable. Basically the problem is duplicate key error in FTS_DOC_ID_INDEX because of history row. Do you think FTS records should be duplicated for the history? I'd really benefit from some advice here...
|
|
I think that it is easiest that each index is treated in the same way by system-versioned tables.
For FTS_DOC_ID_INDEX, whether it is created by the SQL as a hidden index by InnoDB on a visible or InnoDB-internal FTS_DOC_ID column, that would mean that the secondary index records should consist of the FTS_DOC_ID as well as all the clustered index fields, including any versioning-related columns that are part of the clustered index key.
|
|
The RQG testing on
|
origin/bb-10.3-midenok commit fad5c55c83e24508b731457499916324891d722a
|
hit quite fast an assert.
|
Same problem on
|
origin/10.6 commit 1f02280904fcfbb2bd86404d1c85c025634f8c9d
|
|
Corresponding MTR based test
|
------------------------------------------------
|
--source include/have_innodb.inc
|
|
CREATE TABLE `table300_innodb_int_autoinc` (
|
`col_char_255_utf8_fulltext_key` CHAR(255) CHARACTER SET utf8,
|
`col_varchar_355_utf8_fulltext_key` VARCHAR(355) CHARACTER SET utf8,
|
pk INTEGER AUTO_INCREMENT,
|
`col_varchar_255_latin1_fulltext_key` VARCHAR(255) CHARACTER SET latin1,
|
FULLTEXT KEY (`col_char_255_utf8_fulltext_key` ),
|
PRIMARY KEY (pk),
|
FULLTEXT KEY (`col_varchar_255_latin1_fulltext_key` )) ENGINE=innodb ;
|
INSERT INTO table300_innodb_int_autoinc VALUES ('model', 'inmate', NULL, 'proper') ;
|
ALTER TABLE `table300_innodb_int_autoinc` ADD SYSTEM VERSIONING, FORCE ;
|
UPDATE `table300_innodb_int_autoinc` SET `col_varchar_355_utf8_fulltext_key` = 'revenue';
|
DELETE HISTORY FROM `table300_innodb_int_autoinc` BEFORE SYSTEM_TIME NOW(6) ;
|
|
# Not strict required but helps to detect the crash.
|
sleep 1;
|
CREATE TABLE TMP1 (c1 INT) ENGINE= InnoDB;
|
DROP TABLE TMP1;
|
|
./mtr --mem MDEV-25004
|
Logging: ./mtr --mem MDEV-25004
|
vardir: /data/Server_bin/bb-10.3-midenok_asan/mysql-test/var
|
Checking leftover processes...
|
Removing old var directory...
|
Creating var directory '/data/Server_bin/bb-10.3-midenok_asan/mysql-test/var'...
|
- symlinking 'var' to '/dev/shm/var_auto_oPv7'
|
Checking supported features...
|
MariaDB Version 10.3.32-MariaDB-debug
|
- SSL connections supported
|
- binaries are debug compiled
|
- binaries built with wsrep patch
|
Collecting tests...
|
Installing system database...
|
|
==============================================================================
|
|
TEST RESULT TIME (ms) or COMMENT
|
--------------------------------------------------------------------------
|
|
worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 16000..16019
|
CREATE TABLE `table300_innodb_int_autoinc` (
|
`col_char_255_utf8_fulltext_key` CHAR(255) CHARACTER SET utf8,
|
`col_varchar_355_utf8_fulltext_key` VARCHAR(355) CHARACTER SET utf8,
|
pk INTEGER AUTO_INCREMENT,
|
`col_varchar_255_latin1_fulltext_key` VARCHAR(255) CHARACTER SET latin1,
|
FULLTEXT KEY (`col_char_255_utf8_fulltext_key` ),
|
PRIMARY KEY (pk),
|
FULLTEXT KEY (`col_varchar_255_latin1_fulltext_key` )) ENGINE=innodb ;
|
INSERT INTO table300_innodb_int_autoinc VALUES ('model', 'inmate', NULL, 'proper') ;
|
ALTER TABLE `table300_innodb_int_autoinc` ADD SYSTEM VERSIONING, FORCE ;
|
UPDATE `table300_innodb_int_autoinc` SET `col_varchar_355_utf8_fulltext_key` = 'revenue';
|
DELETE HISTORY FROM `table300_innodb_int_autoinc` BEFORE SYSTEM_TIME NOW(6) ;
|
main.MDEV-25004 'innodb' [ fail ]
|
Test ended at 2021-10-26 15:30:02
|
|
CURRENT_TEST: main.MDEV-25004
|
mysqltest: At line 14: query 'DELETE HISTORY FROM `table300_innodb_int_autoinc` BEFORE SYSTEM_TIME NOW(6) ' failed: 2013: Lost connection ...
|
...
|
2021-10-26 15:30:01 0 [Note] /data/Server_bin/bb-10.3-midenok_asan/bin/mysqld: ready for connections.
|
Version: '10.3.32-MariaDB-debug-log' socket: '/data/Server_bin/bb-10.3-midenok_asan/mysql-test/var/tmp/mysqld.1.sock' port: 16000 Source distribution
|
2021-10-26 15:30:01 9 [ERROR] InnoDB: Record in index `FTS_DOC_ID_INDEX` of table `test`.`table300_innodb_int_autoinc` was not found on update: TUPLE (info_bits=0, 3 fields): {[8] (0x0000000000000002),[4] (0x80000001),[7]ax (0x61781EF90DA2E5)} at: COMPACT RECORD(info_bits=0, 3 fields): {[8] (0x0000000000000001),[4] (0x80000001),[7] B?(0x7FFFFFFF0F423F)}
|
mysqld: /data/Server/bb-10.3-midenok/storage/innobase/row/row0upd.cc:2439: dberr_t row_upd_sec_index_entry(upd_node_t*, que_thr_t*): Assertion `0' failed.
|
211026 15:30:01 [ERROR] mysqld got signal 6 ;
|
stack_bottom = 0x7fe95ac739e0 thread_stack 0x5fc00
|
/lib/x86_64-linux-gnu/libasan.so.5(+0x6cd30)[0x7fe96d60fd30]
|
mysys/stacktrace.c:174(my_print_stacktrace)[0x556721c2813a]
|
sql/signal_handler.cc:221(handle_fatal_signal)[0x5567209cb10c]
|
sigaction.c:0(__restore_rt)[0x7fe96d14a3c0]
|
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcb)[0x7fe96cf8918b]
|
/lib/x86_64-linux-gnu/libc.so.6(abort+0x12b)[0x7fe96cf68859]
|
/lib/x86_64-linux-gnu/libc.so.6(+0x25729)[0x7fe96cf68729]
|
/lib/x86_64-linux-gnu/libc.so.6(+0x36f36)[0x7fe96cf79f36]
|
row/row0upd.cc:2443(row_upd_sec_index_entry(upd_node_t*, que_thr_t*))[0x5567212e1f1f]
|
row/row0upd.cc:2582(row_upd_sec_step(upd_node_t*, que_thr_t*))[0x5567212e2f01]
|
row/row0upd.cc:3331(row_upd(upd_node_t*, que_thr_t*))[0x5567212e7d2f]
|
row/row0upd.cc:3446(row_upd_step(que_thr_t*))[0x5567212e84f9]
|
row/row0mysql.cc:1846(row_update_for_mysql(row_prebuilt_t*))[0x556721213059]
|
handler/ha_innodb.cc:9023(ha_innobase::delete_row(unsigned char const*))[0x556720ef12b6]
|
sql/handler.cc:6566(handler::ha_delete_row(unsigned char const*))[0x5567209f58f5]
|
sql/sql_delete.cc:245(TABLE::delete_row())[0x556720ddf9ea]
|
sql/sql_delete.cc:734(mysql_delete(THD*, TABLE_LIST*, Item*, SQL_I_List<st_order>*, unsigned long long, unsigned long long, select_result*))[0x556720dd8981]
|
sql/sql_parse.cc:4707(mysql_execute_command(THD*))[0x5567202fc1b8]
|
sql/sql_parse.cc:7870(mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool))[0x55672031106b]
|
sql/sql_parse.cc:1855(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool))[0x5567202ea13e]
|
sql/sql_parse.cc:1398(do_command(THD*))[0x5567202e7441]
|
sql/sql_connect.cc:1403(do_handle_one_connection(CONNECT*))[0x556720654d37]
|
sql/sql_connect.cc:1309(handle_one_connection)[0x5567206545f1]
|
nptl/pthread_create.c:478(start_thread)[0x7fe96d13e609]
|
/lib/x86_64-linux-gnu/libc.so.6(clone+0x43)[0x7fe96d065293]
|
|
|
Please review bb-10.3-midenok-MDEV-25004
|
|
There appears to be a problem in the regression test suite:
|
bb-10.3-midenok-MDEV-25004
|
CURRENT_TEST: innodb_fts.innodb_fts_plugin
|
mysqltest: At line 18: query 'ALTER TABLE articles ENGINE=InnoDB' failed: 4120: Not allowed for system-versioned `test`.`articles`. Change to/from native system versioning engine is not supported.
|
The statement is normally equivalent to ALTER TABLE articles FORCE or OPTIMIZE TABLE articles. Such a table rebuild is expected to succeed for any table definition.
Please document in detail (preferably in the form of executable scripts) how you tested upgrade and downgrade. Usually, downgrade and upgrade within the same release series is supposed to work without issues. In this case, we may have to accept that a table that was created with the fix present causes trouble for older minor versions. We may also have to deny write access to affected tables that were created in an older minor version.
|
|
Engine change between MyISAM and InnoDB is prohibited for system-versioned tables as of now. That's not strictly the same as FORCE.
Upgrade/downgrade was done:
1. Initialize DB and make table under version A;
2. Compile and run version B;
3. Check the table.
|
|
Can you please provide machine executable scripts for testing the upgrade and downgrade, with some evidence that the scripts are covering the affected code.
CHECK TABLE does not cover FULLTEXT INDEX at all. I think that we would need something like INSERT, UPDATE, DELETE followed by SELECT queries that exercise fulltext indexes. I would also recommend to include the following:
SET @save_optimize=@@GLOBAL.innodb_optimize_fulltext_only;
|
SET GLOBAL innodb_optimize_fulltext_only=ON;
|
OPTIMIZE TABLE t1;
|
SET GLOBAL innodb_optimize_fulltext_only=@save_optimize;
|
|
|
marko Please provide the instructions what test suite inside our tree supports pp.1-2 and the basics of how to use it. I'm not sure I'm going to write a new one or integrate 3rd party software for this bug.
|
|
midenok, in the past I tested some aspect of upgrade manually, by executing
./mtr --manual-gdb innodb.read_only_recover_committed
|
and switching servers in one of the restarts. This was somewhat cumbersome, because at least back then, mtr did not output anything to indicate that it wanted the server to be started, nor did it display the parameters for restarting the server.
I think that this idea could be automated, however. Create a test that restarts the server, and write a simple sh script that takes care of starting the servers, and invokes that test with the manual-gdb or extern option. The test should probably exist in both source trees (old and new server). For running the test, builds of both servers need to be available.
|
|
may be it's something to be better tested in buildbot? it runs upgrade tests
|
|
Upgrade/downgrade test added (see last commit).
Please review bb-10.3-midenok-MDEV-25004
|
|
The general idea of the fix looks good to me. I posted some review comments to the main commit.
Please check all test results before requesting a re-review. The test innodb_fts.basic is failing.
|
|
Your comments were addressed.
|
|
Thank you, it is much better, but still a few minor changes are needed. I’d like to see normal and upgrade/downgrade test coverage of the versioned stopword table. Can an unversioned stopword table be combined with a versioned table, or vice versa?
Please rebase this on the latest 10.3 branch. The branch that I checked was 15 commits behind.
|
|
There is 'vers' combination for the most of innodb_fts test and there stopword table is tested as versioned table.
User stopword table works only for current data. Since there is no big difference between unversioned/versioned stopword table (only the current data is used) that doesn't matter what kind of stopword table is used with what kind of fts table.
But I added combinations to stopword.test so it tests now all 4 conjuntions versioned/unversioned fts table + versioned/unversioned stopword table.
Added upgrade/downgrade tests for stopword table with some history. The result is different between the revisions since the old revision works with such stopword table incorrectly.
The branch was rebased.
|
|
I posted some review comments to some tests.
I still see some breakage, like this in a CMAKE_BUILD_TYPE=RelWithDebInfo build:
innodb_fts.fulltext2 'innodb,vers' w10 [ skipped ] Requires debug
|
line
|
2022-04-12 15:47:54 0 [Warning] 'debug' is disabled in this build
|
^ Found warnings in /dev/shm/10.6mg/mysql-test/var/10/log/mysqld.1.err
|
ok
|
Even ‘better’, all combinations of the test innodb_fts.stopword complain the same. I think that if the test used have_debug.inc then those combinations would not be expanded in the first place.
I see many errors on AddressSanitizer and Valgrind. Please fix those as well before requesting a new review. At the minimum, please run the innodb_fts suite with AddressSanitizer and Valgrind.
|
|
marko I have no problems with ASAN. Can you reproduce on latest branch (5d0874a10df) and post the stacktrace?
|
|
serg Please review commit MDEV-25004 restart_bindir option to restart server from different location in bb-10.3-midenok-MDEV-25004
|
|
midenok, I cannot review this before all tests (including ASAN and Valgrind) have been run on our CI system. I think that it can take a few days.
I think that it is a bad idea to add a debug option to filter WITH SYSTEM VERSIONING from the output of SHOW CREATE TABLE. If such filtering is needed, it should be done in a test case. We only run tests against debug versions of the server on very few operating systems. All of them are on AMD64 ISA. I think that we need more diversity, and some more effort should be spent to write tests that do not depend on debug options.
|
|
Please fix the AddressSanitizer failure:
|
bb-10.3-midenok-MDEV-25004 5d0874a10df72e2d94d73cdad78e6b1012776268
|
CURRENT_TEST: innodb_fts.crash_recovery
|
mysqltest: At line 57: query 'INSERT INTO articles (title,body) VALUES
|
('MySQL Tutorial','DBMS stands for DataBase ...')' failed: 2013: Lost connection to MySQL server during query
|
…
|
==131088==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000109f7 at pc 0x000002095d59 bp 0x7ff376b07230 sp 0x7ff376b07228
|
READ of size 1 at 0x6020000109f7 thread T28
|
#0 0x2095d58 in mach_read_from_4(unsigned char const*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/include/mach0data.inl:181:27
|
#1 0x2095d58 in mach_read_from_8(unsigned char const*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/include/mach0data.inl:360:9
|
#2 0x2095d58 in fts_init_get_doc_id(void*, void*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0fts.cc:6279:34
|
#3 0x1b4ae9b in fetch_step(que_thr_t*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/row/row0sel.cc:2437:17
|
#4 0x19d2705 in que_thr_step(que_thr_t*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/que/que0que.cc:1022:9
|
#5 0x19d2705 in que_run_threads_low(que_thr_t*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/que/que0que.cc:1100:14
|
#6 0x19d2705 in que_run_threads(que_thr_t*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/que/que0que.cc:1140:2
|
#7 0x20cad8c in fts_eval_sql(trx_t*, que_fork_t*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0sql.cc:213:2
|
#8 0x2086b8e in fts_doc_fetch_by_doc_id(fts_get_doc_t*, unsigned long, dict_index_t*, unsigned long, unsigned long (*)(void*, void*), void*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0fts.cc:3860:10
|
#9 0x207c9bb in fts_init_index(dict_table_t*, unsigned long) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0fts.cc:6445:3
|
#10 0x2071848 in fts_init_doc_id(dict_table_t const*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0fts.cc:4923:3
|
#11 0x20710d7 in fts_get_next_doc_id(dict_table_t const*, unsigned long*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/fts/fts0fts.cc:2592:3
|
#12 0x1aafc21 in row_mysql_convert_row_to_innobase(dtuple_t*, row_prebuilt_t*, unsigned char const*, mem_block_info_t**) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/row/row0mysql.cc:663:6
|
#13 0x1aafc21 in row_insert_for_mysql(unsigned char const*, row_prebuilt_t*, ins_mode_t) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/row/row0mysql.cc:1395:2
|
#14 0x16ff5f0 in ha_innobase::write_row(unsigned char*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/handler/ha_innodb.cc:8190:10
|
#15 0x10c4afb in handler::ha_write_row(unsigned char*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/handler.cc:6490:3
|
#16 0x8ad197 in write_record(THD*, TABLE*, st_copy_info*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_insert.cc:2050:12
|
#17 0x8a3656 in mysql_insert(THD*, TABLE_LIST*, List<Item>&, List<List<Item> >&, List<Item>&, List<Item>&, enum_duplicates, bool) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_insert.cc:1072:15
|
#18 0x961206 in mysql_execute_command(THD*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_parse.cc:4504:10
|
#19 0x952230 in mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_parse.cc:7870:18
|
#20 0x943e7f in dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_parse.cc:1852:7
|
#21 0x94c29c in do_command(THD*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_parse.cc:1398:17
|
#22 0xd04473 in do_handle_one_connection(CONNECT*) /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_connect.cc:1403:11
|
#23 0xd03cc3 in handle_one_connection /buildbot/amd64-ubuntu-1804-clang10-asan/build/sql/sql_connect.cc:1308:3
|
#24 0x24ebd25 in pfs_spawn_thread /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/perfschema/pfs.cc:1869:3
|
#25 0x7ff38f71d6da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
|
#26 0x7ff38f02661e in clone (/lib/x86_64-linux-gnu/libc.so.6+0x12161e)
|
0x6020000109f7 is located 0 bytes to the right of 7-byte region [0x6020000109f0,0x6020000109f7)
|
allocated by thread T28 here:
|
#0 0x63aa4d in malloc /home/brian/src/final/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
|
#1 0x1fbbc7d in eval_node_alloc_val_buf(void*, unsigned long) /buildbot/amd64-ubuntu-1804-clang10-asan/build/storage/innobase/eval/eval0eval.cc:82:29
|
Valgrind reported it as well:
|
bb-10.3-midenok-MDEV-25004 5d0874a10df72e2d94d73cdad78e6b1012776268
|
innodb_fts.crash_recovery 'debug,innodb,vers' w9 [ fail ] Found warnings/errors in server log file!
|
Test ended at 2022-04-14 02:58:17
|
Warnings:
|
Warning 1406 Data too long for column 'line' at row 24
|
line
|
==53192== Thread 27:
|
==53192== Invalid read of size 1
|
==53192== at 0x134BC97: mach_read_from_4(unsigned char const*) (mach0data.inl:181)
|
==53192== by 0x134BD2D: mach_read_from_8(unsigned char const*) (mach0data.inl:360)
|
==53192== by 0x135F92B: fts_init_get_doc_id(void*, void*) (fts0fts.cc:6279)
|
==53192== by 0x111763E: fetch_step(que_thr_t*) (row0sel.cc:2438)
|
==53192== by 0x105F757: que_thr_step(que_thr_t*) (que0que.cc:1022)
|
==53192== by 0x105FAC0: que_run_threads_low(que_thr_t*) (que0que.cc:1100)
|
==53192== by 0x105FD22: que_run_threads(que_thr_t*) (que0que.cc:1140)
|
==53192== by 0x137771F: fts_eval_sql(trx_t*, que_fork_t*) (fts0sql.cc:213)
|
==53192== by 0x13598E9: fts_doc_fetch_by_doc_id(fts_get_doc_t*, unsigned long, dict_index_t*, unsigned long, unsigned long (*)(void*, void*), void*) (fts0fts.cc:3860)
|
==53192== by 0x136003B: fts_init_index(dict_table_t*, unsigned long) (fts0fts.cc:6445)
|
==53192== Address 0xfe6beb7 is 0 bytes after a block of size 7 alloc'd
|
==53192== at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
|
==53192== by 0x1302EC0: eval_node_alloc_val_buf(void*, unsigned long) (eval0eval.cc:82)
|
==53192== by 0x11113F1: eval_node_ensure_val_buf(void*, unsigned long) (eval0eval.inl:75)
|
==53192== by 0x11116F5: eval_node_copy_and_alloc_val(void*, unsigned char const*, unsigned long) (eval0eval.inl:231)
|
==53192== by 0x1112CCA: row_sel_fetch_columns(dict_index_t*, unsigned char const*, unsigned short const*, sym_node_t*) (row0sel.cc:616)
|
==53192== by 0x11163F5: row_sel(sel_node_t*, que_thr_t*) (row0sel.cc:2014)
|
==53192== by 0x1117425: row_sel_step(que_thr_t*) (row0sel.cc:2394)
|
==53192== by 0x105F66B: que_thr_step(que_thr_t*) (que0que.cc:1014)
|
==53192== by 0x105FAC0: que_run_threads_low(que_thr_t*) (que0que.cc:1100)
|
==53192== by 0x105FD22: que_run_threads(que_thr_t*) (que0que.cc:1140)
|
|
|
marko It is presumed that system-versioned table works the same way as non-versioned table. The idea was to run any basic test from main and other test suites like it was done for system versioning with near-zero effort of adapting the tests. In development of system versioning that helped a lot to find many bugs. Do you think this is a bad idea and better idea is to utilize clumsy include system, add result combinations or copy the whole tests?
(Assignment was not for you, so please don't change the status of the bug until it is assigned to you)
|
|
midenok, about restart_bindir. I think it's very pointless to have a test in mtr that cannot be run with ./mtr testname. We have tests like that, they're never run, not even once in ten years.
If I understand correctly, to use your test one has to
- checkout 42908dc5fb06 or any other commit before the first commit for
MDEV-25004
- build it into, say, ./build-old
- checkout new commit, e.g. 10.3 branch
- build it
- run the test as OLD_BINDIR=`pwd`/../build-old ./mtr innodb_fts.versioning
Is that right? If yes — nobody will ever do that after the MDEV is closed. So, pushing this mtr change serves no purpose.
|
|
serg This is a step to implement upgrade/downgrade tests on buildbot. All it has to do is to checkout and build previous revision as well, which is not a big deal to do.
|
|
serg If the test innodb_fts.versioning is not useful now for buildbots the mtr change itself is useful as it helps to check this as marko requested. He did the same things manually. Why you are against this tool in MTR? I already had a need to check this in different bugfix.
|
|
But it's not "checkout and build previous revision". One needs to checkout and build commit 42908dc5fb06. Not a "previous" one.
Technically, buildbot could build twice, two different revisions and one a couple of tests. It could add one hour to the test time, but we don't need to do it on all builders. But here it'd be a different revision for every test that uses this feature. Completely impractical.
Practical options are:
- check in binary innodb files. I've just tested, empty ib* and *.ibd files are less than 20K compressed, not a big deal.
- do the upgrade tests in buildbot, upgrade-major step
|
|
serg By previous revision I meant "any usable" previous revision. F.ex. it can be first minor release or last previous major release. In any case, please let this feature be added into MTR. Any suggestions for the user interface are appreciated. I would add command-line option --old-bindir (or maybe --aux-bindir), also I would think of something more graceful than ENV overrider in the function interface.
|
|
Fixed ASAN failure.
|
|
Thank you midenok, I see that the branch was rebased to a recent 10.3, and no tests failed on the CI systems due to these changes.
I posted some review comments to the main commit, mostly noting code that is not covered by any tests. Mainly, it is about id-versioned tables. I would do a final review once everything is fully covered. It would be nice to also have a 10.6 based version of this for stress testing, and to assist in merging this (there are several conflicting InnoDB files).
serg, not all options of the mtr test driver are executed on a CI system. Some options are designed for interactive use. Common wisdom is that generated files should not be added to a version control system, especially when not also adding a way to generate those files. InnoDB file formats have changed in several major versions since 10.3. While the data file format change MDEV-15562 should not affect this, the log format changes in MDEV-12353, MDEV-14425 would require re-generating the files. I hope that we can have a convenient way to write mtr tests for downgrade and upgrade. It is also needed for MDEV-16417 and MDEV-22361 in hopefully not too distant future.
|
|
I agree with serg that it would be even better if an upgrade test was runnable on CI systems without any special measures. To allow that, I think that we could indeed check in a minimal data directory where all InnoDB tables have been created with innodb_file_per_table=0, that is, within the system tablespace itself. After a normal shutdown, the ib_logfile* can be safely deleted, and the redo log format changes of MDEV-12353 and MDEV-14425 should not affect this; an upgrade is supposed to work. (If a data directory was created in 10.8 or later after MDEV-14425 and MDEV-27199, then we would also need a dummy ib_logfile0 that would contain the startup LSN. 10.8 and later versions can read the FIL_PAGE_FILE_FLUSH_LSN from the first page of an old system tablespace file just fine.)
Along with a copy of such a data directory, the exact procedure for generating the data directory should be added. It could for example be a mention of a script name in the commit message; the generating script would be part of the same commit.
|
|
TBR-1468 was fixed. Comments were addressed.
Please review bb-10.3-midenok-MDEV-25004
|
|
I posted some comments. All changed code is covered now, except one that would require a table to contain at least some 300 columns.
Can you please also provide a 10.6 based branch for final testing, and to help with potential merge conflicts? Some error handling for corrupted pages was changed in MDEV-13542.
|
|
Comments were addressed.
|
|
Please fix the test failures for the 10.6 version.
|
|
What tests exactly? Please name at least one test which is specific for MDEV-25004.
|
|
I reproduced the following test failures locally:
|
bb-10.6-midenok-MDEV-25004 c849505c99179a591319381aeeabbc7b66b1361b
|
Failing test(s): gcol.virtual_index_drop innodb.innodb_corrupt_bit innodb.table_definition_cache_debug innodb.innodb-index-online innodb.alter_mdl_timeout innodb.alter_crash innodb.alter_dml_apply
|
I do not remember any of those tests failing recently. In the cross-reference, the test gcol.virtual_index_drop previously failed on some development branch more than 1 year ago. For this commit, it failed 3 times. I checked also innodb.alter_mdl_timeout: 8 failures for this commit, in 3 different environments, and previous failures were in a development branch in January 2021.
|
|
Thanks! That was really wrong merge.
|
|
This fixup is missing from the 10.6 version.
The 10.3 commit that I previous reviewed appears to correspond to this commit after a rebase. When a code branch is under development, it is easier to follow the development when the history is not rewritten (that is, the baseline is updated by a merge rather than a rebase).
Please address my comments to the main commit and the fixup, and squash them together when rebasing the branch for the final push to 10.3.
I spotted a latching order violation related to the added function vers_row_same_trx(), which could cause the server to hang when using FOREIGN KEY constraints on versioned tables. I think that it needs to be addressed before the final test run (which I think has to be done for both the 10.3 and 10.6 versions).
Before the final push, I think that serg should review the changes outside InnoDB.
|
|
The fixup is not missing, check the code of `fts_get_max_doc_id()` in 10.6
|
|
Oh, I see, the redundant offsets had been removed from fts_get_max_doc_id() in MDEV-17750 (10.4.1).
The comment of the function fts_valid_stopword_table() needs to be replaced also in fts0fts.cc.
The comment of the function upd_node_t::vers_make_delete() that was added by you in MDEV-22061 and modified in this patch is inaccurate and incorrect.
Other than that, the revised 10.3 version of the main commit looks correct to me.
Please provide an updated 10.6 version for testing.
|
|
Preliminary result of RQG testing on origin/bb-10.3-midenok-MDEV-25004 d757cd72977f6f3f038cb14be797526d1a67a153 2022-07-01T12:13:28+03:00
|
|
9 of ~ 8000 tests failed with
|
# 2022-07-25T13:28:56 [359760] | [rr 363084 577540]mysqld: /data/Server/bb-10.3-midenok-MDEV-25004/storage/innobase/row/row0merge.cc:2204: dberr_t row_merge_read_clustered_index(trx_t*, TABLE*, const dict_table_t*, dict_table_t*, bool, dict_index_t**, dict_index_t*, fts_psort_t*, merge_file_t*, const ulint*, ulint, const dtuple_t*, const dict_add_v_col_t*, const ulint*, ulint, ib_sequence_t&, row_merge_block_t*, bool, pfs_os_file_t*, ut_stage_alter_t*, double, row_merge_block_t*, TABLE*, bool): Assertion `!history_row' failed.
|
Test runs on actual origin/10.3 did not replay that problem.
|
|
--source include/have_innodb.inc
|
|
CREATE TABLE D (
|
pk INTEGER AUTO_INCREMENT,
|
col_int_nokey INTEGER ,
|
col_int_key INTEGER,
|
col_date_key DATE,
|
col_date_nokey DATE ,
|
col_time_key TIME,
|
col_time_nokey TIME ,
|
col_datetime_key DATETIME,
|
col_datetime_nokey DATETIME ,
|
col_varchar_key VARCHAR(1),
|
col_varchar_nokey VARCHAR(1) ,
|
PRIMARY KEY (pk),
|
KEY (col_int_key),
|
KEY (col_date_key),
|
KEY (col_time_key),
|
KEY (col_datetime_key),
|
KEY (col_varchar_key, col_int_key)
|
) ENGINE=InnoDB ;
|
|
INSERT INTO D (
|
col_int_key, col_int_nokey,
|
col_date_key, col_date_nokey,
|
col_time_key, col_time_nokey,
|
col_datetime_key, col_datetime_nokey,
|
col_varchar_key, col_varchar_nokey
|
) VALUES (4, 8, '2008-01-03', '2008-01-03', '01:01:02.033918', '01:01:02.033918', NULL, NULL, 'c', 'c') ;
|
|
ALTER TABLE `D` ADD SYSTEM VERSIONING ;
|
DELETE FROM `D` LIMIT 9 ;
|
ALTER TABLE `D` ADD FULLTEXT KEY(`col_varchar_nokey`) ;
|
|
DROP TABLE `D`;
|
|
The comment of the function upd_node_t::vers_make_delete() that was added by you in MDEV-22061 and modified in this patch is inaccurate and incorrect.
That was mostly exerpt from comment of vers_update_fields() made by Eugene.
|
|
Updated branches:
bb-10.3-midenok-MDEV-25004
bb-10.6-midenok-MDEV-25004
|
|
Please fix the 10.6 version so that it builds also with clang, and update it with the latest 10.6.
|
|
I posted some comments on the main commit of bb-10.6-midenok-MDEV-25004 and the rebase fixup.
I identified some code that is not covered by tests, as well as inadequate error handing of vers_row_same_trx() and row_get_clust_rec(), which need to be addressed in the 10.6 version, to be compatible with MDEV-13542.
As far as I can tell by the following (comparing old to new):
git diff 33f233f80b466197896c47a1eb9e4f296ee1b72c~..5fc0530dd8ff8f9d8b9e486f781f4b438dbe9c51|diff -I^@@ -w - <(git diff 8ab183902896eca822125a25dc51295f88c2887e~..3ea07fa786e13228fcd41efc8d542e0ef1668ffe)
|
the main change in the 10.3 version is that the test case posted by mleich has been added and the code in row_merge_read_clustered_index() adjusted for it.
serg’s feedback regarding tests has not been addressed.
|
|
Please review bb-10.3-midenok-MDEV-25004
|
|
The upgrade tests introduce a runtime dependency. I think that the upgrade tests do not currently run on Microsoft Windows, and an added dependency on tar and xz would not make that easier. In any case, the test failures on CentOS 6 will have to be fixed. I think that the tests need to be moved to the versioning suite, which would execute also in our Windows environments. The test name could be something like versioning.innodb_fts, perhaps.
In an archive of an old data directory that was copied after the old server was shut down, you can omit ib_logfile* to save some space. Such optimization would not be possible for data directories that were created with MariaDB Server 10.8 or later, due to MDEV-27199.
The debug injection for sysvers_force_trx and sysvers_force seems to be avoidable to me. We have very few debug builders. To allow the tests to run also with an uninstrumented server, the SQL statements can be adapted by the test itself. In mysql-test/suite/versioning/common.inc you can see some examples where the variables MTR_COMBINATION_TRX_ID and MTR_COMBINATION_TIMESTAMP affect the choice of some SQL snippets.
This fixup must be part of the main commit. The main comment is identical to the 10.3-based commit 33f233f80b466197896c47a1eb9e4f296ee1b72c that I previously checked.
Curiously, neither of the added return true in fts_init_get_doc_id() is covered in the 10.6 version, which has not been updated since my previous review. I just checked the updated bb-10.3-midenok-MDEV-25004 branch, and both statements are covered by 2 combinations of the test innodb_fts.crash_recovery.
It looks like my review comment regarding row_merge_buf_add() has not been addressed.
Also, my comment regarding vers_row_same_trx() must be addressed, not necessarily in the 10.3 version. That type of error handling will likely depend on changes made in MDEV-13542. The code coverage could be missing for the 10.6 version because various things related to crash recovery and fulltext search were rewritten in 10.6. I think that a separate 10.6 version of the fix is necessary.
|
|
To reduce the size of the image needed for upgrade testing further, you can create the tables with innodb_file_per_table=0. After the server is shut down, the only files that should really need to be copied are the InnoDB system tablespace (typically ibdata1) and any .frm files of the MariaDB data dictionary that point to the InnoDB system tablespace. This may include some files mysql/*.frm.
|
|
marko To simplify merge process I do not squash anything now. After you say OK to push for 10.3 I merge to 10.6 and you may check it then.
|
|
I would not move any innodb_fts tests into versioning suite by a couple of reasons:
1. innodb_fts tests are common between non-versionied/versioned via basic.inc and stopword.inc;
2. Versioned code for innodb_fts is feature-specific, it does not touch SQL-layer versioning code. Moving to versioning suite would make its runtime longer while testing it for SQL-layer changes does not make much sense. OTOH changes in FTS code make sense to test versioned tests for FTS.
|
|
Please review bb-10.3-midenok-MDEV-25004
|
|
Due to MDEV-13542, the 10.6 fix is a superset of the 10.3 fix. We need to review a 10.6-based version of this.
|
|
bb-10.6-midenok-MDEV-25004
|
|
The upgrade test fails to execute on Microsoft Windows:
innodb_fts.versioning 'innodb,prepare' w26 [ fail ]
|
Test ended at 2022-12-15 15:56:39
|
CURRENT_TEST: innodb_fts.versioning
|
mysqltest: In included file "./include/have_gzip.inc":
|
included from D:/Buildbot/amd64-windows/build/mysql-test/suite/innodb_fts/t/versioning.test at line 3:
|
At line 2: command "gzip --version > /dev/null 2> /dev/null" failed with wrong error: 1
|
If we are going through the trouble of archiving a copy of the system tablespace in the regression test suite, then I think that we should make use of it on all supported platforms, not only to test the upgrade for this bug, but for unrelated things, such as MDEV-29694.
I would also suggest some cleanup of the upgrade test.
|
|
I rechecked that all changed code in InnoDB is covered, including the fts_init_get_doc_id() that is uncovered according to gcov, but is actually covered by the test innodb_fts.crash_recovery.
I posted a suggestion to avoid adding a Boolean output parameter to vers_row_same_trx(), and simply make it return a special value.
I hope to complete the review tomorrow.
|
|
To reduce your effort for addressing my review comments such as this one, I chose to suggest some changes in the form of code commits, based on the 10.3 version:
Simplify vers_row_same_trx() and its caller
Slightly smaller upgrade dataset
Avoid some pessimization
OK to push after applying these changes, and after fixing the have_gzip.inc.
|
|
origin/bb-10.6-MDEV-25004 9226ee211327f595314e2e5a8519858050b841bb 2022-12-16T16:25:24+02:00
behaved well during RQG testing.
|
RR_REPLAY_SIMP.sh
breaks.txt