Re: [Tarantool-patches] [PATCH 1/1] [tosquash] test: add a test for sync heartbeats

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

> Please see 1 comment below.
> 
> 24.06.2020 01:39, Vladislav Shpilevoy пишет:
>> Should be squashed into the commit closing 5100.
>> ---
>> Branch: http://github.com/tarantool/tarantool/tree/gh-4842-sync-replication
>> Issue: https://github.com/tarantool/tarantool/issues/4842
>>
>>   .../sync_replication_sanity.result            | 50 +++++++++++++++++++
>>   .../sync_replication_sanity.test.lua          | 22 ++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/test/replication/sync_replication_sanity.result b/test/replication/sync_replication_sanity.result
>> index 4b9823d77..a0591dcf3 100644
>> --- a/test/replication/sync_replication_sanity.result
>> +++ b/test/replication/sync_replication_sanity.result
>> @@ -178,6 +178,53 @@ box.space.sync:select{}
>>    |   - [3]
>>    | ...
>>   +--
>> +-- gh-5100: replica should send ACKs for sync transactions after
>> +-- WAL write immediately, not waiting for replication timeout or
>> +-- a CONFIRM.
>> +--
>> +box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
>> + | ---
>> + | ...
> 
> You should remember previous replication_timeout here and set it back during cleanup.

Oh shit, you are right. I looked at how you restore replication_synchro_timeout
on line 98 and somewhy decided that the normal timeout is also restored.

I also found that instead of configuring master and replica I configured the
replica 2 times. Because I thought that the previous tests ends in 'default'
instance. Fixed this too.

Force pushed to this commit.

====================
diff --git a/test/replication/sync_replication_sanity.result b/test/replication/sync_replication_sanity.result
index a0591dcf3..8b37ba6f5 100644
--- a/test/replication/sync_replication_sanity.result
+++ b/test/replication/sync_replication_sanity.result
@@ -90,10 +90,10 @@ box.schema.user.grant('guest', 'replication')
  | ---
  | ...
 -- Set up synchronous replication options.
-quorum = box.cfg.replication_synchro_quorum
+old_synchro_quorum = box.cfg.replication_synchro_quorum
  | ---
  | ...
-timeout = box.cfg.replication_synchro_timeout
+old_synchro_timeout = box.cfg.replication_synchro_timeout
  | ---
  | ...
 box.cfg{replication_synchro_quorum=2, replication_synchro_timeout=0.1}
@@ -186,16 +186,15 @@ box.space.sync:select{}
 box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
  | ---
  | ...
-test_run:switch('replica')
+test_run:switch('default')
  | ---
  | - true
  | ...
-box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
+old_timeout = box.cfg.replication_timeout
  | ---
  | ...
-test_run:switch('default')
+box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
  | ---
- | - true
  | ...
 -- Commit something non-sync. So as applier writer fiber would
 -- flush the pending heartbeat and go to sleep with the new huge
@@ -231,7 +230,11 @@ test_run:cmd('switch default')
  | - true
  | ...
 
-box.cfg{replication_synchro_quorum=quorum, replication_synchro_timeout=timeout}
+box.cfg{                                                                        \
+    replication_synchro_quorum = old_synchro_quorum,                            \
+    replication_synchro_timeout = old_synchro_timeout,                          \
+    replication_timeout = old_timeout,                                          \
+}
  | ---
  | ...
 test_run:cmd('stop server replica')
diff --git a/test/replication/sync_replication_sanity.test.lua b/test/replication/sync_replication_sanity.test.lua
index f769804ca..b0326fd4b 100644
--- a/test/replication/sync_replication_sanity.test.lua
+++ b/test/replication/sync_replication_sanity.test.lua
@@ -38,8 +38,8 @@ engine = test_run:get_cfg('engine')
 
 box.schema.user.grant('guest', 'replication')
 -- Set up synchronous replication options.
-quorum = box.cfg.replication_synchro_quorum
-timeout = box.cfg.replication_synchro_timeout
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+old_synchro_timeout = box.cfg.replication_synchro_timeout
 box.cfg{replication_synchro_quorum=2, replication_synchro_timeout=0.1}
 
 test_run:cmd('create server replica with rpl_master=default,\
@@ -77,9 +77,9 @@ box.space.sync:select{}
 -- a CONFIRM.
 --
 box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
-test_run:switch('replica')
-box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
 test_run:switch('default')
+old_timeout = box.cfg.replication_timeout
+box.cfg{replication_timeout = 1000, replication_synchro_timeout = 1000}
 -- Commit something non-sync. So as applier writer fiber would
 -- flush the pending heartbeat and go to sleep with the new huge
 -- replication timeout.
@@ -95,7 +95,11 @@ box.space.sync:select{4}
 -- Cleanup.
 test_run:cmd('switch default')
 
-box.cfg{replication_synchro_quorum=quorum, replication_synchro_timeout=timeout}
+box.cfg{                                                                        \
+    replication_synchro_quorum = old_synchro_quorum,                            \
+    replication_synchro_timeout = old_synchro_timeout,                          \
+    replication_timeout = old_timeout,                                          \
+}
 test_run:cmd('stop server replica')
 test_run:cmd('delete server replica')
 box.space.test:drop()