Code review comment for lp:~percona-dev/percona-xtrabackup/bug723097

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Valentine,

Some comments after analyzing the test case more closely:

- The test does not really involve any non-latin characters and thus, does not really test the problem. It's easy to verify by looking at the hex dump for the .sql file:

$ hexdump -C experimental/bug723097.sql | grep INSERT
000004c0 2a 21 34 30 30 30 30 20 41 4c 54 45 52 20 54 41 |*!40000 ALTER TA|
000004d0 42 4c 45 20 60 6d 65 73 73 61 67 65 73 60 20 44 |BLE `messages` D|
000004e0 49 53 41 42 4c 45 20 4b 45 59 53 20 2a 2f 3b 0a |ISABLE KEYS */;.|
000004f0 49 4e 53 45 52 54 20 49 4e 54 4f 20 60 6d 65 73 |INSERT INTO `mes|
00000500 73 61 67 65 73 60 20 56 41 4c 55 45 53 20 28 27 |sages` VALUES ('|
00000510 3f 3f 3f 3f 20 3f 3f 3f c2 b1 20 3f 3f 3f 3f 3f |???? ???.. ?????|
00000520 3f 3f 3f 3f c2 b1 27 29 3b 0a 2f 2a 21 34 30 30 |????..');./*!400|

So those '?' characters are literally question marks (3f).

That's why the test cases passes even on an unmodified xtrabackup binary.

- I don't think we need a 49-line .sql file just to create 1-row table.

- I don't think the root problem has anything to do with race conditions and hence requires some parallel background activities. This has to be investigated further. If it turns out we don't really need to run any background activities, the test case has to be added to the main 'suite' rather than 'experimental'. Otherwise we have to think about implementing some kind of synchronization facility for xtrabackup tests, because the number tests which are experimental simply due to the lack of such facility is growing.

- The query following query: "insert into messages select * from messages" doubles the test table size and is run in a tight loop during the test. On my laptop it resulted in ~60 MB table during the first few seconds of the test run. Does this test really have to consume that much resources?

« Back to merge proposal