Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, gorcunov@gmail.com
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 1/7] replication: always send raft state to subscribers
Date: Fri, 18 Jun 2021 00:00:03 +0300
Message-ID: <40a4914d-9e87-8bf2-93d6-dc5d9bb2801f@tarantool.org> (raw)
In-Reply-To: <3dc87744-6880-78a3-b77e-3a8811763c39@tarantool.org>



15.06.2021 23:53, Vladislav Shpilevoy пишет:
> Thanks for working on this! On 10.06.2021 15:32, Serge Petrenko via 
> Tarantool-patches wrote:
>> Tarantool used to send out raft state on subscribe only when raft was 
>> enabled. This was a safeguard against partially-upgraded clusters, 
>> where some nodes had no clue about Raft messages and couldn't handle 
>> them properly. Actually, Raft state should be sent out always. For 
>> example, promote bumps Raft term, even when raft is disabled, and 
>> it's important that everyone in cluster has the same term, for the 
>> sake of promote at least. So, send out Raft state to every subscriber 
>> with version >= 2.6.0 (that's when Raft was introduced). Closes #5438 
>> --- src/box/box.cc | 11 +-- 
>> test/replication/gh-5438-raft-state.result | 73 ++++++++++++++++++++ 
>> test/replication/gh-5438-raft-state.test.lua | 30 ++++++++ 
> The test still works when I revert box.cc changes. 

Hi! Thanks for pointing this out!

Yep, this happened because of raft broadcasts.
When relay is off, it still saves the lates broadcast message from raft,
and delivers it as soon as replica reconnects.

Thanks to this, I've also fixed raft broadcasts possibly being sent to 
old instances.

This could only happen after the patch which persists the latest raft 
broadcast and only
if raft was turned on for some time. So this wasn't such a big deal for 
upgrading installations,
because they wouldn't turn raft on.

Anyway, this issue is fixed as well and I made the test fail without the 
patch.
Here's the diff:

================================================

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1196b65b9..5d72cab13 100644
diff --git a/src/box/relay.cc b/src/box/relay.cc
index b1571b361..b47767769 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -760,7 +760,7 @@ relay_subscribe_f(va_list ap)
            &relay->relay_pipe, NULL, NULL, cbus_process);

      struct relay_is_raft_enabled_msg raft_enabler;
-    if (!relay->replica->anon)
+    if (!relay->replica->anon && relay->version_id >= version_id(2, 6, 0))
          relay_send_is_raft_enabled(relay, &raft_enabler, true);

      /*
@@ -842,7 +842,7 @@ relay_subscribe_f(va_list ap)
          cpipe_push(&relay->tx_pipe, &relay->status_msg.msg);
      }

-    if (!relay->replica->anon)
+    if (!relay->replica->anon && relay->version_id >= version_id(2, 6, 0))
          relay_send_is_raft_enabled(relay, &raft_enabler, false);

      /*
diff --git a/test/replication/gh-5438-raft-state.result 
b/test/replication/gh-5438-raft-state.result
index 7982796a8..6985f026a 100644
--- a/test/replication/gh-5438-raft-state.result
+++ b/test/replication/gh-5438-raft-state.result
@@ -6,26 +6,6 @@ test_run = require('test_run').new()
  --
  -- gh-5428 send out Raft state to subscribers, even when Raft is disabled.
  --
-box.schema.user.grant('guest', 'replication')
- | ---
- | ...
-test_run:cmd('create server replica with rpl_master=default,\
- script="replication/replica.lua"')
- | ---
- | - true
- | ...
-test_run:cmd('start server replica')
- | ---
- | - true
- | ...
-test_run:wait_lsn('replica', 'default')
- | ---
- | ...
-test_run:cmd('stop server replica')
- | ---
- | - true
- | ...
-
  -- Bump Raft term while the replica's offline.
  term = box.info.election.term
   | ---
@@ -41,10 +21,19 @@ test_run:wait_cond(function() return 
box.info.election.term > term end)
   | - true
   | ...

--- Make sure the replica receives new term on resubscribe.
+-- Make sure the replica receives new term on subscribe.
  box.cfg{election_mode = 'off'}
   | ---
   | ...
+
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/replica.lua"')
+ | ---
+ | - true
+ | ...
  test_run:cmd('start server replica')
   | ---
   | - true
@@ -56,6 +45,7 @@ end)
   | ---
   | - true
   | ...
+
  -- Cleanup.
  box.cfg{election_mode = old_election_mode}
   | ---
diff --git a/test/replication/gh-5438-raft-state.test.lua 
b/test/replication/gh-5438-raft-state.test.lua
index 179f4b1c9..60c3366c1 100644
--- a/test/replication/gh-5438-raft-state.test.lua
+++ b/test/replication/gh-5438-raft-state.test.lua
@@ -3,26 +3,24 @@ test_run = require('test_run').new()
  --
  -- gh-5428 send out Raft state to subscribers, even when Raft is disabled.
  --
-box.schema.user.grant('guest', 'replication')
-test_run:cmd('create server replica with rpl_master=default,\
- script="replication/replica.lua"')
-test_run:cmd('start server replica')
-test_run:wait_lsn('replica', 'default')
-test_run:cmd('stop server replica')
-
  -- Bump Raft term while the replica's offline.
  term = box.info.election.term
  old_election_mode = box.cfg.election_mode
  box.cfg{election_mode = 'candidate'}
  test_run:wait_cond(function() return box.info.election.term > term end)

--- Make sure the replica receives new term on resubscribe.
+-- Make sure the replica receives new term on subscribe.
  box.cfg{election_mode = 'off'}
+
+box.schema.user.grant('guest', 'replication')
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/replica.lua"')
  test_run:cmd('start server replica')
  test_run:wait_cond(function()\
      return test_run:eval('replica', 'return 
box.info.election.term')[1] ==\
             box.info.election.term\
  end)
+
  -- Cleanup.
  box.cfg{election_mode = old_election_mode}
  test_run:cmd('stop server replica')

================================================



-- Serge Petrenko

  reply	other threads:[~2021-06-17 21:00 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 13:32 [Tarantool-patches] [PATCH 0/7] forbid implicit limbo ownership transition Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 1/7] replication: always send raft state to subscribers Serge Petrenko via Tarantool-patches
2021-06-10 16:47   ` Cyrill Gorcunov via Tarantool-patches
2021-06-11  8:43     ` Serge Petrenko via Tarantool-patches
2021-06-11  8:44       ` Cyrill Gorcunov via Tarantool-patches
2021-06-15 20:53   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches [this message]
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 2/7] replication: forbid implicit limbo owner transition Serge Petrenko via Tarantool-patches
2021-06-15 20:55   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches
2021-06-18 22:49   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-21 10:13     ` Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 3/7] txn_limbo: fix promote term filtering Serge Petrenko via Tarantool-patches
2021-06-15 20:57   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches
2021-06-18 22:49       ` Vladislav Shpilevoy via Tarantool-patches
2021-06-21  8:55         ` Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 4/7] txn_limbo: persist the latest effective promote in snapshot Serge Petrenko via Tarantool-patches
2021-06-15 20:59   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 5/7] replication: send latest effective promote in initial join Serge Petrenko via Tarantool-patches
2021-06-15 21:00   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches
2021-06-18 22:52       ` Vladislav Shpilevoy via Tarantool-patches
2021-06-21 10:12         ` Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 6/7] box: introduce `box.ctl.demote` Serge Petrenko via Tarantool-patches
2021-06-18 22:52   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-21 14:56     ` Serge Petrenko via Tarantool-patches
2021-06-10 13:32 ` [Tarantool-patches] [PATCH 7/7] box: make promote/demote always bump the term Serge Petrenko via Tarantool-patches
2021-06-15 21:00   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-17 21:00     ` Serge Petrenko via Tarantool-patches
2021-06-18 22:53   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-21 15:02     ` Serge Petrenko via Tarantool-patches
2021-06-15 20:53 ` [Tarantool-patches] [PATCH 0/7] forbid implicit limbo ownership transition Vladislav Shpilevoy 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=40a4914d-9e87-8bf2-93d6-dc5d9bb2801f@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /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

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git