Re: [Tarantool-patches] [PATCH] Add options for upgrade testing

[Alexander Turenko <alexander.turenko@tarantool.org>]

Not a review.

On Wed, Nov 18, 2020 at 12:30:49PM +0300, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Option '--snapshot' specifies a path to snapshot that will be loaded in
> Tarantool before testing.
> 
> Option '--disable-schema-upgrade' allows to skip execution
> of Tarantool upgrade script using error injection mechanism.
> 
> Closes https://github.com/tarantool/tarantool/issues/4801

This change cannot close the issue in tarantool. We need to look at all
variants of bootstrap snapshot we had over the history of supported
release branches (and maybe a bit deeper, down to some 1.9) and manifest
compatibility matrix between given data layouts and runtimes ([1]). Once
the support matrix will be defined, we should catch all test fails,
analyze and categorize them. File issue for real problems, exclude
irrelevant fails. Setup CI. It is not a small task.

[1]: https://github.com/tarantool/doc/issues/836

I think we should file an issue to test-run, solve it within our
planning iteration and estimate #4801 for the next iteration.

While I thinking around the task, the idea how to solve the problem with
replication tests comes to me. I know, know. I should have to say it
before you start the task, but I don't know everything in advance,
learning together with you and do my best.

Back to the idea. We can implement an option or errinj to pass a path to
bootstrap snapshot to tarantool and feed it instead the built-in
bootstrap.snap. This way we'll go to the right branch here:

 [src/box/box.cc; box_cfg_xc()]

 | if (checkpoint != NULL) {
 |         /* Recover the instance from the local directory */                            
 |         local_recovery(&instance_uuid, &replicaset_uuid,                               
 |                        &checkpoint->vclock);                                           
 | } else {
 |         /* Bootstrap a new instance */
 |         bootstrap(&instance_uuid, &replicaset_uuid,                                    
 |                   &is_bootstrap_leader);                                               
 | }

And so will land here:

 [src/box/memtx_engine.c; memtx_engine_bootstrap()]

 | /* Recover from bootstrap.snap */
 | say_info("initializing an empty data directory");
 | struct xlog_cursor cursor;
 | if (xlog_cursor_openmem(&cursor, (const char *)bootstrap_bin,
 |                         sizeof(bootstrap_bin), "bootstrap") < 0)
 |         return -1;

It is the place, where we should give our custom snapshot instead of
`bootstrap_bin` (which contains src/box/bootstrap.snap content).

I would call the test-run option that feeds a snapshot to tarantool this
way --bootstrap instead of --snapshot to don't confuse the local
recovery process with bootstrapping from an empty directory.

Let's discuss tasks, planning, worthiness of eating a pie step-by-step
voicely.