- Jun 01, 2020
-
-
Kirill Yukhin authored
Make it possible to set temporary directory where module will be copied before load. @TarantoolBot document Title: Justify module (re-)loading semantics It is now possible to set directory where temporary copies of modules to be loaded will be created. It is done by setting $(TMPDIR) variable. It will be "/tmp" if variable was not set. Follow up #4945
-
Kirill Yukhin authored
Tarantool module reload mechanism allows to reload a module even if there're functions running. It is implemented by refcounting each invocation of module's routines. If reload is called, then refcounter is checked: - If it is 0, then no routines are in flight and module is reloaded by simple pair of dlclose()/dlopen(). - If it is non-zero, then there're routines in flight. To allow to load multiple versions of modules it is loaded not from the DSO specified. Symlink to tempdir is created and dlopen() is invoked against it (w/RTLD_LOCAL flag to avoid conflicts). This trick was implemented in order to fool a dynamic linker: one cannot invoke dlopen() against same file, so let's pretend there're to independent DSOs. The problem is that dynamic linker is smart enough. It tracks not filenames, but i-nodes. Moreover it is smart enough to do stat -L against DSO to follow symlinks! [1][2] So, any attempts to create a symlinks to fool dynamic linker fail and instead of doing actual load it just increments internal refcounter in map w/ corresponding inode, which in turn leads to not-reloading. This wasn't caught by test since old module was always unlinked before new one is copied in place. The patch always copies DSO instead of creating a symlink. Also it fixes the test so in SEGFAULTs without the change. Closes #4945 [1] - https://sourceware.org/git/?p=glibc.git;a=blob;f=elf/dl-load\ .c;h=a5318f9c8d1d42745a254479cf6bb1cd2acd516f;hb=58557c229319a3b8d\ 2eefdb62e7df95089eabe37#l898 [2] - https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/pos\ ix/dl-fileid.h;hb=58557c229319a3b8d2eefdb62e7df95089eabe37#l33
-
- May 28, 2020
-
-
Vladislav Shpilevoy authored
Test modules are shared libraries. They link with Tarantool at runtime and resolve missing symbols then. However in case they already contain some of the same symbols used by tarantool, that leads to a conflict in ASAN build. Nothing serious, there are strict rules how a linker should behave in this case, but there is still a warning. This is how it looked (the text is shortened): ==25624==ERROR: AddressSanitizer: odr-violation (0x000001123b60): [1] size=1024 'mp_type_hint' /builds/nZUxDh2c/0/tarantool/tarantool/src/lib/msgpuck/hints.c:39:20 [2] size=1024 'mp_type_hint' /builds/nZUxDh2c/0/tarantool/tarantool/src/lib/msgpuck/hints.c:39:20 These globals were registered at these points: [1]: #0 0x478b8e in __asan_register_globals (/builds/nZUxDh2c/0/tarantool/tarantool/src/tarantool+0x478b8e) #1 0x7ff7a9bc9d0b in asan.module_ctor (/tmp/tntz2FLhA/function1.so+0x6d0b) [2]: #0 0x478b8e in __asan_register_globals (/builds/nZUxDh2c/0/tarantool/tarantool/src/tarantool+0x478b8e) #1 0xab990b in asan.module_ctor (/builds/nZUxDh2c/0/tarantool/tarantool/src/tarantool+0xab990b) The symbol mp_type_hint is defined both in tarantool executable, and in function1.so dynamic library. This is because both build with msgpuck static library. However the modules don't need to be built with msgpuck. They can link with it at runtime from tarantool executable. This patch removes msgpuck from test dynamic modules, and exports all msgpuck non-inlined symbols explicitly, so they couldn't be removed by the linker in future. Follow-up #2971 Follow-up #5001 Closes #5023
-
Vladislav Shpilevoy authored
The function returns a number of bytes needed to store an fk_constraint_def object with its name and links. However it used sizeof(struct fk_constraint) instead of sizeof(struct fk_constraint_def) to calculate base object size. This worked only because fk_constraint is bigger than fk_constraint_def.
-
Cyrill Gorcunov authored
Part-of #689 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
The implementation has been removed in commit ef2c171d Part-of #689 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
- May 27, 2020
-
-
Vladislav Shpilevoy authored
Vclock depends on bit library, but wasn't linked to it.
-
Nikita Pettik authored
Before this patch box.snapshot() bails out immediately if it sees that the scheduler is throttled due to errors. For instance: box.error.injection.set('ERRINJ_VY_RUN_WRITE', true) snapshot() -- fails due to ERRINJ_VY_RUN_WRITE box.error.injection.set('ERRINJ_VY_RUN_WRITE', false) snapshot() -- still fails despite the fact that injected error is unset As a result, one has to wait up to a minute to make a snapshot. The reason why throttling was introduced was to avoid flooding the log in case of repeating disk errors. What is more, to deal with schedule throttling in tests, we had to introduce a new error injection (ERRINJ_VY_SCHED_TIMEOUT). It reduces time duration during which the scheduler remains throttled, which is ugly and race prone. So, let's unthrottle scheduler when checkpoint process is launched via manual box.snapshot() invocation. Closes #3519
-
Nikita Pettik authored
In some cases it may turn out to be useful to know whether checkpoint process was launched manually (explicitly calling box.snapshot()) or automatically via checkpoint daemon. In particular, to unthrottle vinyl scheduler when it comes for manual checkpoints. So let's extend engine's vtab method begin_checkpoint() with corresponding argument. Needed for #3519
-
Nikita Pettik authored
Assert in vy_tx_write() validates that upsert applied on the top of cached statement is always replace. In fact, it is not always so. If vy_apply_upsert() fails due to the fact that PK is modified, it returns old statement (i.e. statement which upsert is applied on). In this regard, if tuple cache contains insert and invalid upsert is executed, vy_apply_upsert() will return insert. As a result, assert will fire. Let's fix it and take into account that vy_apply_upsert() is able to return inserts as well. Closes #5005
-
- May 25, 2020
-
-
Vladislav Shpilevoy authored
vinyl_engine_create_space() didn't set an error in the diagnostics area when region_alloc() failed. This could lead to a crash, although was almost impossible to happen.
-
Vladislav Shpilevoy authored
There were lots of errors of kind: /builds/M4RrgQZ3/0/tarantool/tarantool/src/exports.h:395:1: error: variable ‘uuid_nil’ redeclared as function EXPORT(uuid_nil) ^ /builds/M4RrgQZ3/0/tarantool/tarantool/src/lib/uuid/tt_uuid.c:39:22: note: previously declared here const struct tt_uuid uuid_nil; when LTO was enabled. That happened because exports.c file, to take symbol addresses, declared lots of functions and variables from all the code base as extern void <symbol>(void); This is crazy, but it worked for normal builds. Because symbol is symbol. The compilers couldn't find conflicting declarations, because they never met in one compilation unit. However the lie was revealed by linker with LTO enabled. It could see, that actual symbol definitions didn't match their exports in exports.c. It could live with mismatching function-function or variable-variable cases, but couldn't withstand function-variable mismatches. When a symbol was declared as a variable in one place and as a function in another. This was the case for variables: - uuid_nil - tarantool_lua_ibuf - log_pid - log_format - crc32_calc - _say - log_level The errors were false positive, because the symbols were never used for anything except taking their addresses. To calm the linker down exports.c now does not participate in LTO. Closes #5001
-
- May 21, 2020
-
-
Vladislav Shpilevoy authored
Functions tt_uuid_is_nil() and tt_uuid_is_equal() used struct tt_uuid as an array of two uint64. But it is wrong, because struct tt_uuid has 4 byte alignment, while uint64 requires 8. That led to unaligned memory access in these 2 functions. The patch makes these functions use memcmp() - it appers to be faster (disassembly shows much smaller code and without jumps), and does not depend on alignment. Part of #4609 Reviewed-by:
Aleksandr Lyapunov <alyapunov@tarantool.org> Reviewed-by:
Timur Safin <tsafin@tarantool.org>
-
Vladislav Shpilevoy authored
Lib/bit library in some functions dereferenced a byte array as a pointer at uint64 or uint32. It led to unaligned memory access, when the array didn't have the needed alignment. That made clang 'alignment' sanitizer crazy. Also there were places, where bitwise shift was done for the type size. For example, a uint32 number was allowed to make 32 bit shift in both sides. This is undefined behaviour. The patch fixes these UBs. Bitwise shift of too long size is banned. Unaligned access is fixed in some places by changing uint64 to uint8, when it is possible without perf loss. And in the others by using explicitly specified unaligned load. So it works the same, but the sanitizer is aware of that. Part of #4609 Reviewed-by:
Aleksandr Lyapunov <alyapunov@tarantool.org> Reviewed-by:
Timur Safin <tsafin@tarantool.org>
-
Vladislav Shpilevoy authored
Msgpuck functions mp_snprint() and mp_fprint() now support customizable MP_EXT serializer. This patch makes them able to correctly serialize all tarantool's MP_EXT extensions: MP_DECIMAL, MP_UUID, MP_ERROR. Closes #4719 Reviewed-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Serge Petrenko <sergepetrenko@tarantool.org> Reviewed-by:
Nikita Pettik <korablev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Vladislav Shpilevoy authored
Msgpuck functions mp_snprint() and mp_fprint() now support customizable MP_EXT serializer. This patch adds such for MP_ERROR. All extension serializers will be activated in a separate commit. Part of #4719 Reviewed-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Serge Petrenko <sergepetrenko@tarantool.org> Reviewed-by:
Nikita Pettik <korablev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Vladislav Shpilevoy authored
Msgpuck functions mp_snprint() and mp_fprint() now support customizable MP_EXT serializer. This patch adds such for MP_UUID. It is not activated yet, but will be later, when last extension - MP_ERROR - also gets its serializer. Part of #4719 Reviewed-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Serge Petrenko <sergepetrenko@tarantool.org> Reviewed-by:
Nikita Pettik <korablev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Vladislav Shpilevoy authored
Msgpuck functions mp_snprint() and mp_fprint() now support customizable MP_EXT serializer. This patch adds such for MP_DECIMAL. It is not activated yet, but will be later, when other extensions also get their serializers. Part of #4719 Reviewed-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Serge Petrenko <sergepetrenko@tarantool.org> Reviewed-by:
Nikita Pettik <korablev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
Vladislav Shpilevoy authored
In the new version an API is added to be able to customize MP_EXT serializer for mp_snprint() and mp_fprint() functions. Part of #4719 Reviewed-by:
Cyrill Gorcunov <gorcunov@gmail.com> Reviewed-by:
Serge Petrenko <sergepetrenko@tarantool.org> Reviewed-by:
Nikita Pettik <korablev@tarantool.org> Reviewed-by:
Alexander Turenko <alexander.turenko@tarantool.org>
-
- May 20, 2020
-
-
Nikita Pettik authored
vy_read_iterator_cmp_stmt() uses tuple_compare_with_key() under the hood which follows next convention: (a) it returns 0 if key_fields of tuple equal to given key; (b) < 0 if key_fields less than given key; (c) > 0 otherwise. However, comment to vy_read_iterator_cmp_stmt() says that: (a) it returns -1 if first statement precedes second one; (b) 0 if they at the same position; (c) +1 if first statement supersedes second one. so, since there's no "normalization" of return code from tuple_compare_with_key() it would be correct to say that vy_read_iterator_cmp_stmt() returns: (a) it returns arbitrary integer value < 0 if first statement precedes second one; (b) 0 if they at the same position; (c) arbitrary integer value > 0 if first statement supersedes second one.
-
Kirill Yukhin authored
Add MAP_STACK to mmap() flags.
-
Sergey Bronnikov authored
- define macros LOG_MAKEPRI() on OpenBSD as it is absent - replace sigtimedwait() by sigwait() as latter is unsupported on OpenBSD Part of #4967
-
Sergey Bronnikov authored
Part of #4967
-
Sergey Bronnikov authored
Part of #4967
-
Sergey Bronnikov authored
Part of #4967
-
Sergey Bronnikov authored
OpenBSD includes DL library in a base system Part of #4967
-
Alexander Turenko authored
killpg() on Mac OS may don't deliver a signal to a process: it seems that there is a race when a process is just forked. It means that popen_handle:close() may leave a process alive, when `opts.setsid` and `opts.group_signal` are set. There is simple reproducer, which does not leave alive `sleep 120` processes on Linux, but does it on Mac OS (within three-four runs in rows): | #include <signal.h> | #include <unistd.h> | #include <fcntl.h> | | int | main() | { | char *child_argv[] = { | "/bin/sh", | "-c", | "sleep 120", | NULL, | }; | pid_t pid; | int fd[2]; | pipe(fd); | fcntl(fd[0], F_SETFD, FD_CLOEXEC); | fcntl(fd[1], F_SETFD, FD_CLOEXEC); | | if ((pid = fork()) == 0) { | /* Child. */ | close(fd[0]); | setpgrp(); | for (int i = 0; i < 10; ++i) { | /* Proceed with killpg. */ | if (i == 5) | close(fd[1]); | if (fork() == 0) { | /* Child. */ | execve("/bin/sh", child_argv, NULL); | } | } | } else { | /* Parent. */ | close(fd[1]); | char c; | read(fd[0], &c, 1); | killpg(pid, SIGKILL); | } | return 0; | } Compile it (`cc test.c -o test`) and run several times: $ for i in $(seq 1 1000); do \ echo $i; \ ./test \ && ps -o pid,pgid,command -ax | grep [s]leep \ && break; \ done This is the reason why `sleep 120` process may be alive even when the whole test passes. test-run captures stdout and stderr of a 'core = app' test and waits EOF on them. If a child process inherit one of them or both, the fd is still open for writing and so EOF situation will not appear until `sleep 120` will exit. This commit doesn't try to overcome the root of the problem, but close stdout and stderr for the child process that may not be killed / exited in time. Aside of this, updated found Mac OS peculiars in API comments of C and Lua popen modules. Fixes #4938 @TarantoolBot document Title: popen: add note re group signaling on Mac OS Copyed from the popen_handle:signal() updated description: > Note: Mac OS may don't deliver a signal to a process in a group when > opts.setsid and opts.group_signal are set. It seems there is a race > here: when a process is just forked it may be not signaled. Copyed from the popen_handle:close() updated description: > Details about signaling: > > <...> > - There are peculiars in group signaling on Mac OS, > @see popen_handle:signal() for details. Follows up https://github.com/tarantool/doc/issues/1248
-
Alexander Turenko authored
popen_delete() always frees a handle memory even when it reports a failure to send SIGKILL, see [1]. We should reflect this contract in popen_handle:close() and mark the handle as closed despite popen_delete() return value. There are cases, when killpg() fails with EPERM on Mac OS, so popen_delete() reports a failure. See [1] for more information. [1]: 01657bfb ('popen: always free resources in popen_delete()') Fixes #4995
-
- May 19, 2020
-
-
Vladislav Shpilevoy authored
Closes #4689 @TarantoolBot document Title: box.session.push() 'sync' is deprecated box.session.push() had two parameters - data to push and 'sync'. The sync was a request ID with which the out of bound data should be pushed into a socket. This was introduced as a workaround for #3450, and is useless since its resolution. A user anyway can't push to different sessions, where that parameter could be useful. And pushing into requests of the same session, on the contrary, is something not really needed anywhere, not portable to non-binary session types (console, background), and is just dangerous since it is easy to add a bug here. The patch removes the parameter. Now there will be thrown a 'Usage' error at attempt to use 'sync' parameter. In version 2.4 it is deprecated, prints warnings into logs, but still works. In 2.5 it is removed completely.
-
Kirill Yukhin authored
Revert 03790ac5 commit in small library part.
-
- May 18, 2020
-
-
Vladislav Shpilevoy authored
dynamic-list (exported_symbols_list on Mac) was used to forbid export of all symbols of the tarantool executable except a given list. Motivation of that was to avoid hacking the linker with false usage of symbols needed to be exported. As a consequence, symbols not listed in these options became invisible. Before these options, when a symbol was defined, but not used in the final executable, the linker could throw it away, even though many symbols were used by Lua FFI, or should be visible for user's dynamic modules. Where the linker, obviously, can't see if they are needed. To make the linker believe the symbols are actually needed there was a hack with getting pointers at these functions and doing something with them. For example, assume we have 'test()' function in 'box' static library: int test(void); It is not used anywhere in the final executable. So to trick the linker there is a function 'export_syms()' declared, which takes a pointer at 'test()' and seemingly does something with it (or actually does - it does not matter): void export_syms() { void *syms[] = {test}; if (time(NULL) == 0) { syms[0](); syms[1](); ... } } Some users want to use not documented but visible symbols, so the patch removes the dynamic-list option, and returns the linker hack back. But with 0 dependencies in the export file. Closes #2971
-
Vladislav Shpilevoy authored
src/cpu_feature.c, src/box/error.cc and src/box/vclock.c were used twice. As a result it could lead to duplicate symbols.
-
Cyrill Gorcunov authored
I do not drop the serpent module for a while since I need to make sure that everything work as expected as we've not break backward compatibility significantly (though we didn't claim the lua mode output is stable enough). Fixes #4682 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
When we print output in console (especially in Lua mode) it is impossible to find out the internal type an entry represents, which in turn leads to inability to encode "ULL" entries properly. Moreover we always introduce new types (for example decimals, uuids) and the serpent module we currently use for encodings has no clue about them. Thus lets implement own lua serializer similarly to the yaml encoder. This allows us to properly detect every field and encode it accordingly. Part-of #4682 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
As we gonna implement lua output serializer lets rename luaL_yaml_default to serializer_yaml which will be more general name, for other serializers we will use same serializer_ prefix. Part-of #4682 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
This will allow us to implement own formatter for Lua output mode, so to be precise which exactly formatter is caller lets rename general "format" to "format_yaml". Part-of #4682 Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
For some reason we're using spaces here to adjust code. Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
For better maintainance. Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
Cyrill Gorcunov authored
The member is defined as unsigned 32 bit value, so we have to use an exactly the same type here. Signed-off-by:
Cyrill Gorcunov <gorcunov@gmail.com>
-
- May 15, 2020
-
-
Serge Petrenko authored
Since the anonymous replica implementation, it was forbidden to replicate (join/subscribe/register) from anonymous instances. Actually, only joining and register should be banned, since an anonymous replica isn't able to register its peer in _cluster anyway. Let's allow other anonymous replicas, but not the normal ones, to subscribe to an anonymous replica. Also remove unnecessary ER_UNSUPPORTED errors from box_process_join() and box_process_register() for anonymous replicas. These cases are covered with ER_READONLY checks later on, since anonymous replicas must be read-only. Note, this patch doesn't allow normal instances to subscribe to anonymous ones. Even though it is technically possible, it may bring more problems than profits. Let's allow it later on if there's an explicit demand. Closes #4696
-