Re: [Tarantool-patches] [PATCH 2/2] test: add a test for wal_cleanup_delay option

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Fri, Mar 19, 2021 at 12:04:11AM +0100, Vladislav Shpilevoy wrote:
> > +
> > +function func_xlog_snap(space, value)
> 
> 1. This fails luacheck (which I don't like having in the tests,
> but still we must keep it green):
> 
> Checking test/replication/gh-5806-master.lua      1 warning
> 
>     test/replication/gh-5806-master.lua:5:10: (W111) setting non-standard global variable func_xlog_snap
> 
> Checking test/replication/gh-5806-slave.lua       1 warning
> 
>     test/replication/gh-5806-slave.lua:5:10: (W111) setting non-standard global variable func_xlog_snap

Thanks, Vlad! Actually I don't understand what this warning means. Should I make
these functions local or what?

> 
> Also why does it have 'func' prefix? It is a function
> obviously, we don't add 'func' prefix to all functions.

OK

> > +--
> > +-- gh-5806: defer xlog cleanup to keep xlogs until
> > +-- replicas present in "_cluster" are connected.
> > +-- Otherwise we are getting XlogGapError since
> > +-- master might go far forwad from replica and
> 
> 2. forwad -> forward.

Thanks!

> > + | ---
> > + | ...
> > +for i=1,2 do func_xlog_snap(box.space.testtemp, {i}) end
> 
> 3. Honestly, it would look much simpler if it would be just 4
> lines with 2 inserts and 2 snapshots.

Sure, I can make it so.

> 
> 4. Why do you do rw requests both on the replica and master?

To intermix data so that lsns would be different

> And why do you need 2 spaces?

This allows to write xlog data specifically on replica and
only in this case I managed to reach XlogGapError (otherwise
the replica reported that it is too old and restarted rejoin
procedure).

> > +
> > +--
> > +-- Restart the masted and generate the
> 
> 5. masted -> master.

Thanks:)

> 
> > +-- next range of xlogs.
> > +test_run:switch('default')
> > + | ---
> > + | - true
> > + | ...
> > +test_run:cmd('stop server master')
> > + | ---
> > + | - true
> > + | ...
> > +test_run:cmd('start server master with wait_load=True')
> 
> 6. Does 'restart server master' command work?

I must confess I didn't know about this command. Will try.

> > +--
> > +-- Restart master node and the replica then.
> 
> 7. Why do you need to restart the master 2 times?

To make sure the GC passed a few iterations. Maybe
we could manage with one as well. Thanks for the
point will try.

>> > +--
> > +-- Wait error to appear.
> > +while test_run:grep_log("master", "XlogGapError") == nil do fiber.sleep(0.01) end
> 
> 8. We have test_run:wait_log().

Didn't know, thanks!