- Oct 16, 2020
-
-
Alexander Turenko authored
It is the same as luaT_tuple_new(), but returns raw MsgPack data (not <box_tuple_t>) allocated on the box region. The reason to expose this function is to provide ability to use box_tuple_compare_with_key() function from an external module for a key passed as a Lua table. The compare function has the following signature: | API_EXPORT int | box_tuple_compare_with_key(box_tuple_t *tuple_a, const char *key_b, | box_key_def_t *key_def); The second parameter is a key encoded as an MsgPack array, not a tuple structure. So luaT_tuple_new() is not applicable here (it is not worthful to create a tuple structure if we need just MsgPack data). Some complexity was introduced to support encoding on the Lua shared buffer and the box region both. The motivation is the following: - luaT_tuple_encode() is exposed with encoding to the box region, because it is more usual to the module API. In particular a user of the API able to control when the tuple data should be released. - Encoding to the Lua shared buffer is kept internally, because there is no strong reason to change it to the box region for box.tuple.new(). Part of #5273
-
Alexander Turenko authored
This change fixes incorrect behaviour at tuple serialization error in several places: <space_object>:frommap(), <key_def_object>:compare(), <merge_source>:select(). See more in #5382. Disallow creating a tuple from objects on the Lua stack (idx == 0) in luaT_tuple_new() for simplicity. There are no such usages in tarantool. The function is not exposed yet to the module API. This is only necessary in box.tuple.new(), which anyway raises Lua errors by its contract. The better way to implement it would be rewritting of serialization from Lua to msgpack without raising Lua errors, but it is labirous work. Some day, I hope, I'll return here. Part of #5273 Fixes #5382
-
Alexander Turenko authored
It is useful to provide a module specific error when cdata expected, but a value of another type is passed. Alternative would be using of lua_type() to check against LUA_TCDATA, but this constant is not exposed for modules. See more in the luaL_iscdata() API comment. Part of #5273
-
Alexander Turenko authored
It is the better alternative to linking the small library directly to a module. Why not just use the small library in a module? Functions from an executable are preferred over ones that are shipped in a dynamic library (on Linux, Mac OS differs), while a module may use the small library directly. It may cause a problem when some functions from the library are inlined, but some are not, and those different versions of the library offer structures with different layouts. Small library symbols may be exported by the tarantool executable after the change of default symbols visibility (see [1]). See more details and examples in [2]. So it is better to expose so called box region and get rid of all those compatibility problems. [1]: 2.5.0-42-g03790ac55 ('cmake: remove dynamic-list linker option') [2]: https://lists.tarantool.org/pipermail/tarantool-discussions/2020-September/000095.html Part of #5273
-
Alexander Turenko authored
Technically C99 forbids it. Clang complains about typedef redefinitions when C99 is used (it is default prior to clang 3.6): | error: redefinition of typedef 'box_tuple_t' is a C11 feature | [-Werror,-Wtypedef-redefinition] | error: redefinition of typedef 'box_key_def_t' is a C11 feature | [-Werror,-Wtypedef-redefinition] The generated module.h file should define a type using typedef once. This patch moves extra definitions out of public parts of header files. Reordered api headers to place usages of such types after definitions. Set C99 for the module API test in order to catch problems of this kind in a future. Fixed 'unused value' warnings, which appears after the change (it is strange that -Wall was not passed here before). Part of #5273 Fixes #5313
-
- Oct 14, 2020
-
-
Nikita Pettik authored
Previous upsert implementation had a few drawbacks which led to number of various bugs and issues. Issue #5092 (redundant update operations execution) In a nutshell, application of upsert(s) (on top of another upsert) consists of two actions (see vy_apply_upsert()): execute and squash. Consider example: insert({1, 1}) -- terminal statement, stored on disk upsert({1}, {{'-', 2, 20}}) -- old ups1 upsert({1}, {{'+', 2, 10}}) -- new ups2 'Execute' step takes update operations from the new upsert and combines them with key of the old upsert. {1} + {'+', 2, 10} can't be evaluated since key consists of only one field. Note that in case upsert doesn't fold into insert the upsert's tuple and the tuple stored in index can be different. In our particular case, tuple stored on disk has two fields ({1, 1}), so first upsert's update operation can be applied to it: {1, 1} + {'+', 2, 10} --> {1, 11}. If upsert's operation can't be executed using key of old upsert, we simply continue processing squash step. In turn 'squash' is a combination of update operations: arithmetic operations are combined so we don't have to store actions over the same field; the rest operations - are merged into single array. As a result, we get one upsert with squashed operations: upsert({1}, {{'+', 2, -10}}). Then vy_apply_upsert() is called again to apply new upsert on the top of terminal statement - insert{1, 1}. Since now tuple has second field, update operations can be executed and corresponding result is {1, -9}. It is the final result of upsert application procedure. Now imagine that we have following upserts: upsert({1, 1}, {{'-', 2, 20}}) -- old ups1 upsert({1}, {{'+', 2, 10}}) -- new ups2 In this case execution successfully finishes and modifies old upsert's tuple: {1, 1} + {'+', 2, 10} --> {1, 11} However, we still have to squash/accumulate update operations since they may be applied on tuple stored on disk later. After all, we have following upsert: upsert({2, 11}, {{'+', 2, -10}}). Then it is applied on the top of insert({1, 1}) and we get the same result as in the first case - {1, -9}. The only difference is that upsert's tuple was modified. As one can see, execution of update operations applied to upsert's tuple is redundant in the case index already contains tuple with the same key (i.e. when upserts turns into update). Instead, we are able to accumulate/squash update operations only. When the last upsert is being applied, we can either execute all update operation on tuple fetched from index (i.e. upsert is update) OR on tuple specified in the first upsert (i.e. first upsert is insert). Issue #5105 (upsert doesn't follow associative property) Secondly, current approach breaks associative property: after upsert's update operations are merged into one array, part of them (related to one upsert) can be skipped, meanwhile the rest - is applied. For instance: -- Index is over second field. i = s:create_index('pk', {parts={2, 'uint'}}) s:replace{1, 2, 3, 'default'} s:upsert({2, 2, 2}, {{'=', 4, 'upserted'}}) -- First update operation modifies primary key, so upsert must be ignored. s:upsert({2, 2, 2}, {{'#', 1, 1}, {'!', 3, 1}}) After merging two upserts we get the next one: upsert({2, 2, 2}, {{'=', 4, 'upserted'}, {'#', 1, 1}, {'!', 3, 1}} While we executing update operations, we don't distinguish operations from different upserts. Thus, if one operation fails, the rest are ignored as well. As a result, first (in general case - all preceding squashed upserts) upsert won't be applied, even despite the fact it is absolutely correct. What is more, user gets no error/warning concerning this fact. Issue #1622 (no upsert result validation) After upsert application, there's no check verifying that result satisfies space's format: number of fields, their types, overflows etc. Due to this tuples violating format may appear in the space, which in turn may lead to unpredictable consequences. To resolve these issues, let's group update operations of each upsert into separate array. So that operations related to particular upsert are stored in single array. In terms of previous example we will get: upsert({2, 2, 2}, {{{'=', 4, 'upserted'}}, {{'#', 1, 1}, {'!', 3, 1}}} Also note that we don't longer have to apply update operations on tuple in vy_apply_upsert() when it comes for two upserts: it can be done once we face terminal statement; or if there's no underlying statement (i.e. it is delete statement or no statement at all) we apply all update arrays except the first one on upsert's tuple. In case one of operations from array fail, we skip the rest operations from this array and process to the next array. After successful application of update operations of each array, we check that the resulting tuple fits into space format. If they aren't, we rollback applied operations, log error and moving to the next group of operations. Finally, arithmetic operations are not longer able to be combined: it is requirement which is arises from #5105 issue. Otherwise, result of upserts combination might turn out to be inapplicable to the tuple stored on disk (e.g. result applied on tuple leads to integer overflow - in this case only last upsert leading to overflow must be ignored). Closes #1622 Closes #5105 Closes #5092 Part of #5107
-
Vladislav Shpilevoy authored
According to Raft, when a new leader is elected, it should finish transactions of the old leader. In Raft this is done via adding a new transaction originated from the new leader. In case of Tarantool this can be done without a new transaction due to WAL format specifics, and the function doing it is called box_clear_synchro_queue(). Before the patch, when a node was elected as a leader, it didn't finish the pending transactions. The queue clearance was expected to be done by a user. There was no any issue with that, just technical debt. The patch fixes it. Now when a node becomes a leader, it finishes synchronous transactions of the old leader. This is done a bit differently than in the public box.ctl.clear_synchro_queue(). The public box.ctl.clear_synchro_queue() tries to wait for CONFIRM messages, which may be late. For replication_synchro_timeout * 2 time. But when a new leader is elected, the leader will ignore all rows from all the other nodes, as it thinks it is the only source of truth. Therefore it makes no sense to wait for CONFIRMs here, and the waiting is omitted. Closes #5339
-
Vladislav Shpilevoy authored
The test is long, about 10 seconds. But its name is too general. And it would be better used for a simpler more basic test. This is going to happen in the next commits. election_qsync.test.lua will check if the election and qsync work fine together without any stress cases. Needed for #5339
-
Alexander V. Tikhonov authored
Added new checksum for flaky fail on vinyl/gh.test.lua:427 line. Part of #5141
-
Alexander V. Tikhonov authored
On heavy loaded hosts found the following issue: [151] --- replication/replica_rejoin.result Tue Sep 29 10:57:26 2020 [151] +++ replication/replica_rejoin.reject Tue Sep 29 10:57:48 2020 [151] @@ -230,7 +230,12 @@ [151] return box.info ~= nil and box.info.replication[1] ~= nil [151] end) [151] --- [151] -- true [151] +- error: "builtin/box/load_cfg.lua:601: Please call box.cfg{} first\nstack traceback:\n\tbuiltin/box/load_cfg.lua:601: [151] + in function '__index'\n\t[string \"return test_run:wait_cond(function() ...\"]:1: [151] + in function 'cond'\n\t/tmp/tnt/151_replication/test_run.lua:411: in function </tmp/tnt/151_replication/test_run.lua:404>\n\t[C]: [151] + in function 'pcall'\n\tbuiltin/box/console.lua:402: in function 'eval'\n\tbuiltin/box/console.lua:708: [151] + in function 'repl'\n\tbuiltin/box/console.lua:842: in function <builtin/box/console.lua:828>\n\t[C]: [151] + in function 'pcall'\n\tbuiltin/socket.lua:1081: in function <builtin/socket.lua:1079>" [151] ... [151] test_run:wait_upstream(1, {message_re = 'Missing %.xlog file', status = 'loading'}) [151] --- [151] It happened because box.cfg was not ready to provide information. In real there is no need to use local check for replication information parts availablity, due to wait_upstream() function used below, do it itself. Part of #4985
-
- Oct 13, 2020
-
-
Igor Munkin authored
Fixes the regression from e5039742 ('luajit: bump new version'). Reported-by:
Alexander Tikhonov <avtikhon@tarantool.org> Signed-off-by:
Igor Munkin <imun@tarantool.org>
-
Ilya Kosarev authored
key_def didn't support key definitions with array, map, varbinary & any fields. Thus they couldn't be extracted with key_def_object:extract_key(). Since the restriction existed due to impossibility of such types comparison, this patch removes the restriction for the fields extraction and only leaves it for comparison. Closes #4538
-
Alexander V. Tikhonov authored
Added for tests with issues: app/socket.test.lua gh-4978 box/access.test.lua gh-5411 box/access_misc.test.lua gh-5401 box/gh-5135-invalid-upsert.test.lua gh-5376 box/hash_64bit_replace.test.lua test gh-5410 box/hash_replace.test.lua gh-5400 box/huge_field_map_long.test.lua gh-5375 box/net.box_huge_data_gh-983.test.lua gh-5402 replication/anon.test.lua gh-5381 replication/autoboostrap.test.lua gh-4933 replication/box_set_replication_stress.test.lua gh-4992 replication/election_basic.test.lua gh-5368 replication/election_qsync.test.lua test gh-5395 replication/gh-3247-misc-iproto-sequence-value-not-replicated.test.lua gh-5380 replication/gh-3711-misc-no-restart-on-same-configuration.test.lua gh-5407 replication/gh-5287-boot-anon.test.lua gh-5412 replication/gh-5298-qsync-recovery-snap.test.lua.test.lua gh-5379 replication/show_error_on_disconnect.test.lua gh-5371 replication/status.test.lua gh-5409 swim/swim.test.lua gh-5403 unit/swim.test gh-5399 vinyl/gc.test.lua gh-5383 vinyl/gh-4864-stmt-alloc-fail-compact.test.lua test gh-5408 vinyl/gh-4957-too-many-upserts.test.lua gh-5378 vinyl/gh.test.lua gh-5141 vinyl/quota.test.lua gh-5377 vinyl/snapshot.test.lua gh-4984 vinyl/stat.test.lua gh-4951 vinyl/upsert.test.lua gh-5398
-
Alexander V. Tikhonov authored
Testing on FreeBSD 12 had some tests previously blocked to avoid of flaky fails. For now we have the ability to avoid of it in test-run using checksums for fails with opened issues. So adding back 7 tests to testing on FreeBSD 12. Closes #4271
-
Alexander V. Tikhonov authored
Set error message to log output in test: vinyl/gc.test.lua
-
Alexander V. Tikhonov authored
Set error message to log output in test: vinyl/snapshot.test.lua
-
Alexander V. Tikhonov authored
Set error message to log output in test: replication/gh-4402-info-errno.test.lua
-
Alexander V. Tikhonov authored
Set error message to log output in test: replication/replica_rejoin.test.lua
-
Alexander V. Tikhonov authored
Set error message to log output in test: replication/gh-3160-misc-heartbeats-on-master-changes.test.lua
-
- Oct 12, 2020
-
-
Vladislav Shpilevoy authored
The new option can be one of 3 values: 'off', 'candidate', 'voter'. It replaces 2 old options: election_is_enabled and election_is_candidate. These flags looked strange, that it was possible to set candidate true, but disable election at the same time. Also it would not look good if we would ever decide to introduce another mode like a data-less sentinel node, for example. Just for voting. Anyway, the single option approach looks easier to configure and to extend. - 'off' means the election is disabled on the node. It is the same as election_is_enabled = false in the old config; - 'voter' means the node can vote and is never writable. The same as election_is_enabled = true + election_is_candidate = false in the old config; - 'candidate' means the node is a full-featured cluster member, which eventually may become a leader. The same as election_is_enabled = true + election_is_candidate = true in the old config. Part of #1146
-
- Oct 07, 2020
-
-
Aleksandr Lyapunov authored
space:fselect and index:fselect fetch data like ordinal select, but formats the result like mysql does - with columns, column names etc. fselect converts tuple to strings using json, extending with spaces and cutting tail if necessary. It is designed for visual analysis of select result and shouldn't be used stored procedures. index:fselect(<key>, <opts>, <fselect_opts>) space:fselect(<key>, <opts>, <fselect_opts>) There are some options that can be specified in different ways: - among other common options (<opts>) with 'fselect_' prefix. (e.g. 'fselect_type=..') - in special <fselect_opts> map (with or without prefix). - in global variables with 'fselect_' prefix. The possible options are: - type: - 'sql' - like mysql result (default). - 'gh' (or 'github' or 'markdown') - markdown syntax, for copy-pasting to github. - 'jira' - jira table syntax (for copy-pasting to jira). - widths: array with desired widths of columns. - max_width: limit entire length of a row string, longest fields will be cut if necessary. Set to 0 (default) to detect and use screen width. Set to -1 for no limit. - print: (default - false) - print each line instead of adding to result. - use_nbsp: (default - true) - add invisible spaces to improve readability in YAML output. Not applicabble when print=true. There is also a pair of shortcuts: index/space:gselect - same as fselect, but with type='gh'. index/space:jselect - same as fselect, but with type='jira'. See test/engine/select.test.lua for examples. Closes #5161
-
- Oct 06, 2020
-
-
Igor Munkin authored
While running GC hook (i.e. __gc metamethod) garbage collector engine is "stopped": the memory penalty threshold is set to LJ_MAX_MEM and incremental GC step is not triggered as a result. Ergo, yielding the execution at the finalizer body leads to further running platform with disabled LuaJIT GC. It is not re-enabled until the yielded fiber doesn't get the execution back. This changeset extends <cord_on_yield> routine with the check whether GC hook is active. If the switch-over occurs in scope of __gc metamethod the platform is forced to stop its execution with EXIT_FAILURE and calls panic routine before the exit. Relates to #4518 Follows up #4727 Reviewed-by:
Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Reviewed-by:
Sergey Ostanevich <sergos@tarantool.org> Signed-off-by:
Igor Munkin <imun@tarantool.org>
-
Serge Petrenko authored
-
- Oct 02, 2020
-
-
Igor Munkin authored
Since Tarantool fibers don't respect Lua coroutine switch mechanism, JIT machinery stays unnotified when one lua_State substitutes another one. As a result if trace recording hasn't been aborted prior to fiber switch, the recording proceeds using the new lua_State and leads to a failure either on any further compiler phase or while the compiled trace is executed. This changeset extends <cord_on_yield> routine aborting trace recording when the fiber switches to another one. If the switch-over occurs while mcode is being run the platform finishes its execution with EXIT_FAILURE code and calls panic routine prior to the exit. Closes #1700 Fixes #4491 Reviewed-by:
Sergey Ostanevich <sergos@tarantool.org> Reviewed-by:
Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Signed-off-by:
Igor Munkin <imun@tarantool.org>
-
Igor Munkin authored
Tarantool integrates several complex environments together and there are issues occurring at their junction leading to the platform failures. E.g. fiber switch-over is implemented outside the Lua world, so when one lua_State substitutes another one, main LuaJIT engines, such as JIT and GC, are left unnotified leading to the further platform misbehaviour. To solve this severe integration drawback <cord_on_yield> function is introduced. This routine encloses the checks and actions to be done when the running fiber yields the execution. Unfortunately the way callback is implemented introduces a circular dependency. Considering linker symbol resolving methods for static build an auxiliary translation unit is added to the particular tests mocking (i.e. exporting) <cord_on_yield> undefined symbol. Part of #1700 Relates to #4491 Reviewed-by:
Sergey Ostanevich <sergos@tarantool.org> Reviewed-by:
Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Signed-off-by:
Igor Munkin <imun@tarantool.org>
-
- Oct 01, 2020
-
-
Cyrill Gorcunov authored
Convert to uint64_t explicitly. Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
- Sep 29, 2020
-
-
Vladislav Shpilevoy authored
Part of #1146
-
Vladislav Shpilevoy authored
Box.info.election returns a table of form: { state: <string>, term: <number>, vote: <instance ID>, leader: <instance ID> } The fields correspond to the same named Raft concepts one to one. This info dump is supposed to help with the tests, first of all. And with investigation of problems in a real cluster. The API doesn't mention 'Raft' on purpose, to keep it not depending specifically on Raft, and not to confuse users who don't know anything about Raft (even that it is about leader election and synchronous replication). Part of #1146
-
Vladislav Shpilevoy authored
The new options are: - election_is_enabled - enable/disable leader election (via Raft). When disabled, the node is supposed to work like if Raft does not exist. Like earlier; - election_is_candidate - a flag whether the instance can try to become a leader. Note, it can vote for other nodes regardless of value of this option; - election_timeout - how long need to wait until election end, in seconds. The options don't do anything now. They are added separately in order to keep such mundane changes from the main Raft commit, to simplify its review. Option names don't mention 'Raft' on purpose, because - Not all users know what is Raft, so they may not even know it is related to leader election; - In future the algorithm may change from Raft to something else, so better not to depend on it too much in the public API. Part of #1146
-
- Sep 28, 2020
-
-
Roman Khabibov authored
Ban ability to modify view on box level. Since a view is a named select, and not a table, in fact, altering view is not a valid operation.
-
Alexander V. Tikhonov authored
Added for tests with issues: app/fiber.test.lua gh-5341 app-tap/debug.test.lua gh-5346 app-tap/http_client.test.lua gh-5346 app-tap/inspector.test.lua gh-5346 box/gh-2763-session-credentials-update.test.lua gh-5363 box/hash_collation.test.lua gh-5247 box/lua.test.lua gh-5351 box/net.box_connect_triggers_gh-2858.test.lua gh-5247 box/net.box_incompatible_index-gh-1729.test.lua gh-5360 box/net.box_on_schema_reload-gh-1904.test.lua gh-5354 box/protocol.test.lua gh-5247 box/update.test.lua gh-5247 box-tap/net.box.test.lua gh-5346 replication/autobootstrap.test.lua gh-4533 replication/autobootstrap_guest.test.lua gh-4533 replication/ddl.test.lua gh-5337 replication/gh-3160-misc-heartbeats-on-master-changes.test.lua gh-4940 replication/gh-3247-misc-iproto-sequence-value-not-replicated.test.lua.test.lua gh-5357 replication/gh-3637-misc-error-on-replica-auth-fail.test.lua gh-5343 replication/long_row_timeout.test.lua gh-4351 replication/on_replace.test.lua gh-5344, gh-5349 replication/prune.test.lua gh-5361 replication/qsync_advanced.test.lua gh-5340 replication/qsync_basic.test.lua gh-5355 replication/replicaset_ro_mostly.test.lua gh-5342 replication/wal_rw_stress.test.lua gh-5347 replication-py/multi.test.py gh-5362 sql/prepared.test.lua test gh-5359 sql-tap/selectG.test.lua gh-5350 vinyl/ddl.test.lua gh-5338 vinyl/gh-3395-read-prepared-uncommitted.test.lua gh-5197 vinyl/iterator.test.lua gh-5336 vinyl/write_iterator_rand.test.lua gh-5356 xlog/panic_on_wal_error.test.lua gh-5348
-
- Sep 25, 2020
-
-
Alexander V. Tikhonov authored
Removed dust line from merge.
-
Alexander V. Tikhonov authored
In test-run implemented the new format of the fragile lists based on JSON format set as fragile option in 'suite.ini' files per each suite: fragile = { "retries": 10, "tests": { "bitset.test.lua": { "issues": [ "gh-4095" ], "checksums": [ "050af3a99561a724013995668a4bc71c", "f34be60193cfe9221d3fe50df657e9d3" ] } }} Added ability to check results file checksum on tests fail and compare with the checksums of the known issues mentioned in the fragile list. Also added ability to set 'retries' option, which sets the number of accepted reruns of the tests failed from 'fragile' list that have checksums on its fails. Closes #5050
-
Alexander V. Tikhonov authored
Found flaky issues multi running replication/anon.test.lua test on the single worker: [007] --- replication/anon.result Fri Jun 5 09:02:25 2020 [007] +++ replication/anon.reject Mon Jun 8 01:19:37 2020 [007] @@ -55,7 +55,7 @@ [007] [007] box.info.status [007] | --- [007] - | - running [007] + | - orphan [007] | ... [007] box.info.id [007] | --- [094] --- replication/anon.result Sat Jun 20 06:02:43 2020 [094] +++ replication/anon.reject Tue Jun 23 19:35:28 2020 [094] @@ -154,7 +154,7 @@ [094] -- Test box.info.replication_anon. [094] box.info.replication_anon [094] | --- [094] - | - count: 1 [094] + | - count: 2 [094] | ... [094] #box.info.replication_anon() [094] | --- [094] It happend because replications may stay active from the previous runs on the common tarantool instance at the test-run worker. To avoid of it added restarting of the tarantool instance at the very start of the test. Closes #5058
-
- Sep 23, 2020
-
-
Aleksandr Lyapunov authored
Closes #4897
-
Aleksandr Lyapunov authored
txn_proxy is a special utility for transaction tests. Formerly it was used only for vinyl tests and thus was placed in vinyl folder. Now the time has come to test memtx transactions and the utility must be placed amongst other utils - in box/lua. Needed for #4897
-
Aleksandr Lyapunov authored
Define memtx TX manager. It will store data for MVCC and conflict manager. Define also 'memtx_use_mvcc_engine' in config that enables that MVCC engine. Part of #4897
-
- Sep 18, 2020
-
-
Vladislav Shpilevoy authored
The test tried to start a replica whose box.cfg would hang, with replication_connect_quorum = 0 to make it return immediately. But the quorum parameter was added and removed during work on 44421317 ("replication: do not register outgoing connections"). Instead, to start the replica without blocking on box.cfg it is necessary to pass 'wait=False' with the test_run:cmd('start server') command. Closes #5311
-
- Sep 17, 2020
-
-
Vladislav Shpilevoy authored
Replication protocol's first stage for non-anonymous replicas is that the replica should be registered in _cluster to get a unique ID number. That happens, when replica connects to a writable node, which performs the registration. So it means, registration always happens on the master node when appears an *incoming* request for it, explicitly asking for a registration. Only relay can do that. That wasn't the case for bootstrap. If box.cfg.replication wasn't empty on the master node doing the cluster bootstrap, it registered all the outgoing connections in _cluster. Note, the target node could be even anonymous, but still was registered. That breaks the protocol, and leads to registration of anon replicas sometimes. The patch drops it. Another motivation here is Raft cluster bootstrap specifics. During Raft bootstrap it is going to be very important that non-joined replicas should not be registered in _cluster. A replica can only register after its JOIN request was accepted, and its snapshot download has started. Closes #5287 Needed for #1146
-
Vladislav Shpilevoy authored
Previously XlogGapError was considered a critical error stopping the replication. That may be not so good as it looks. XlogGapError is a perfectly fine error, which should not kill the replication connection. It should be retried instead. Because here is an example, when the gap can be recovered on its own. Consider the case: node1 is a leader, it is booted with vclock {1: 3}. Node2 connects and fetches snapshot of node1, it also gets vclock {1: 3}. Then node1 writes something and its vclock becomes {1: 4}. Now node3 boots from node1, and gets the same vclock. Vclocks now look like this: - node1: {1: 4}, leader, has {1: 3} snap. - node2: {1: 3}, booted from node1, has only snap. - node3: {1: 4}, booted from node1, has only snap. If the cluster is a fullmesh, node2 will send subscribe requests with vclock {1: 3}. If node3 receives it, it will respond with xlog gap error, because it only has a snap with {1: 4}, nothing else. In that case node2 should retry connecting to node3, and in the meantime try to get newer changes from node1. The example is totally valid. However it is unreachable now because master registers all replicas in _cluster before allowing them to make a join. So they all bootstrap from a snapshot containing all their IDs. This is a bug, because such auto-registration leads to registration of anonymous replicas, if they are present during bootstrap. Also it blocks Raft, which can't work if there are registered, but not yet joined nodes. Once the registration problem will be solved in a next commit, the XlogGapError will strike quite often during bootstrap. This patch won't allow that happen. Needed for #5287
-