Maksim Kaitmazian authored 1 year ago and Дмитрий Кольцов committed 1 year ago

User name is usually used as a salt for user password in order to
avoid password repeating.
For instance, postgres md5 authentication stores passwords as
md5("password", "user"), so that the same passwords are represented by
different hashes.

part of picodata/picodata/sbroad!377

@TarantoolBot document
Title: Document updated `box.schema.user.password` declaration.

Since auth methods can use user name for hashing, user name is
added to argument list of `box.schema.user.password`.

NO_TEST=there are no methods that use user name

Denis Smirnov authored 1 year ago and Дмитрий Кольцов committed 1 year ago

Tuple hash calculation tests for the C API were incorrect. Thanks
to the full pipeline with DEBUG build we detected the problem and
fixed it.

NO_DOC=picodata internal patch
NO_CHANGELOG=picodata internal patch

Denis Smirnov authored 1 year ago and Дмитрий Кольцов committed 1 year ago

Picodata supports cluster-wide SQL and needs some predictable
method to calculate tuple hashes for the bucket ids. Method
should be available for Lua, C and Rust users. It was decided
to expose a murmur3 hash calculation method of the key_def module.

NO_DOC=picodata internal patch
NO_CHANGELOG=picodata internal patch

godzie44 authored 1 year ago and Дмитрий Кольцов committed 1 year ago

Introduced a new type of cbus pipe - lcpipe. The current pipe in the
cbus - cpipe, has a number of limitations, first of all - the cpipe
cannot be used from the 3rd party threads, cpipe only works as a channel
between two cords. That why lcpipe is needed. Its main responsibility -
create channel between any thread and tarantool cord.

Internally lcpipe is a cpipe, but:
- on flush triggers removed, cause triggers use thread-local mem-pool,
this is not possible on a third party thread
- producer event loop removed, cause there is no libev event loop in
third party thread

Also, lcpipe interface is exported to the outside world.

NO_DOC=core feature

NO_DOC=disable feedback
NO_TEST=disable feedback

Due to inconsistency of Tarantool type casting while using strict
data types as "double" or "unsigned" it is needed
to use "number" data type in a whole bunch of cases.
However "number" may contain "decimal" that will be serialized into
string by JSON builtin module.

This commit adds "encode_decimal_as_number" parameter to json.cfg{}.
That forces to encode `decimal` as JSON number to force type
consistency in JSON output.
Use with catious - most of JSON parsers assume that number is restricted
to float64.

NO_DOC=we do not host doc

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 1 year ago

Previously,

select "t1"."a" from (select "a" from "t") as "t1";

returned a result column name `t1` instead of `t1.a` because of
incorrect work of a dequoting function. The reason was that
previously sqlDequote() function finished its work when found the
first closing quote.

Old logic worked for simple selects where the column name doesn't
contain an explicit scan name ("a" -> a).
But for the sub-queries results sqlDequote() finished its work right
after the scan name ("t1"."a" -> t1). Now the function continues its
deqouting till it gets the null terminator at the end of the string.

Closes #7063

NO_DOC=don't change any public API, only a bug fix

Co-authored-by: Mergen Imeev <imeevma@gmail.com>

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 1 year ago

Actually there is no reason to throw an error and make a user
manually recreate prepared statement when it expires. A much more
user friendly way is to recreate it under hood when statement's
schema version differs from the box one.

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 1 year ago

Problem description.

When we prepare a statement with parameters in the result columns
(for example box.prepare('select ?')) Tarantool has no information
about the type of the output column and set it to default boolean.
Then, on the execution phase, the type would be recalculated during
the parameter binding.

Tarantool expects that there is no way for parameter to appear in the
result tuple other than exactly be mentioned in the final projection.
But it is incorrect - we can easily propagate parameter from the inner
part of the join. For example

box.prepare([[select COLUMN_1 from t1 join (values (?)) as t2 on true]])

In this case column COLUMN_1 in the final projection is not a
parameter, but a "reference" to it and its type depends on the
parameter from the inner part of the join. But as Tarantool
recalculates only binded parameters in the result projection,
it doesn't change the default boolean metadata type of the COLUMN_1
and the query fails on comparison with the actual type of the tuple.

Solution.
As we don't want to patch Vdbe to make COLUMN_1 refer inner parameter,
it was decided to make a simple workaround: change the default
column type from BOOLEAN to ANY for parameters. It fixes the
comparison with the actual tuple type (we do not fail), but in some
cases get ANY column in the results where we would like to have
explicitly defined type. Also NULL parameters would also have ANY
type, though Tarantool prefers to have BOOLEAN in this case.

Closes https://github.com/tarantool/tarantool/issues/7283

NO_DOC=bug fix

We need to call `tx_accept_msg` in `tx_process_override` before we pass
message to the override handler. Unfortunately if handler response with
IPROTO_HANDLER_FALLBACK we call the builtin handler for message that
calls `tx_accept_msg` again which is not expected. Some actions of
this function are idempotent and some are not.

Let's make the function NOP if it called once again.

Closes #9345

NO_DOC=bugfix

(cherry picked from commit 21112b06)

Without checking the return value of lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes #9396

(cherry picked from commit 98474f70)

Current split-brain detector implementation raises an error each time a
CONFIRM or ROLLBACK entry is received from the previous synchronous
transaction queue owner. It is assumed that the new queue owner must
have witnessed all the previous CONFIRMS. Besides, according to Raft,
ROLLBACK should never happen.

Actually there is a case when a CONFIRM from an old term is legal: it's
possible that during leader transition old leader writes a CONFIRM for
the same transaction that is confirmed by the new leader's PROMOTE. If
PROMOTE and CONFIRM lsns match there is nothing bad about such
situation.

Symmetrically, when an old leader issues a ROLLBACK with the lsn right
after the new leader's PROMOTE lsn, it is not a split-brain.

Allow such cases by tracking the last confirmed lsn for each synchronous
transaction queue owner and silently nopifying CONFIRMs with an lsn less
than the one recorded and ROLLBACKs with lsn greater than that.

Closes #9138

NO_DOC=bugfix

(cherry picked from commit ffa6ac15)

Previously the replicas only persisted the confirmed lsn of the current
synchronous transaction queue owner. As soon as the onwer changed, the
info about which lsn was confirmed by the previous owner was lost.

Actually, this info is needed to correctly filter synchro requests
coming from the old term, so start tracking confirmed vclock instead of
the confirmed lsn on replicas.

In-scope of #9138

NO_TEST=covered by the next commit
NO_CHANGELOG=internal change

@TarantoolBot document
Title: Document new IPROTO_RAFT_PROMOTE request field

IPROTO_RAFT_PROMOTE and IPROTO_RAFT_DEMOTE requests receive a new key
value pair:

IPROTO_VCLOCK : MP_MAP

The vclock holds a confirmed vclock of the node sending the request.

(cherry picked from commit c4415d44)

There was an error in xrow_decode_synchro: it compared the expected type
of the value to the type of the key (MP_UINT) instead of the type of the
actual value. This went unnoticed because all values in synchro requests
were integers.

This is going to change soon, when PROMOTE requests will start holding a
vclock, so fix the wrong type check.

In-scope-of #9138

NO_DOC=bugfix
NO_CHANGELOG=not user-visible

(cherry picked from commit c18410f5)

The fix is simple: look up the function in `box.func` by name and, if
found, execute its `call` method. The only tricky part is to avoid the
lookup before `box.cfg` is called because `box.func` is unavailable at
the time. We achieve that by checking `box.ctl.is_recovery_finished`.

Closes #9131

NO_DOC=bug fix

(cherry picked from commit e92a8e7b)

On Tarantool shutdown we destroy all the fibers in some sequence. We
don't require that all the fibers are finished before shutdown. So it
may turn out that we first destroy some alive fiber and then destroy
another alive fiber which joins the first one. Currently we have
use-after-free issue in this case because clearing `link` field of
the second fiber changes `wake` field of the first fiber.

Close #9406

NO_DOC=bugfix

(cherry picked from commit 2f7ec948)

Graceful shutdown is done in a special fiber which is started for
example on SIGTERM. So it can run concurrently with fiber executing
Tarantool init script. On init fiber exit we break event loop to pass
control back to the Tarantool initialization code. But we fail to run
event loop a bit more to finish graceful shutdown.

The test is a bit contrived. A more real world case is when Tarantool is
termintated during lingering box.cfg().

Close #9411

NO_DOC=bugfix

(cherry picked from commit 786eb2ac)

According to ANSI, EXISTS is a predicate that tests a given subquery and
returns true if it returns more than 0 rows, false otherwise. However,
after 2a720d11, EXISTS worked correctly only if there were exactly 0
or 1 rows, and in all other cases it gave an error. This patch makes
EXITS work properly.

Closes #8676

NO_DOC=bugfix

(cherry picked from commit a5e498d1)

NO_DOC=tests
NO_TEST=tests
NO_CHANGELOG=tests

(cherry picked from commit 941a85c3)

The tests are TAP compatible and applicable to all Tarantool builds so
there's no need to run them with ctest. We just need to add a couple
skip conditions:
 - The luarocks test shouldn't be run on dynamic builds because luarocks
   modules aren't embedded there.
 - The traceback test should be run only if ENABLE_BACKTRACE was set at
   build time.

Part of #9242

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 1eb98ef3)

Prior to this patch, there were many possible code execution options
that were not covered by tests. After this commit, any assert(false)
inside box_wait_limbo_acked cause a crash.

Closes #7318

NO_DOC=test
NO_CHANGELOG=test

(cherry picked from commit 7fce5bec)

Old: "Failed to allocate 2147483648 bytes in hash_table for key"
New: "Failed to allocate 16384 bytes in hash_table for key"

ERRINJ_INDEX_ALLOC cannot be used to test this error, because it fails
earlier, so ERRINJ_HASH_INDEX_REPLACE is introduced.

Follow-up #3594

NO_DOC=minor
NO_CHANGELOG=minor

(cherry picked from commit 0a8043d1)

This is part of master commit bd4c6675 ("memtx: use MemtxAllocator
stats for box.info.memory"). The test is failed without the patch in
ASAN build.

Follow-up #7327

NO_CHANGELOG=test fix
NO_DOC=test fix

The test is quite a flacky in debug ASAN CI workflow. The issue is test
check upper boundary of it's execution time. I run many instances of
this test on in parallel and got average time of 40s for memtex and 70s
for vinyl.

The time quota is already changed by the commit 84cb1e04 ("sql:
increase time quota for selectG test on vinyl") for laptops with HDD.
I did not check execution time for HDD though. I guess the bottleneck
for debug ASAN is CPU.

Follow-up #7327

NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 703d11fe)

This patch also includes:
  - misc trivial fixes for ASAN discovered issues
  - minor adaptations for ASAN friendly allocators

Closes #7327

NO_DOC=internal
NO_CHANGELOG=internal

(cherry picked from commit 446201b8)

With new ASAN-friendly small implementation unit/fiber_stack.c test
start to fail. The issue is leak sanitizer reports a leak. This is an
expected leak of test for mprotect failure on fiber stack destruction.
Let's tell sanitizer to ignore this case.

By the way let's drop test code for temporary redirecting stderr. It is
outdated as test is TAP-compatible. It was a PITA as due to this
redirection there was no leak report only error exit code.

Part of #7327

NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 41844ffb)

When SMALL_MALLOC_IMPL is defined and ASAN-friendly allocators are used
the arena allocator is not used at all as we not allocate memory
directly from there. And other ASAN-friendly allocators are not allocate
from it too. Thus box.slab.info().arena_size == 0. Same for usage
of runtime arena box.runtime.info().used.

Also usage with ASAN-friendly lsregion is a bit different as it does
not account for size of alignment padding. Thus we need to adapt
box.stat.vinyl().memory.level0 tests. Approach is to check for lower
and upper limit instead of checking for exact values.

Part of #7327

NO_DOC=test changes
NO_CHANGELOG=test changes

(cherry picked from commit 1436eb41)

ASAN-friendly implementation poisons memory after allocation with
ibuf_alloc so we need to fix existing places in code where we access
memory after allocation.

Part of ibuf implementation is inline functions in headers. Thus ibuf
implementation in Lua reimplement this parts. We add poison to these
inline functions in ASAN-friedly implementation so we need add same poison
in Lua implementation.

Part of #7327

NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 4f542bb7)

We are going to include generated small_config.h into small allocator
headers (currently it is only included in small source files).
core/memory.h depends on small headers and salad/heap.h depends on
core/memory.h. As a result we need to provide a way for salad/heap.h
users to find small_config.h header.

Instead let's drop dependency from core/memory.h as we only use it for
typeof definition.

Part of #7327

NO_CHANGELOG=code cleanup
NO_DOC=code cleanup

(cherry picked from commit d01609a4)

If leak sanitizer reaches the memory protected from read with mprotect
it exhibits all sorts of odd behaviour. It can hang, can crash, can
return errors with no leak backtraces.

We use mprotect to create guard zones at the end of fiber stack so if
stack is overflowed we get a signal and crash. We take protection off
when fiber is destroyed. Unfortunately we do not destroy cords (and its
fibers) which cancelled through cord_cancel_and_join. This is going to
be addressed in patch for issue #8423 ("Get rid of pthread_cancel()").
Until that moment let's disable protection for ASAN builds.

Note that we did not hit this behaviour before because LSAN only scans
memory allocated using malloc and regular slab cache uses mmap to get
memory.

Part of #7327

NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 2ee15793)

ASAN small object allocator implementation has a bit different pattern
on quota leasing on allocating memory. So we may need to allocate more
objects to hit the quota etc.

Part of #7327

NO_CHANGELOG=test tuning
NO_DOC=test tuning

(cherry picked from commit d456a986)

They are rather noisy. Also delete debug log on arena creation. These
two make sense only with each other.

Part of #7327

NO_TEST=internal
NO_DOC=internal
NO_CHANGELOG=internal

(cherry picked from commit 0dc37356)

This patch replaces region_*() functions with xregion_*() functions.

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 1ba84fe3)

Proposed ASAN implementation of region allocator does not support double
reservation for the sake of simplicity. Every reservation is supposed to
be followed by one or more allocations.

This restriction does not work well with mpstream currently. The issue is
mpstream_init/mpstream_reserve do reservation of size 0. For example In
case of region slab of min order is reserved (a chunk of memory of page
size currently). If the first data we want to write to mpstream is
larger then the reservation done then we make reservation again.

Let's get rid of this reservation at the beginning as it is suboptimal
behaviour. Moreover let's get rid of mpstream_reset as mpstream_init
is lightweight and we can create a new mpstream instead of reusing
exiting.

Also while we at it avoid allocation of 0 size in mpstream_flush as it
is done in mpstream_reserve_slow (see 3.0.0-alpha3-19-g8159347d0 "misc:
avoid allocations of size 0 for region" for details).

NO_TEST=internal
NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 3b1de78d)

We already use this info in one of the test and going to use it more.

Part of #7327

@TarantoolBot document
Title: new tarantool.build.asan flag

It is `true` if `ENABLE_ASAN` build option is set and `false` otherwise.

(cherry picked from commit 23012356)

The check_param and check_param_table Lua helpers are defined in
box/lua/schema.lua but used across the whole code base. The problem is
we can't use them in files that are loaded before box/lua/schema.lua,
like box/lua/session.lua. Let's move them to a separate source file
lua/utils.lua to overcome this limitation. Also, let's add some tests.

NO_DOC=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit d8d267c5)

We hit #3807 in release/2.11 for release ASAN build with ASAN-friendly
small allocators.

Follow-up #7327

NO_CHANGELOG=internal
NO_DOC=internal

(cherry picked from commit 3fbd7fcb)

Configuring log modules work differently with log.cfg and box.cfg:
box.cfg{log_modules=...} overwrites the current config completely while
log.cfg{modules=...} overwrites the currently config only for the
specified modules. Let's fix this inconsistency by making log.cfg behave
exactly as box.cfg.

Closes #7962

NO_DOC=bug fix

(cherry picked from commit c13e59a5)

We install a signal handler that prints the stack trace on SIGSEGV,
SIGBUS, SIGILL, SIGFPE. The signal handler uses the current stack.
This works fine for most issues, but not for stack overflow, because
the latter makes the current stack unusable, leading to a crash in
the signal handler. Let's install an alternative signal stack in each
thread so that we can print the stack trace on stack overflow.

Note that we skip this for ASAN because it installs its own signal
stack. (Installing a custom stack would result in a crash.)

Closes #9222

NO_DOC=bug fix

(cherry picked from commit cb8e903b)

The motivation is to reduce time slip on Tarantool startup before
running init scripts. Internal ev time is set in fiber_init/ev_default_loop
and is not get updated until starting event loop. This causes
timeouts slip up to 0.3 in debug ASAN build in init script (see #9261).

Let's run event loop right at the beginning of the run_script_f before
executing any script. This way besides updating internal ev time we make
an explicit place of starting script event loop. Currently it is started
lazily when config script yields.

This will fix CI for PR https://github.com/tarantool/tarantool-ee/pull/572
for debug ASAN workflow.

We can also remove start_loop condition. It does not make sense now. It
was added in the commit 3a851430 ("Fix tarantool -e "os.exit()"
hang") but since then we start to stop event loop after handling
os.exit().

Also this fixes #9266. The issue is we don't have an event loop to run
on shutdown triggers if -e command line expression add such a trigger
and then call os.exit().

Follow-up #7327
Closes #9266

NO_DOC=bugfix

(cherry picked from commit 1fcfb8c2)