- Nov 15, 2019
-
-
Alexander Turenko authored
The problem appears after 6c627af3 ('test: tarantoolctl: verify delayed box.cfg()'), where the test case was changed and it doesn't more assume an error at the instance start. So we need to stop it to prevent a situation when instances are stay after `make test`. Fixes #4600. Reviewed-by:
Vladislav Shpilevoy <v.shpilevoy@tarantool.org> (cherry picked from commit 8d363c43)
-
- Nov 14, 2019
-
-
Alexander Turenko authored
Before commit 03f85d4c ('app: fix boolean handling in argparse module') the module does not expect a value after a 'boolean' argument. However there was the problem: a 'boolean' argument can be passed only at end of an argument list, otherwise it wrongly consumes a next argument and gives a confusing error message. The mentioned commit fixes this behaviour in the following way: it still allows to pass a 'boolean' argument at end of the list w/o a value, but requires a value ('true', 'false', '1', '0') if a 'boolean' argument is not at the end to be provided using {'--foo=true'} or {'--foo', 'true'} syntax. Here this behaviour is changed: a 'boolean' argument does not assume an explicitly passed value despite its position in an argument list. If a 'boolean' argument appears in the list, then argparse.parse() returns `true` for its value (a list of `true` values in case of 'boolean+' argument), otherwise it will not be added to the result. This change also makes the behaviour of long (--foo) and short (-f) 'boolean' options consistent. The motivation of the change is simple: it is easier and more natural to type, say, `tarantoolctl cat --show-system 00000000000000000000.snap` then `tarantoolctl cat --show-system true 00000000000000000000.snap`. This commit adds several new test cases, but it does not mean that we guarantee that the module behaviour will not be changed around some corner cases, say, handling of 'boolean+' arguments. This is internal module. Follows up #4076. Reviewed-by:
Vladislav Shpilevoy <v.shpilevoy@tarantool.org> (cherry picked from commit e47f2c91)
-
- Nov 12, 2019
-
-
Vladislav Shpilevoy authored
The admin user has universal privileges before bootstrap or recovery are done. That allows to, for example, bootstrap from a remote master, because to do that the admin should be able to insert into system spaces, such as _priv. But after the patch on online credentials update was implemented (#2763, 48d00b0e) the admin could loose its universal access if, for example, a role was granted to him before universal access was recovered. That happened by two reasons: - Any change in access rights, even in granted roles, led to rebuild of universal access; - Any change in access rights updated the universal access in all existing sessions, thanks to #2763. What happened: two tarantools were started. One of them master, granted 'replication' role to admin. Second node, slave, tried to bootstrap from the master. The slave created an admin session and started loading data. After it loaded 'grant replication role to admin' command, this nullified admin universal access everywhere, including this session. Next rows could not be applied. Closes #4606 (cherry picked from commit 95237ac8)
-
- Nov 11, 2019
-
-
Alexander V. Tikhonov authored
After the issue #4537 fixed for the data segment size limit, the temporary blocked tests because of it unblocked. Part of #4271 (cherry picked from commit e6866550)
-
- Nov 08, 2019
-
-
Cyrill Gorcunov authored
When invalid command is passed we should send an error message to a client. Instead a nil dereference occurs that causes abnormal exit of a console. This is the regression from 96dbc49d ('box/console: Refactor command handling'). Reported-by:
Mergen Imeev <imeevma@tarantool.org> Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org> (cherry picked from commit ada8c97c)
-
Alexander V. Tikhonov authored
Added build + test jobs in GitLab-CI and build + test + deploy jobs on Travis-CI for CentOS 8. Updated testing dependencies in the RPM spec to follow the new Python 2 package naming scheme that was introduced in CentOS 8: it uses 'python2-' prefix rather then 'python-'. CentOS 8 does not provide python2-gevent and python2-greenlet packages, so they were pushed to https://packagecloud.io/packpack/backports repository. This repository is enabled in our build image (packpack/packpack:el-8) by default. Those dependencies are build-time, so nothing was changed for a user. The source RPM packages were gathered from https://cbs.centos.org . Disabled app-tap/pwd.test.lua on CentOS 8 due to systemd-nss issue, which was not worked around properly. Filed #4592 to resolved it in the future. Eliminated libunwind runtime dependency (and libunwind-devel build dependency) on CentOS 8, because the base system does not provide it. fiber.info() backtraces and printing of a backtrace after a crash will not be available on this system. Hopefully we'll fix it in the future, filed #4611 on this. Closes #4543 Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org> Reviewed-by:
Igor Munkin <imun@tarantool.org> (cherry picked from commit e3d9d8c9)
-
- Nov 05, 2019
-
-
Vladislav Shpilevoy authored
There was a bug that netbox at any schema update called on_connect() triggers. This was due to overcomplicated logic of handling of changes in the netbox state machine. On_connect() was fired each time the machine entered 'active' state, even if its previous states were 'active' and then 'fetch_schema'. The latter state can be entered many times without reconnects. Another bug was about on_disconnect() - it could be fired even if the connection never entered active state. For example, if its first 'fetch_schema' has failed. Now there is an explicit flag showing the machine connect state. The triggers are fired only when it is changed, on 'active' and on any error states. Intermediate states (fetch_schema, auth) do not matter anymore. Thanks @mtrempoltsev for the initial investigation and a draft fix. Closes #4593 (cherry picked from commit d56d869a)
-
- Nov 01, 2019
-
-
Vladislav Shpilevoy authored
This function is supposed to return NULL on an error. For exceptions there is user_find_by_name_xc. (cherry picked from commit 8b6bdb43)
-
Vladislav Shpilevoy authored
Some guest user privileges were not revoked in the end. (cherry picked from commit 44d4555b)
-
- Oct 30, 2019
-
-
Vladislav Shpilevoy authored
Argparse module stores unspecified parameter values as boolean true. It led to a problem, that a command line '--value' with 'value' defined as a number or a string, showed a strange error message: Expected number/string, got "true" Even though a user didn't pass any value. Now it shows 'nothing' instead of '"true"'. That is clearer. Follow up #4076 (cherry picked from commit c214d086)
-
Vladislav Shpilevoy authored
There was a complaint that tarantoolctl --show-system option is very hard to use. It incorrectly parsed passed values, and provided strange errors. tarantoolctl cat --show-system true Bad input for parameter "show-system". Expected boolean, got "true" tarantoolctl cat --show-system 1 Bad input for parameter "show-system". Expected boolean, got "1" tarantoolctl cat --show-system=true Bad input for parameter "show-system". Expected boolean, got "true" First of all, appeared that the complaining people didn't read documentation in 'tarantoolctl --help'. It explicitly says, that '--show-system' should go after a file name, and does not have a value. Secondly, even having taken the documentation into account, the errors indeed look ridiculous. 'Expected boolean, got "true"' looks especially weird. The problem appeared to be with argparse module, how it parses boolean parameters, and how stores parameter values not specified in a command line. All parameters were parsed into a dictionary: parameter name -> value. If a name is alone (no value), then it is boolean true. Otherwise it was always a string value. An attempt to specify an explicit parameter value 'true' led to storing string 'true' in that dictionary. Consequential check for boolean parameters was trivial: type(value) == 'boolean', which was obviously wrong, and didn't pass for 'true' string, but passed for an empty value. Closes #4076 (cherry picked from commit 03f85d4c)
-
Vladislav Shpilevoy authored
Credentials is a cache of user universal privileges. And that cache can become outdated in case user privs were changed after creation of the cache. The patch makes user update all its credentials caches with new privileges, via a list of all creds. That solves a couple of real life problems: - If a user managed to connect after box.cfg started listening port, but before access was granted, then he needed a reconnect; - Even if access was granted, a user may connect after box.cfg listen, but before access *is recovered* from _priv space. It was not possible to fix without a reconnect. And this problem affected replication. Closes #2763 Part of #4535 Part of #4536 @TarantoolBot document Title: User privileges update affects existing sessions and objects Previously if user privileges were updated (via `box.schema.user.grant/revoke`), it was not reflected in already existing sessions and objects like functions. Now it is. For example: ``` box.cfg{listen = 3313} box.schema.user.create('test_user', {password = '1'}) function test1() return 'success' end c = require('net.box').connect(box.cfg.listen, { user = 'test_user', password = '1' }) -- Error, no access for this connection. c:call('test1') box.schema.user.grant('test_user', 'execute', 'universe') -- Now works, even though access was granted after -- connection. c:call('test1') ``` A similar thing happens now with `box.session.su` and functions created via `box.schema.func.create` with `setuid` flag. In other words, now user privileges update is reflected everywhere immediately. (cherry picked from commit 06dbcec597f14fae6b3a7fa2361f2ac513099662) (cherry picked from commit 2b599c0efa9ae265fb7464af6abae3f6a192e30e)
-
- Oct 28, 2019
-
-
Vladislav Shpilevoy authored
Before the patch there was a race in replication password configuration. It was possible that a replica connects to a master with a custom password before that password is actually set. The replica treated the error as critical and exited. But in fact it is not critical. Replica even can withstand absence of a user and keeps reconnecting. Wrong password situation arises from the same problem of non atomic configuration and is fixed the same - keep reconnect attempts if the password was wrong. Closes #4550 (cherry picked from commit aa2e2c56)
-
Vladislav Shpilevoy authored
Before the patch the nil UUID was ignored and a new random one was generated. This was because internally box treats nil UUID as its absence. Now a user will see an explicit message that nil UUID is a reserved value. Closes #4282 (cherry picked from commit a8ebd334)
-
- Oct 21, 2019
-
-
Kirill Shcherbatov authored
Is_nullable JSON-path indexes used to lose is_nullable property being defined on a space having format. This patch fixes this problem. Closes #4520 (cherry picked from commit 3d01b3af)
-
Kirill Shcherbatov authored
Functional index extractor code used to raise an invalid error message when the user-defined extractor function returns a scalar instead of a table. Closes #4553 (cherry picked from commit 260a3b3a)
-
Ilya Kosarev authored
End recovery (which means building secondary indexes) just after last known log file was read. This allows fast switch to hot standby instance without any delay for secondary index to be built. Due to engine_end_recovery carryover, xdir_collect_inprogress, previously being called from it, is now moved to garbage collector. Closes #4135 (cherry picked from commit 5aa243de)
-
- Oct 17, 2019
-
-
Vladislav Shpilevoy authored
Rows_per_wal option was deprecated because it can be covered by wal_max_size. In order not to complicate WAL code with that option's support this commit drops it completely. In some tests the option was used to create several small xlog files. Now the same is done via wal_max_size. Where it was needed, number of rows per wal is estimated as wal_max_size / 50. Because struct xrow_header size ~= 50 not counting paddings and body. Note, file box/configuration.result was deleted here, because it is a stray result file, and it contained the rows_per_wal option mentioning. Its test was dropped much earlier in fdc3d1dd. Closes #3762 (cherry picked from commit c6012920)
-
Vladislav Shpilevoy authored
In patch c6bea65f I added a bug - replication/misc leaves a bad value in box.cfg.replication. Before that patch the test was resetting this to empty replication. In my patch I forgot about that, and left there the value {box.cfg.listen, "12345"} This patch cleans it up. Follow up #3760 (cherry picked from commit 399899a1)
-
- Oct 12, 2019
-
-
Vladislav Shpilevoy authored
Replication quorum 0 not only affects orphan status, but also, according to documentation, makes box.cfg() return immediately regardless of whether connections to upstreams are established. It was not so before the patch. What is worse, even with non 0 quorum the instance was blocked on reconfiguration for connect timeout seconds, if at least one node is not connected. Now quorum is respected on reconfiguration. On a bootstrap it is still impossible to return earlier than replication_connect_timeout, because nodes need to choose some cluster settings. Too early start would make it impossible - cluster's participants will just start and choose different cluster UUIDs. Closes #3760 (cherry picked from commit c6bea65f)
-
- Oct 09, 2019
-
-
Serge Petrenko authored
A successfully fetched remote instance ballot isn't updated during bootstrap procedure. This leads to a case when different instances choose different masters as their bootstrap leaders. Imagine such a situation. You start instance A without replication set up. Instance A successfully bootstraps. You also have instances B and C both with replication set up to {A, B, C} and replication_connect_quorum set to 3 You first start instance B. It doesn't proceed to choosing a leader until one of the events happens: either the replication_connect_timeout runs out, or instance C is up and starts listening on its port. B has established connection to A and fetched its ballot, with some vclock, say, {1: 1}. B retries connection to C every replication_timeout seconds. Then you start instance C. Instance C succeeds in connecting to A and B right away and bootstraps from instance A. Instance A registers C in its _cluster table. This registration is replicated to instance C. Meanwhile, instance C is trying to sync with quorum instances (which is 3), and stays in orphan mode. Now replication_timeout on instance B finally runs out. It retries a previously unsuccessful connection to C and succeeds. C sends its ballot to B with vclock = {1: 2, 2:0} (in our example), since it has already incremented it after _cluster registration. B sees that C has a greater vclock than A, and chooses to bootstrap from C instead of A. C is orphan and rejects B's attempt to join. B dies. To fix such ungentlemanlike behaviour of C, we should at least include loading status in ballot and prefer fully bootstrapped instances to the ones still syncing with other replicas. We also need to use a separate flag instead of ballot's already existent is_ro, since we still want to prefer loading instances over the ones explicitly configured to be read-only. Closes #4527 (cherry picked from commit dc1e4009)
-
- Oct 01, 2019
-
-
Roman Khabibov authored
(cherry picked from commit 0b9de586)
-
Roman Khabibov authored
Count lines in the json parsing structure. It is needed to print the number of line and column where a mistake was made. Closes #3316 (cherry picked from commit 9f9bd3eb2d064129ff6b1a764140ebef242d7ff7) (cherry picked from commit 53d43160)
-
Vladislav Shpilevoy authored
Code to run main script (passed via command line args, or interactive console) has a footer where it notifies systemd, logs a happened error, and panics. Before the patch that code was unreachable in case of any exception in a main script, because panic happened earlier. Now a happened exception is correctly carried to the footer with proper error processing. A first and obvious solution was replace all panics with diag_set and use fiber_join on the script runner fiber. But appeared, that the fiber running a main script can't be joined. This is because normally it exits via os.exit() which never returns and therefore its caller never dies = can't be joined. The patch solves this problem by passing main fiber diag to the script runner by pointer, eliminating fiber_join necessity. Closes #4382 (cherry picked from commit 157a2d88)
-
- Sep 25, 2019
-
-
Vladislav Shpilevoy authored
Closes #4434 Follow-up #4366 @TarantoolBot document Title: json/msgpack.cfg.encode_deep_as_nil option Tarantool has several so called serializers to convert data between Lua and another format: YAML, JSON, msgpack. YAML is a crazy serializer without depth restrictions. But for JSON, msgpack, and msgpackffi a user could set encode_max_depth option. That option led to crop of a table when it had too many nested levels. Sometimes such behaviour is undesirable. Now an error is raised instead of data corruption: t = nil for i = 1, 100 do t = {t} end msgpack.encode(t) -- Here an exception is thrown. To disable it and return the old behaviour back here is a new option: <serializer>.cfg({encode_deep_as_nil = true}) Option encode_deep_as_nil works for JSON, msgpack, and msgpackffi modules, and is false by default. It means, that now if some existing users have cropping, even intentional, they will get the exception. (cherry picked from commit d7a8942a)
-
Vladislav Shpilevoy authored
Tuple is a C library exposed to Lua. In Lua to translate Lua objects into tuples and back luaL_serializer structure is used. In Tarantool we have several global serializers, one of which is for msgpack. Tuples store data in msgpack, and in theory should have used that global msgpack serializer. But in fact the tuple module had its own private serializer because of tuples encoding specifics such as never encode sparse arrays as maps. This patch makes tuple Lua module use global msgpack serializer always. But how does tuple handle sparse arrays now? In fact, the tuple module still has its own serializer, but it is updated each time when the msgpack serializer is changed. Part of #4434 (cherry picked from commit 676369b1)
-
Vladislav Shpilevoy authored
Msgpack Lua module is not a simple set of functions. It is a global serializer object used by plenty of other Lua and C modules. Msgpack as a serializer can be configured, and in theory its configuration updates should affect all other modules. For example, a user could change encode_max_depth: require('msgpack').cfg({encode_max_depth = <new_value>}) And that would make tuple:update() accept tables with <new_value> depth without a crop. But in fact msgpack configuration didn't affect some places, such as this one. And all the others who use msgpackffi. This patch fixes it, for encode_max_depth option. Other options are still ignored. Part of #4434 (cherry picked from commit 4bb253f7)
-
Vladislav Shpilevoy authored
There are some objects called serializers - msgpack, cjson, yaml, maybe more. They are global objects affecting both Lua and C modules. A serializer have settings which can be updated. But before the patch an update changed only C structure of the serializer. It made impossible to use settings of the serializers from Lua. Now any update of any serializer is reflected both in its C and Lua structures. Part of #4434 (cherry picked from commit fe4a8047)
-
- Sep 17, 2019
-
-
Mergen Imeev authored
Since ARRAY and MAP cannot be converted to SCALAR type, this operation should throw an error. But when the error is raised in SQL, it is displayed in unreadable form. The reason for this is that the given array or map is not correctly converted to a string. This patch fixes the problem by converting ARRAY or MAP to their string representation. For example: box.execute('CREATE TABLE t1(i INT PRIMARY KEY, a SCALAR);') format = {} format[1] = {type = 'integer', name = 'I'} format[2] = {type = 'array', name = 'A'} s = box.schema.space.create('T2', {format=format}) i = s:create_index('ii') s:insert({1, {1,2,3}}) box.execute('INSERT INTO t1 SELECT * FROM t2;') Should return: - error: 'Type mismatch: can not convert [1, 2, 3] to scalar' Follow-up #4189 (cherry picked from commit 736cdd81)
-
Mergen Imeev authored
Function mem_apply_type() implements implicit type conversion. As a rule, tuple to be inserted to the space is exposed to this conversion which is invoked during execution of OP_MakeRecord opcode (which in turn forms tuple). This function was not adjusted to operate on ARRAY, MAP and ANY field types since they are poorly supported in current SQL implementation. Hence, when tuple to be inserted in space having mentioned field types reaches this function, it results in error. Note that we can't set ARRAY or MAP types in SQL, but such situation may appear during UPDATE operation on space created via Lua interface. This problem is solved by extending implicit type conversions with obvious casts: array field can be casted to array, map to map and any to any. Closes #4189 (cherry picked from commit de79b714)
-
- Sep 16, 2019
-
-
Nikita Pettik authored
It was forgotten to swap old and new mask (holding fields involved into foreign key relation) during space alteration (lists of object representing FK metadata are swapped successfully). Since mask is vital and depending on its value different byte-codes implementing SQL query can be produced, this mistake resulted in assertion fault in debug build and wrong constraint check in release build. Let's fix this bug and swap masks as well as foreign key lists. Closes #4495 (cherry picked from commit 33236ecc)
-
- Sep 12, 2019
-
-
Mergen Imeev authored
This patch provides a test suite that allows us to make sure that the SQL BOOLEAN type works as intended. Part of #4228 (cherry picked from commit 7cf84a54)
-
Serge Petrenko authored
In a configuration with several read-only and read-write instances, if replication_connect_quorum is not greater than the amount of read-only instances and replication_connect_timeout happens to be small enough for some read-only instances to form a quorum and exceed the timeout before any of the read-write instaces start, all these read-only instances will choose themselves a read-only bootstrap leader. This 'leader' will successfully bootstrap itself, but will fail to register any of the other instances in _cluster table, since it isn't writeable. As a result, some of the read-only instances will just die unable to bootstrap from a read-only bootstrap leader, and when the read-write instances are finally up, they'll see a single read-only instance which managed to bootstrap itself and now gets a REPLICASET_UUID_MISMATCH error, since no read-write instance will choose it as bootstrap leader, and will rather bootstrap from one of its read-write mates. The described situation is clearly not what user has hoped for, so throw an error, when a read-only instance tries to initiate the bootstrap. The error will give the user a cue that he should increase replication_connect_timeout. Closes #4321 @TarantoolBot document Title: replication: forbid to bootstrap read-only masters. It is no longer possible to bootstrap a read-only instance in an emply data directory as a master. You will see the following error trying to do so: ``` ER_BOOTSTRAP_READONLY: Trying to bootstrap a local read-only instance as master ``` Now if you have a fresh instance, which has `read_only=true` in an initial `box.cfg` call, you need to set up replication from an instance which is either read-write, or has your local instance's uuid in its `_cluster` table. In case you have multiple read-only and read-write instances with replication set up, and you still see the aforementioned error message, this means that none of your read-write instances managed to start listening on their port before read_only instances have exceeded the `replication_connect_timeout`. In this case you should raise `replication_connect_timeout` to a greater value. (cherry picked from commit 037bd58c)
-
- Sep 11, 2019
-
-
Igor Munkin authored
* All test chunks related to luajit were moved from tarantool source tree to the luajit repo * Adjusted CMakeLists via creating a symlink to luajit test directory to fix out-of-source tests Closed #4478 (cherry picked from commit 43575303)
-
- Sep 09, 2019
-
-
Kirill Shcherbatov authored
The json.encode() used to cause a segfault in case of recursive table: tbl = {} tbl[1] = tbl json.encode(tbl) Library doesn't test whether given object on Lua stack parsed earlier, because it performs a lightweight in-depth traverse of Lua stack. However it must stop when encode_max_depth is reached (by design). Tarantool's lua_cjson implementation has a bug introduced during porting original library: it doesn't handle some corner cases: entering into a map correctly increases a current depth, while entering into an array didn't. This patch adopts author's approach to check encode_max_depth limit. Thanks to handling this constraint correctly the segfault no longer occurs. Closes #4366 (cherry picked from commit d2641e66e65ab57317b8e70bb7115517ec81238e)
-
- Sep 02, 2019
-
-
Alexander V. Tikhonov authored
The test sql-tap/gh-3083-ephemeral-unref-tuples.test.lua took too long runtime and failed to finish in timeout limit of 2 minutes when it runs near the end of the running queue: No output during 120 seconds. Will abort after 120 seconds without output. List of workers not reporting the status: - 012_sql-tap [sql-tap/gh-3083-ephemeral-unref-tuples.test.lua, vinyl] at var/012_sql-tap/gh-3083-ephemeral-unref-tuples.result:0 Test hung! Result content mismatch: [File does not exists: sql-tap/gh-3083-ephemeral-unref-tuples.result] [Main process] No output from workers. It seems that we hang. Send SIGKILL to workers; exiting... Due to suggestion from Konstantin Osipov at gh-3845: https://github.com/tarantool/tarantool/issues/3845#issue-385735959 The following tests: gh-3083-ephemeral-unref-tuples.test.lua gh-3332-tuple-format-leak.test.lua where set to run only on memtx configuration. Also the test: gh-3083-ephemeral-unref-tuples.test.lua was removed from the test-run 'fragile' option flaky list. Closed #4128 (cherry picked from commit 40c94af7)
-
- Aug 29, 2019
-
-
Alexander V. Tikhonov authored
Added "fragile" option to the flaky tests that are not intended to be run in parallel with others. Option set at the suite.ini file at the appropriate suites with comments including the issue that stores the fail. (cherry picked from commit 165f8ee6)
-
- Aug 28, 2019
-
-
Kirill Yukhin authored
It turned out that patch d0e38d59 wasn't accompanied w/ a test. Add proper check. (cherry picked from commit c80e9416)
-
Serge Petrenko authored
Currently we only enter orphan mode when instance fails to connect to replication_connect_quorum remote instances during local recovery. On bootstrap and manual replication configuration change an error is thrown. We better enter orphan mode on manual config change, and leave it only in case we managed to sync with replication_connect_quorum instances. Closes #4424 @TarantoolBot document Title: document reaction on error in replication configuration change. Now when issuing `box.cfg{replication={uri1, uri2, ...}}` and failing to sync with replication_connect_quorum remote instances, the server will throw an error, if it is bootstrapping, and just set its state to orphan in all other cases (recovering from existing xlog/snap files or manually changing box.cfg.replication on the fly). To leave orphan mode, you may wait until the server manages to sync with replication_connect_quorum instances. In order to leave orphan mode you need to make the server sync with enough instances. To do so, you may either: 1) set replication_connect_quorum to a lower value 2) reset box.cfg.replication to exclude instances that cannot be reached or synced with 3) just set box.cfg.replication to "" (empty string) (cherry picked from commit 5a0cfe02)
-