Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Cyrill Gorcunov <gorcunov@gmail.com>,
	tml <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v4 2/3] test: add a test for wal_cleanup_delay option
Date: Wed, 24 Mar 2021 23:10:23 +0100	[thread overview]
Message-ID: <4b25e784-84b8-0a5d-d48c-2a47eb939d09@tarantool.org> (raw)
In-Reply-To: <20210324163759.1900553-3-gorcunov@gmail.com>

Thanks for working on this!

See 4 comments below.

> diff --git a/test/replication/gh-5806-xlog-cleanup.result b/test/replication/gh-5806-xlog-cleanup.result
> new file mode 100644
> index 000000000..88b272e62
> --- /dev/null
> +++ b/test/replication/gh-5806-xlog-cleanup.result
> @@ -0,0 +1,356 @@
> +--
> +-- On replica we create an own space which allows us to
> +-- use more complex scenario and disables replica from
> +-- automatic rejoin (since replica can't do auto-rejoin if
> +-- there gonna be an own data loss). This allows us to
> +-- trigger XlogGapError in the log.
> +test_run:switch('replica')
> + | ---
> + | - true
> + | ...
> +box.cfg{checkpoint_count = 1}
> + | ---
> + | ...
> +s = box.schema.space.create('testreplica')
> + | ---
> + | ...
> +_ = s:create_index('pk')
> + | ---
> + | ...
> +box.space.testreplica:insert({1})
> + | ---
> + | - [1]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +
> +--
> +-- Stop the replica node and generate
> +-- xlogs on the master.
> +test_run:switch('default')

1. You could switch to the master right away. No
need to switch through the default node.

> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server replica')
> + | ---
> + | - true
> + | ...
> +
> +test_run:switch('master')
> + | ---
> + | - true
> + | ...
> +box.space.test:insert({1})
> + | ---
> + | - [1]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +
> +--
> +-- We need to restart the master node since otherwise
> +-- the replica will be preventing us from removing old
> +-- xlog because it will be tracked by gc consumer which
> +-- kept in memory while master node is running.
> +--
> +-- Once restarted we write a new record into master's
> +-- space and run snapshot which removes old xlog required
> +-- by replica to subscribe leading to XlogGapError which
> +-- we need to test.
> +test_run:switch('default')

2. You can restart the master without switching to default.

> + | ---
> + | - true
> + | ...
> +test_run:cmd('restart server master with wait_load=True')
> + | ---
> + | - true
> + | ...
> +test_run:switch('master')
> + | ---
> + | - true
> + | ...
> +box.space.test:insert({2})
> + | ---
> + | - [2]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +assert(box.info.gc().is_paused == false)
> + | ---
> + | - true
> + | ...
> +
> +--
> +-- Start replica and wait for error.
> +test_run:switch('default')

3. You don't need to switch to default to start the replica. All the
same switch-comments for 'Case 2' test.

> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica with wait=False, wait_load=False')
> + | ---
> + | - true
> + | ...
> +
> +--
> +-- Wait error to appear.
> +test_run:wait_log('master', 'XlogGapError', 1024, 10) ~= nil

4. 10 seconds is a tiny timeout. Seconds is not enough as the
synchronous replication tests have shown. But why do you even need
non-default parameters?

> + | ---
> + | - true
> + | ...
> +

  reply	other threads:[~2021-03-24 22:10 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-24 16:37 [Tarantool-patches] [PATCH v4 0/3] gc/xlog: delay xlog cleanup until relays are subscribed Cyrill Gorcunov via Tarantool-patches
2021-03-24 16:37 ` [Tarantool-patches] [PATCH v4 1/3] " Cyrill Gorcunov via Tarantool-patches
2021-03-24 22:10   ` Vladislav Shpilevoy via Tarantool-patches
2021-03-25 11:25     ` Cyrill Gorcunov via Tarantool-patches
2021-03-25 19:59       ` Vladislav Shpilevoy via Tarantool-patches
2021-03-25 21:02         ` Cyrill Gorcunov via Tarantool-patches
2021-03-25 21:29           ` Cyrill Gorcunov via Tarantool-patches
2021-03-25 23:50             ` Vladislav Shpilevoy via Tarantool-patches
2021-03-26  7:04               ` Cyrill Gorcunov via Tarantool-patches
2021-03-25 23:51           ` Vladislav Shpilevoy via Tarantool-patches
2021-03-26  7:04             ` Cyrill Gorcunov via Tarantool-patches
2021-03-24 16:37 ` [Tarantool-patches] [PATCH v4 2/3] test: add a test for wal_cleanup_delay option Cyrill Gorcunov via Tarantool-patches
2021-03-24 22:10   ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-03-25 12:07     ` Cyrill Gorcunov via Tarantool-patches
2021-03-25 19:56       ` Vladislav Shpilevoy via Tarantool-patches
2021-03-25 20:19         ` Cyrill Gorcunov via Tarantool-patches
2021-03-24 16:37 ` [Tarantool-patches] [PATCH v4 3/3] test: box-tap/gc -- add test for is_paused field Cyrill Gorcunov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b25e784-84b8-0a5d-d48c-2a47eb939d09@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v4 2/3] test: add a test for wal_cleanup_delay option' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox