- Oct 16, 2020
-
-
Timur Safin authored
Introduced the bare minimum of ibuf accessors, which make external merger possible: - box_ibuf_reserve - box_ibuf_read_range - box_ibuf_write_range Part of #5384
-
Alexander Turenko authored
box_key_def_validate_key() verifies a probably partial key, while the new function is to apply the extra restriction on the key part count: it should be the same as in a given key definition. Fixes #5273
-
Alexander Turenko authored
The function allows to verify a key against a key definition. It accepts a partial key. Part of #5273
-
Alexander Turenko authored
Unlike box_tuple_extract_key() it accepts a key_def structure, not space_id, index_id pair. Another difference from box_tuple_extract_key() is that this function allows to pass so called multikey index. See commit 2.2.0-259-gf1d9f2575 ('box: introduce multikey indexes in memtx') for details. Note: The <multikey_idx> parameter is ignored on the backported version of the patch on 1.10. Part of #5273
-
Alexander Turenko authored
Part of #5273
-
Alexander Turenko authored
Part of #5273
-
Alexander Turenko authored
The function dumps an opaque <box_key_def_t> structure into a non-opaque array of <box_key_part_def_t> structures in order to allow an external module to obtain information about the key definition. Part of #5273
-
Alexander Turenko authored
Unlike box_key_def_new() it allows to set nullability, collation and JSON path. Note: JSON paths are not supported in the backported version of the patch for 1.10. Provided public non-opaque key part definition structure to create a key definition. The next commit will also use this structure to dump a key definition. There are several technical points around the box_key_part_def_t structure. They are mainly about providing stable ABI. - Two uint32_t fields are placed first for better aligning of next fields (pointers, which are usually 64 bit wide). - A padding is used to guarantee that the structure size will remain the same across tarantool versions. It allows to allocate an array of such structures. - The padding array is not a field of the structure itself, but added as a union variant (see the code). It allows to get rid of manual calculation of cumulative fields size, which is hard to do in a platform independent way. - A minimal size of the structure is guaranteed by the union with padding, but a static assert is required to guarantee that the size will not overrun the predefined value. - PACKED is added as an extra remedy to make the structure layout predictable (on given target architecture). - A bit flag is used for is_nullable. bool is considered as too expensive (it requires 8 bits). bitfields (int:1 and so on) do no guarantee certain data layout (it is compiler implementation detail), while a module is compiled outside of tarantool build and may use different toolchain. A bit flag is the only choice. - A collation is identified using a string. Different IDs may be used on different tarantool instances for collations. The only 'real' identifier is a collation name, so using it as identifier in the API should be more convenient and less error-prone. - A field type is also identified using a string instead of a number. We have <enum field_type> in the module API, but it cannot be used here, because IDs are changed across tarantool versions. Aside of this, size of a enum is compiler defined. Anyway, we can expose field types as numbers and implement number-to-name and name-to-number mapping functions, but IMHO it would just add extra complexity. The dependency on the fiber compilation unit is added for key_def: it is purely to obtain the box region in the module API functions. The key_def compilation unit does not use anything fiber related in fact. Part of #5273
-
Alexander Turenko authored
It is the rule of thumb to use API_EXPORT with module API functions. To be honest, I don't see strict reason to use this macro except for unification with other module API functions. It adds 'extern', which is default for functions. It adds __attribute__((visibility("default"))), but symbols to be exported are listed in extra/exports or src/export.h (depending of tarantool version, see [1]). It adds __attribute__((nothrow)), which maybe allows a compiler to produce more optimized code and also catch an obvious problem and emit a warning. I don't know. So the reason for me is unification. Part of #5273 [1]: 2.5.0-42-g03790ac55 ('cmake: remove dynamic-list linker option') [2]: 1.6.8-71-g55605c5c9 ('Add __attribute__((nothrow)) to API_EXPORT macro')
-
Alexander Turenko authored
The reason is unification of declarations. It is the rule of thumb to use API_EXPORT with module API functions. Part of #5273
-
Alexander Turenko authored
It is convenient wrapper around box_tuple_new() to create a tuple from a Lua table (or from another tuple). Part of #5273
-
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 simplifies further changes around encoding a tuple: next commits will get rid of throwing serialization errors and will expose a function to encode a table (or tuple) from the Lua stack on the box region to the module API. Part of #5273 Part of #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 15, 2020
-
-
Alexander Turenko authored
We should remove a tag after fetching of a remote repository. It is hotfix of commit 0f564f34 ('gitlab-ci: remove tag from pushed branch commit'). Follows up #3745 Co-authored-by:
Alexander V. Tikhonov <avtikhon@tarantool.org>
-
Kirill Yukhin authored
* jit: fix cdatanum addressing for GC64 mode on x86
-
Kirill Yukhin authored
* test: force enable assert checks in release build
-
Alexander V. Tikhonov authored
Found on GCC 4.8.5 on CentOS 7 issue: build/usr/src/debug/tarantool-2.6.0.144/src/box/txn.c:944:30: error: ‘limbo_entry’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (txn_limbo_wait_complete(&txn_limbo, limbo_entry) < 0) Set limbo_entry variable to NULL on initialization. Needed for #4941
-
Kirill Yukhin authored
* jit: fix cdatanum addressing for GC64 mode on x86
-
- Oct 14, 2020
-
-
Alexander V. Tikhonov authored
Drop a tag that points to a current commit (if any) on a job triggered by pushing to a branch (as against of pushing a tag). Otherwise we may get two jobs for the same x.y.z-0-gxxxxxxxxx build: one is run by pushing a branch and another by pushing a tag. The idea is to hide the new tag from the branch job as if a tag would be pushed strictly after all branch jobs for the same commit. Closes #3745 Co-authored-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Nikita Pettik authored
After previous commit, there's no need in these functions. However, someday we may want to reconsider it squash optimization and make it work without breaking upsert associativity rule. So to not lost initial squash implementation, let's put its removal in a separate patch. Follow-up #5107
-
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
-
Artem Starshov authored
Changed passing global variable arg to function find_instance_name(arg) instead of passing arg[0] and arg[2] separately. And removed exception in .luacheckrc for file /extra/dist/tarantoolctl.in. This change only solves linter warning, nothing else. Fixed #4929. Reviewed-by:
Leonid Vasiliev <lvasiliev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Vladislav Shpilevoy authored
Order of LSNs from different instances can be any. It means, that if the synchronous transaction limbo changed its owner, the old value of maximal confirmed LSN does not make sense. The new owner means new LSNs, even less than the previously confirmed LSN of an old owner. The patch resets the confirmed LSN when the owner is changed. No specific test for that, as this is a hotfix - tests start fail on every run after a new election test was added in the previous commit. A test for this bug will be added later. Part of #5395
-
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
Raft state machine now has a trigger invoked each time when any of the visible Raft attributes is changed: state, term, vote. The trigger is needed to commit synchronous transactions of an old leader, when a new leader is elected. This is done via a trigger so as not to depend on box in raft code too much. That would make it harder to extract it into a new module later. The trigger is executed in the Raft worker fiber, so as not to stop the state machine transitions anywhere, which currently don't contain a single yield. And the synchronous transaction queue clearance requires a yield, to write CONFIRM and ROLLBACK records to WAL. Part of #5339
-
Vladislav Shpilevoy authored
When a raft node was configured to be a candidate via election_mode, it didn't do anything if there was an active leader. But it should have started monitoring its health in order to initiate a new election round when it dies. The patch fixes this bug. It does not contain a test, because will be covered by a test for #5339. Needed for #5339
-
Vladislav Shpilevoy authored
Raft has a worker fiber to perform async tasks such as WAL write, state broadcast. The worker was created and woken up from 2 places, leading at least to code duplication. The patch wraps it into a new function raft_worker_wakeup(), and uses it. The patch is not need for anything functional, but was created while working on #5339 and trying ideas. The patch seems to be good refactoring making the code simpler, and therefore it is submitted.
-
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
-
Kirill Yukhin authored
* misc: add C and Lua API for platform metrics * core: introduce various platform metrics
-
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
Met flaky issues on test: replication/gh-3637-misc-error-on-replica-auth-fail.test.lua Found memory leaks: [093] Last 15 lines of Tarantool Log file [Instance "replica_auth"][/builds/DtQXhC5e/0/tarantool/tarantool/test/var/093_replication/replica_auth.log]: [093] #3 0xa13df8 in coio_on_call /builds/DtQXhC5e/0/tarantool/tarantool/src/lib/core/coio_task.c:264:16 [093] #4 0xfcedbe in eio_execute /builds/DtQXhC5e/0/tarantool/tarantool/third_party/libeio/eio.c:2015:9 [093] #5 0xfcedbe in etp_proc /builds/DtQXhC5e/0/tarantool/tarantool/third_party/libeio/etp.c:373 [093] #6 0x7f8c8260ffa2 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7fa2) [093] [093] Indirect leak of 4 byte(s) in 1 object(s) allocated from: [093] #0 0x525dfa in calloc (/builds/DtQXhC5e/0/tarantool/tarantool/src/tarantool+0x525dfa) [093] #1 0xa2eb4a in mh_i64ptr_new /builds/DtQXhC5e/0/tarantool/tarantool/src/lib/salad/mhash.h:408:22 [093] #2 0x8a516d in vy_recovery_new_f /builds/DtQXhC5e/0/tarantool/tarantool/src/box/vy_log.c:2321:23 [093] #3 0xa13df8 in coio_on_call /builds/DtQXhC5e/0/tarantool/tarantool/src/lib/core/coio_task.c:264:16 [093] #4 0xfcedbe in eio_execute /builds/DtQXhC5e/0/tarantool/tarantool/third_party/libeio/eio.c:2015:9 [093] #5 0xfcedbe in etp_proc /builds/DtQXhC5e/0/tarantool/tarantool/third_party/libeio/etp.c:373 [093] #6 0x7f8c8260ffa2 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7fa2) To stabilize testing these leaks added as suppressions to asan list. Part of #5343
-