Re: [Tarantool-patches] [PATCH] net.box: fix fetching of schema of an old version

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

> >> The test and the bug are still upgrade related. 'xlog/upgrade' is not
> >> called 'xlog/upgrade_called'. It is called just 'upgrade'. For all cases
> >> when an old snapshot is used to start a new tarantool.
> > 
> > I disagree here. 'upgrade' is not something about working upward an old
> > snapshot.
> 
> It is. Because you won't work on the old snapshot forever. You are
> going to upgrade anyway. Your bug is for when upgrade is started but
> not finished. Because Tarantools are new, but upgrade() is not called
> yet.

It is like "sooner or later a user will use feature X after Y, so let's
call Y as X". I understood your point, but still think that the name
'upgrade' is misleading. More general 'snap' or 'snapshots' looks better
for me.

In fact upgrade may break old connectors (say, due to unicode_ci
collation of _func on 2.2) and I guess the old schema may be kept for
quite long time so.

> > Ouch, okay, fill.lua is called on the old tarantool version (on which
> > you generate a snapshot). It is okay for my case to disable autoupgrade
> > and insert the data using the new tarantool version. I guess it is okay
> > too for most of cases. Your case with upgrade of a sequence can be
> > tested this way too.
> 
> No, it can't. Unless you tried and actually managed to reproduce this.
> Because I just tried your way. I made checkout to 2.1, took an empty
> snapshot (00000000000000000000.snap). Took 2.2 before the sequence fix
> (that commit: 13de917a0e3b8affb693d041663c174d9ec7fcc9). Then I started
> Tarantool in read-only mode:
>
> <...snipped steps to reproduce...>
> 
> So even though in your particular case it is ok, it is very
> artificial, and can't be applied to all tests.

You're right. I missed that we'll unable to reproduce the problem if
we'll call fixed upgrade script (from a new tarantool version).

> Okey, you can omit fill.lua, but I want upgrade-related tests in
> one place. Or organized in one way in maximum two places, if you are
> so desperate to use tap.

I'll rework the patch to keep snapshots together. Don't sure about
feeding initial data in my case (from fill.lua or from a test): I'll
took a time to think around.

It seems logical to move snapshots out from test suites, since they will
be used across the test suites. What do you think?