Don't change password on restart (#1550)

Eliminate unnecessary transactions after the restart before the
replication sync. This reduces the chance the hardware restart
leads to WAL corruption.

Author: yngvar-antonsson

CHANGELOG.rst

@@ -12,9 +12,21 @@ and this project adheres to
 [Unreleased]
 -------------------------------------------------------------------------------
 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Added
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 - 'Make all instances writeable' configuration field can be hidden via
   frontend-core's ``set_variable`` feature or at runtime.
 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Fixed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- Eliminate unnecessary transactions after the restart before the replication
+  sync. This reduces the chance the hardware restart leads to WAL corruption
+  ([#1546](https://github.com/tarantool/cartridge/issues/1546)).
+
 -------------------------------------------------------------------------------
 [2.7.1] - 2021-08-18
 -------------------------------------------------------------------------------

cartridge/confapplier.lua

@@ -491,23 +491,29 @@ local function boot_instance(clusterwide_config)
     end
 
     do
-        log.info('Setting password for user %q ...', username)
-        -- To be sure netbox is operable, password should always be
-        -- equal to the cluster_cookie.
-        -- Function `passwd` is safe to be called on multiple replicas,
-        -- it never cause replication conflict
-
         local read_only = box.cfg.read_only
-        box.cfg({read_only = false})
 
         if vars.upgrade_schema then
+            log.info('Upgrading schema ...')
+            box.cfg({read_only = false})
             cartridge_schema_upgrade(clusterwide_config)
         end
 
-        BoxError:pcall(
-            box.schema.user.passwd,
-            username, password
-        )
+        -- To be sure netbox is operable, password should always be
+        -- equal to the cluster_cookie.
+        -- Function `passwd` is safe to be called on multiple replicas,
+        -- it never cause replication conflict
+        -- But don't commit anything if it's already ok.
+        local user = box.space[box.schema.USER_ID].index.name:get(username)
+        -- https://github.com/tarantool/tarantool/blob/2.7.3/src/box/lua/schema.lua#L2719-L2724
+        if user == nil or user.auth['chap-sha1'] ~= box.schema.user.password(password) then
+            log.info('Setting password for user %q ...', username)
+            box.cfg({read_only = false})
+            BoxError:pcall(
+                box.schema.user.passwd,
+                username, password
+            )
+        end
 
         box.cfg({read_only = read_only})
     end

test/integration/master_restart_test.lua

@@ -0,0 +1,82 @@
+local t = require('luatest')
+local h = require('test.helper')
+local g = t.group()
+
+local fio = require('fio')
+
+g.before_each(function()
+    g.cluster = h.Cluster:new({
+        datadir = fio.tempdir(),
+        server_command = h.entrypoint('srv_basic'),
+        cookie = h.random_cookie(),
+        replicasets = {
+            {
+                roles = {},
+                servers = {
+                    { alias = 'master' },
+                    { alias = 'replica' },
+                },
+            },
+        },
+    })
+    g.cluster:start()
+    g.master = g.cluster:server('master')
+    g.replica = g.cluster:server('replica')
+end)
+
+g.after_all(function()
+    g.cluster:stop()
+    fio.rmtree(g.cluster.datadir)
+end)
+
+function g.test_master_restart_with_missing_xlog()
+    g.master.net_box:eval([[
+        -- create space
+        box.schema.create_space('test')
+        box.space.test:create_index('primary')
+        box.space.test:insert{1}
+
+        box.snapshot()
+
+        -- that transaction now stored in memory and xlog
+        box.begin()
+        box.space.test:insert{2}
+        box.space.test:insert{3}
+        box.commit()
+    ]])
+
+    -- check that data replicated
+    t.helpers.retrying({}, function()
+        local content = g.replica.net_box:eval([[
+            return box.space.test:select()
+        ]])
+        t.assert_equals(content, {{1}, {2}, {3}})
+    end)
+
+    -- stop master and remove xlogs
+    g.master:stop()
+    for _, name in ipairs(fio.glob(g.master.workdir .. '/*.xlog')) do
+        t.assert(fio.unlink(name))
+    end
+    g.master:start()
+
+    -- check that data still on replica
+    local content = g.replica.net_box:eval([[
+        return box.space.test:select()
+    ]])
+    t.assert_equals(content, {{1}, {2}, {3}})
+
+    t.helpers.retrying({}, function()
+        local info_replication = g.master.net_box:eval([[
+            return box.info.replication
+        ]])
+        t.assert_covers(info_replication[2].downstream, { status = 'follow' })
+        t.assert_covers(info_replication[2].upstream, { status = 'follow' })
+
+        -- check that data restored on master
+        local content = g.master.net_box:eval([[
+            return box.space.test:select()
+        ]])
+        t.assert_equals(content, {{1}, {2}, {3}})
+    end)
+end

test/integration/zones_test.lua

@@ -263,14 +263,16 @@ h.after_all(function()
 end)
 
 function h.test_zones_distances()
-    t.assert_items_equals(
-        h.A1:graphql({query = '{servers {uuid zone}}'}).data.servers,
-        {
-            {uuid = h.A1.instance_uuid, zone = 'z1'},
-            {uuid = h.A2.instance_uuid, zone = 'z2'},
-            {uuid = h.A3.instance_uuid, zone = box.NULL},
-        }
-    )
+    helpers.retrying({}, function()
+        t.assert_items_equals(
+            h.A1:graphql({query = '{servers {uuid zone}}'}).data.servers,
+            {
+                {uuid = h.A1.instance_uuid, zone = 'z1'},
+                {uuid = h.A2.instance_uuid, zone = 'z2'},
+                {uuid = h.A3.instance_uuid, zone = box.NULL},
+            }
+        )
+    end)
 
     local distances = h.cluster.main_server:call(
         'package.loaded.cartridge.config_get_readonly', {'zone_distances'}