Skip to content
Snippets Groups Projects
  1. Jun 13, 2024
    • Vladislav Shpilevoy's avatar
      election: fix box.ctl.demote() nop in off-mode · 42631d5b
      Vladislav Shpilevoy authored
      box.ctl.demote() used not to do anything with election_mode='off'
      if the synchro queue didn't belong to the caller in the same term
      as the election state.
      
      The reason could be that if the synchro queue term is "outdated",
      there is no guarantee that some other instance doesn't own it in
      the latest term right now.
      
      The "problem" is that this could be workarounded easily by just
      calling promote + demote together.
      
      There isn't much sense in fixing it for the off-mode because the
      only reasons off-mode exists are 1) for people who don't use
      synchro at all, 2) who did use it and want to stop. Hence they
      need demote just to disown the queue.
      
      The patch "legalizes" the mentioned workaround by allowing to
      perform demote in off-mode even if the synchro queue term is old.
      
      Closes #6860
      
      NO_DOC=bugfix
      
      (cherry picked from commit 1afe2274)
      42631d5b
    • Vladimir Davydov's avatar
      tuple: don't use offset_slot_cache in vinyl threads · 7d90a94c
      Vladimir Davydov authored
      `key_part::offset_slot_cache` and `key_part::format_epoch` are used for
      speeding up tuple field lookup in `tuple_field_raw_by_part()`. These
      structure members are accessed and updated without any locks, assuming
      this code is executed exclusively in the tx thread. However, this isn't
      necessarily true because we also perform tuple field lookups in vinyl
      read threads. Apparently, this can result in unexpected races and bugs,
      for example:
      
      ```
        #1  0x590be9f7eb6d in crash_collect+256
        #2  0x590be9f7f5a9 in crash_signal_cb+100
        #3  0x72b111642520 in __sigaction+80
        #4  0x590bea385e3c in load_u32+35
        #5  0x590bea231eba in field_map_get_offset+46
        #6  0x590bea23242a in tuple_field_raw_by_path+417
        #7  0x590bea23282b in tuple_field_raw_by_part+203
        #8  0x590bea23288c in tuple_field_by_part+91
        #9  0x590bea24cd2d in unsigned long tuple_hint<(field_type)5, false, false>(tuple*, key_def*)+103
        #10 0x590be9d4fba3 in tuple_hint+40
        #11 0x590be9d50acf in vy_stmt_hint+178
        #12 0x590be9d53531 in vy_page_stmt+168
        #13 0x590be9d535ea in vy_page_find_key+142
        #14 0x590be9d545e6 in vy_page_read_cb+210
        #15 0x590be9f94ef0 in cbus_call_perform+44
        #16 0x590be9f94eae in cmsg_deliver+52
        #17 0x590be9f9583e in cbus_process+100
        #18 0x590be9f958a5 in cbus_loop+28
        #19 0x590be9d512da in vy_run_reader_f+381
        #20 0x590be9cb4147 in fiber_cxx_invoke(int (*)(__va_list_tag*), __va_list_tag*)+34
        #21 0x590be9f8b697 in fiber_loop+219
        #22 0x590bea374bb6 in coro_init+120
      ```
      
      Fix this by skipping this optimization for threads other than tx.
      
      No test is added because reproducing this race is tricky. Ideally, bugs
      like this one should be caught by fuzzing tests or thread sanitizers.
      
      Closes #10123
      
      NO_DOC=bug fix
      NO_TEST=tested manually with fuzzer
      
      (cherry picked from commit 19d1f1cc)
      7d90a94c
    • Vladimir Davydov's avatar
      vinyl: fix cache iterator skipping tuples in read view · 1bad1afc
      Vladimir Davydov authored
      The tuple cache doesn't store older tuple versions so if a reader is
      in a read view, it must skip tuples that are newer than the read view,
      see `vy_cache_iterator_stmt_is_visible()`. A reader must also ignore
      cached intervals if any of the tuples used as a boundary is invisible
      from the read view, see `vy_cache_iterator_skip_to_read_view()`.
      There's a bug in `vy_cache_iterator_restore()` because of which such
      an interval may be returned to the reader: when we step backwards
      from the last returned tuple we consider only one of the boundaries.
      As a result, if the other boundary is invisible from the read view,
      the reader will assume there's nothing in the index between the
      boundaries and skip reading older sources (memory, disk). Fix this by
      always checking if the other boundary is visible.
      
      Closes #10109
      
      NO_DOC=bug fix
      
      (cherry picked from commit 7b72080d)
      1bad1afc
    • Vladimir Davydov's avatar
      vinyl: fix run iterator skipping tuples following non-terminal statement · 56b6ed79
      Vladimir Davydov authored
      If a run iterator is positioned at a non-terminal statement (UPSERT or
      UPDATE), `vy_run_iterator_next()` will iterate over older statements
      with the same key using `vy_run_iterator_next_lsn()` to build the key
      history. While doing so, it may reach the end of the run file (if the
      current key is the last in the run). This would stop iteration
      permanently, which is apparently wrong for reverse iterators (LE or LT):
      if this happens the run iterator won't return any keys preceding the
      last one in the run file. Fix this by removing `vy_run_iterator_stop()`
      from `vy_run_iterator_next_lsn()`.
      
      Part of #10109
      
      NO_DOC=bug fix
      NO_CHANGELOG=next commit
      
      (cherry picked from commit 72763f94)
      56b6ed79
  2. Jun 10, 2024
    • Yaroslav Lobankov's avatar
      ci: fix RPM package builds on aarch64 runners · 715abaaf
      Yaroslav Lobankov authored
      We're using LXD containers as aarch64 runners. For some reason, OOM
      killer just kills the compilation process while package building when
      `make -j $(nproc)`. The issue happens only with builds where LTO is
      enabled. It's found, that `-j6` works fine. The bigger value causes
      problems.
      
      NO_DOC=ci
      NO_TEST=ci
      NO_CHANGELOG=ci
      715abaaf
    • Yaroslav Lobankov's avatar
      test: bump test-run to new version · 4d8dc4f2
      Yaroslav Lobankov authored
      Bump test-run to new version with the following improvements:
      
      - Calculate parallel jobs based on available CPUs [1]
      - Bump luatest to 1.0.1-15 (--list-test-cases) [2]
      - luatest: detox test searching code [3]
      - luatest: allow to run test cases in parallel [4]
      
      [1] tarantool/test-run@182aa77
      [2] tarantool/test-run@1fbbf9a
      [3] tarantool/test-run@3b0ccd0
      [4] tarantool/test-run@dd00063
      
      NO_DOC=test
      NO_TEST=test
      NO_CHANGELOG=test
      
      (cherry picked from commit 32bcea7d)
      4d8dc4f2
    • Yaroslav Lobankov's avatar
      ci: disable workaround for LuaJIT profiling tests on aarch64 runners · 307e3377
      Yaroslav Lobankov authored
      Disable workaround for LuaJIT profiling tests on aarch64 runners due to
      the following error:
      
          mount: /tmp/luajit-test-vardir: mount failed: Operation not permitted
      
      Looks like it happens because our aarch64 runners are LXD containers.
      
      NO_DOC=ci
      NO_TEST=ci
      NO_CHANGELOG=ci
      
      (cherry picked from commit e64457d9)
      307e3377
    • Vladimir Davydov's avatar
      vinyl: fix crash on invalid upsert · ca21e6d5
      Vladimir Davydov authored
      `vy_apply_result_does_cross_pk()` must be called after the new tuple
      format is validated, otherwise it may crash in case the new tuple has
      fields conflicting with the primary key definition.
      
      While we are at it, fix the operation cursor (`ups_ops`) not advanced
      on this kind of error. This resulted in skipped `upsert` statements
      following an invalid `upsert` statement in a transaction.
      
      Closes #10099
      
      NO_DOC=bug fix
      
      (cherry picked from commit dd0ac814)
      ca21e6d5
  3. Jun 07, 2024
    • Vladimir Davydov's avatar
      vinyl: fix crash on extending secondary key parts with primary · 05fa2f74
      Vladimir Davydov authored
      If a secondary index is altered in such a way that its key parts are
      extended with the primary key parts, rebuild isn't required because
      `cmp_def` doesn't change, see `vinyl_index_def_change_requires_rebuild`.
      In this case `vinyl_index_update_def` will try to update `key_def` and
      `cmp_def` in-place with `key_def_copy`. This will lead to a crash
      because the number of parts in the new `key_def` is greater.
      
      We can't use `key_def_dup` instead of `key_def_copy` there because
      there may be read iterators using the old `key_def` by pointer so
      there's no other option but to force rebuild in this case.
      
      The bug was introduced in commit 64817066 ("vinyl: use update_def
      index method to update vy_lsm on ddl").
      
      Closes #10095
      
      NO_DOC=bug fix
      
      (cherry picked from commit 9b817848)
      05fa2f74
    • Vladimir Davydov's avatar
      vinyl: fix crash in index drop if there is DML request reading from it · f7f01196
      Vladimir Davydov authored
      A DML request (insert, replace, update) can yield while reading from
      the disk in order to check unique constraints. In the meantime the index
      can be dropped. The DML request can't crash in this case thanks to
      commit d3e12369 ("vinyl: abort affected transactions when space is
      removed from cache"), but the DDL operation can because:
       - It unreferences the index in `alter_space_commit`, which may result
         in dropping the LSM tree with `vy_lsm_delete`.
       - `vy_lsm_delete` may yield in `vy_range_tree_free_cb` while waiting
         for disk readers to complete.
       - Yielding in commit triggers isn't allowed (crashes).
      
      We already fixed a similar issue when `index.get` crashed if raced
      with index drop, see commit 75f03a50 ("vinyl: fix crash if space is
      dropped while space.get is reading from it"). Let's fix this issue in
      the same way - by taking a reference to the LSM tree while checking
      unique constraints. To do that it's enough to move `vy_lsm_ref` from
      `vinyl_index_get` to `vy_get`.
      
      Also, let's replace `vy_slice_wait_pinned` with an assertion checking
      that the slice pin count is 0 in `vy_range_tree_free_cb` because
      `vy_lsm_delete` must not yield.
      
      Closes #10094
      
      NO_DOC=bug fix
      
      (cherry picked from commit bde28f0f)
      f7f01196
    • Vladimir Davydov's avatar
      tuple: fix crash on hashing tuple with double fields · 73dd3a8e
      Vladimir Davydov authored
      `tuple_hash_field()` doesn't advance the MsgPack cursor after hashing
      a tuple field with the type `double`, which can result in crashes both
      in memtx (while inserting a tuple into a hash index) and in vinyl
      (while writing a bloom filter on dump or compaction).
      
      The bug was introduced by commit 51af059c ("box: compare and hash
      msgpack value of double key field as double").
      
      Closes #10090
      
      NO_DOC=bug fix
      
      (cherry picked from commit bc0daf99)
      73dd3a8e
  4. Jun 06, 2024
    • Nikolay Shirokovskiy's avatar
      test: bump test-run to new version · 9b8fb7ab
      Nikolay Shirokovskiy authored
      Bump test-run to new version with the following improvements:
      
      - Bump luatest to 1.0.1-14-gdfee2f3 [1]
      - Adjust test result report width to terminal size [2]
      - dispatcher: lift pipe buffer size restriction [3]
      - flake8: fix E721 do not compare types [4]
      
      [1] tarantool/test-run@84ebae5
      [2] tarantool/test-run@1724211
      [3] tarantool/test-run@81259c4
      [4] tarantool/test-run@1037299
      
      We also have to fix several tests that check that script with luatest
      assertions have empty stderr output. test-run brings Luatest which
      logs assertions at 'info' level.
      
      Note that gh_8433_raft_is_candidate_test is different. Original
      assertion involves logging huge tables that have somewhere closed
      sockets inside. And 'socket.__tostring' currently raises error for
      closed sockets.
      
      NO_DOC=submodule bump
      NO_TEST=submodule bump
      NO_CHANGELOG=submodule bump
      
      (cherry picked from commit 97a801e1)
      9b8fb7ab
    • Oleg Chaplashkin's avatar
      test: bump test-run to new version · ac9e8897
      Oleg Chaplashkin authored
      Bump test-run to new version with the following improvements:
      
      - Bump luatest to 1.0.1-5-g105c69d [1]
      - tap13: fix worker fail on failed TAP13 parsing [2]
      
      [1] tarantool/test-run@ed5b623
      [2] tarantool/test-run@7c1a0a7
      
      NO_DOC=test
      NO_TEST=test
      NO_CHANGELOG=test
      
      (cherry picked from commit 4466deaf)
      ac9e8897
  5. May 30, 2024
  6. May 29, 2024
    • Georgiy Lebedev's avatar
      txn: run statement `on_rollback` triggers before rolling back statement · 41af99a2
      Georgiy Lebedev authored
      Logically, we call triggers after running statements. These triggers can
      make significant changes (for instance, DDL triggers), so, for consistency,
      we should call the statement's `on_rollback` triggers before rolling back
      the statement. This also adheres to the logic that transaction
      `on_rollback` triggers are called before rolling back individual
      transaction statements.
      
      One particular bug that this patch fixes is rolling back of DDL on the
      `_space` space. DDL is essentially a replace operation on the `_space`
      space, which also invokes the `on_replace_dd_space` trigger. In this
      trigger, among other things, we swap the indexes of the original space,
      `alter->old_space`, which is equal to the corresponding transaction
      `stmt->space`, with the indexes of the newly created space,
      `alter->new_space`:
      https://github.com/tarantool/tarantool/blob/de80e0264f7deb58ea86ef85b37b92653a803430/src/box/alter.cc#L1036-L1047
      
      If then a rollback happens, we first rollback the replace operation, using
      `stmt->space`, and only after that do we swap back the indexes in
      `alter_space_rollback`:
      https://github.com/tarantool/tarantool/blob/de80e0264f7deb58ea86ef85b37b92653a803430/src/box/memtx_engine.cc#L659-L669
      https://github.com/tarantool/tarantool/blob/de80e0264f7deb58ea86ef85b37b92653a803430/src/box/alter.cc#L916-L925
      
      For DDL on the _space space, the replace operation and DDL occur on the
      same space. This means that during rollback of the replace, we will try to
      do a replace in the empty indexes that were created for `alter->new_space`.
      Not only does this break the replace operation, but also the newly inserted
      tuple, which remains in the index, gets deleted, and access to it causes
      undefined behavior (heap-use-after-free).
      
      As part of the work on this patch, tests of rollback of DDL on system
      spaces which use `on_rollback` triggers were enumerated:
      * `_sequence` — box/sequence.test.lua;
      * `_sequence_data` — box/sequence.test.lua;
      * `_space_sequence` — box/sequence.test.lua;
      * `_trigger` — sql/ddl.test.lua, sql/errinj.test.lua;
      * `_collation` — engine-luatest/gh_4544_collation_drop_test.lua,
                       box/ddl_collation.test.lua;
      * `_space` — box/transaction.test.lua, sql/ddl.test.lua;
      * `_index` — box/transaction.test.lua, sql/ddl.test.lua;
      * `_cluster` — box/transaction.test.lua;
      * `_func` — box/transaction.test.lua, box/function1.test.lua;
      * `_priv` — box/errinj.test.lua,
                  box-luatest/rollback_ddl_on__priv_space_test.lua;
      * `_user` — box/transaction.test.lua,
                  box-luatest/gh_4348_transactional_ddl_test.lua.
      
      Closes #9893
      
      NO_DOC=<bugfix>
      
      (cherry picked from commit d529082f)
      41af99a2
    • Georgiy Lebedev's avatar
      box: pass statement being rolled back (if any) to `priv_grant` · 83ae9be8
      Georgiy Lebedev authored
      In scope of #9893 we are going to run statement `on_rollback` triggers
      before rolling back the corresponding statement. During rollback of DDL in
      the `_priv` space, the database is accessed from `user_reload_privs` to
      reload user privileges, so we need it to account for the current statement
      being rolled back: i.e., the new tuple that was introduced (if any) must
      not be used, while the old tuple (if any) must be used.
      
      Needed for #9893
      
      NO_CHANGELOG=<refactoring>
      NO_DOC=<refactoring>
      
      (cherry picked from commit 797c04ff)
      83ae9be8
    • Ilya Verbin's avatar
      txn: pass txn_stmt instead of txn to on_commit/on_rollback · 817697e8
      Ilya Verbin authored
      Currently on_rollback triggers are called on rollback of the whole
      transaction. To make it possible to invoke them on rollback to a
      savepoint, we need to pass a statement at which the savepoint was
      created.
      
      Needed for #9340
      
      NO_DOC=refactoring
      NO_TEST=refactroring
      NO_CHANGELOG=refactoring
      
      (cherry picked from commit a1d85827)
      817697e8
    • Vladislav Shpilevoy's avatar
      test: fix flaky downstream lag test · abf52e08
      Vladislav Shpilevoy authored
      It could fail in ASAN build. Can't tell why just there.
      
      The main reason was that in a topology server1 + server2->server3
      one of the cases
      - did a txn on server1,
      - then enabled server2->server3 replication,
      - then waited for server2->server3 sync,
      - and instantly assumed the txn reached server3.
      
      Surely it not always did. At the server2->server3 sync the txn
      might not had reached server2 itself yet.
      
      The fix is as simple as explicitly ensure the txn is on server2
      before waiting server2->server3 sync.
      
      Another potential for flakiness was that the default timeout in
      luatest.helpers.retrying is super low, just 5 seconds. The patch
      manually bumps it to 60 seconds to be sure any future failures
      wouldn't be related to too small timeout.
      
      Closes #10031
      
      NO_DOC=test
      NO_CHANGELOG=test
      
      (cherry picked from commit d4ea121b)
      abf52e08
    • Georgiy Lebedev's avatar
      static-build: bump the OpenSSL library version to 3.2.1 · 5d1f8c48
      Georgiy Lebedev authored
      
      Bump the OpenSSL library version to 3.2.1 and remove OpenSSL patches which
      are already present in the updated library version.
      
      Disable modules in OpenSSL configuration to make sure the OpenSSL 3.0
      legacy provider is compiled into the library.
      
      Closes #7502
      
      NO_DOC=<dependency bump>
      NO_TEST=<dependency bump>
      
      Co-authored-by: default avatarSergey Bronnikov <sergeyb@tarantool.org>
      (cherry picked from commit 8de22969)
      5d1f8c48
  7. May 23, 2024
    • Nikolay Shirokovskiy's avatar
      core: build fix for recent gcc · 69f2ddfc
      Nikolay Shirokovskiy authored
      ```
      /home/shiny/dev/tarantool/src/lib/core/coio_task.c:114:58:
      	error: ‘calloc’ sizes specified with ‘sizeof’ in the earlier argument
      	and not in the later argument [-Werror=calloc-transposed-args]
        114 |         struct cord *cord = (struct cord *)calloc(sizeof(struct cord), 1);
      ```
      
      NO_TEST=build fix
      NO_CHANGELOG=build fix
      NO_DOC=build fix
      
      (cherry picked from commit fb6b6c60)
      69f2ddfc
  8. May 22, 2024
    • Andrey Saranchin's avatar
      space_upgrade: respect min_field_count of both old and new formats · 5372ef8a
      Andrey Saranchin authored
      When upgrading a space, attribute `has_optional_parts` of indexes can be
      changed. So in order to correctly index both old and new tuples we should
      set new min_field_count value to the minimal min_field_count of old and
      new formats. Actual value will be set when space upgrade completes.
      
      Part of tarantool/tarantool-ee#698
      Part of tarantool/tarantool-ee#750
      
      NO_TEST=in ee
      NO_CHANGELOG=in ee
      NO_DOC=bugfix
      
      (cherry picked from commit c449ada4)
      5372ef8a
  9. May 21, 2024
    • Serge Petrenko's avatar
      wal: fix wal_queue_max_size assignment during initial box.cfg · 359df4fc
      Serge Petrenko authored
      wal_queue_max_size took effect only after the initial box.cfg call,
      meaning that users with non-zero `replication_sync_timeout` still synced
      using the default 16 Mb queue size. In some cases the default was too
      big and the same issues described in #5536 arose.
      
      Fix this.
      
      Closes #10013
      
      NO_DOC=bugfix
      
      (cherry picked from commit ab0f7913)
      359df4fc
  10. May 20, 2024
    • Vladimir Davydov's avatar
      vinyl: fix index name in duplicate key error message · 77fb489a
      Vladimir Davydov authored
      The code setting ER_TUPLE_FOUND uses index_name_by_id() to find
      the index name, but it passes an index in the dense index map to
      it while the function expects an index in the sparse index map.
      Apparently, this doesn't work as expected after an index is removed
      from the middle of the index map. This bug was introduced by
      commit fc3834c0 ("vinyl: check key uniqueness before modifying
      tx write set").
      
      Instead of just fixing the index passed to index_name_by_id(), we do
      a bit of refactoring. We stop passing index_name and space_name to
      vy_check_is_unique_*() functions and instead get them right before
      raising ER_TUPLE_FOUND. Note, to get the space name, we need to call
      space_by_id() but it should be fine because (a) the space is very likely
      to be cached as the last accessed one and (b) this is an error path so
      it isn't performance critical. We also drop index_name_by_id() and
      extract the index name from the LSM tree object.
      
      Closes #5975
      
      NO_DOC=bug fix
      
      (cherry picked from commit 2cfba5eb)
      77fb489a
    • Vladimir Davydov's avatar
      vinyl: fix index build crash on invalid UPSERT · 83d7fe10
      Vladimir Davydov authored
      Like UPDATE, UPSERT must not modify primary key parts. Unlike UPDATE,
      such an invalid UPSERT statement doesn't fail (raise an error) - we
      just log the error and ignore the statement. The problem is, we don't
      clear txn_stmt. As a result, if we're currently building a new index,
      the on_replace trigger installed by the build procedure will try to
      process this statement, triggering the assertion in the transaction
      manager that doesn't expect any statements in a secondary index without
      the corresponding statement in the primary index:
      
        ./src/box/vy_tx.c:728: vy_tx_prepare:
          Assertion `lsm->space_id == current_space_id' failed.
      
      Let's fix this by clearing the txn_stmt corresponding to a skipped
      UPSERT.
      
      Note, this also means that on_replace triggers installed by the user
      won't run on invalid UPSERT (hence test/vinyl/on_replace.result update),
      but this is consistent with the memtx engine, which doesn't run them
      in this case, either.
      
      Closes #10026
      
      NO_DOC=bug fix
      
      (cherry picked from commit 5ac0d26a)
      83d7fe10
  11. May 17, 2024
    • Vladislav Shpilevoy's avatar
      relay: update lag on any acked txn · 79a8e82e
      Vladislav Shpilevoy authored
      Not only for own txns, but also on the txns authored by other
      instances.
      
      Note that the lag isn't updated when the replica got new txns from
      another master. The lag still only reflects the replication
      between this relay and its specific applier.
      
      The motivation is that otherwise the lag sometimes shows
      irrelevant things, like that the replica is very outdated, while
      it keeps replicating just fine. Only not txns of this specific
      master, who might even turned into a replica itself already.
      
      Closes #9748
      
      NO_DOC=bugfix
      
      (cherry picked from commit 39af9fbe)
      79a8e82e
    • Vladislav Shpilevoy's avatar
      relay: enforce prev and new ack vclocks relation · aeaca11e
      Vladislav Shpilevoy authored
      From the code it isn't obvious, but relay->status_msg.vclock and
      relay->last_recv_ack.vclock are both coming from the applier.
      Status_msg is the previous ack, last_recv_ack is the latest ack.
      
      They can never go down. And are not affected anyhow by the master
      committing its own transactions. I.e. master can commit something,
      relay->r->vclock (recovery cursor) will go up, and recovery vclock
      might become incomparable with the last ACK vclock. But the prev
      and last ACK vclocks are always comparable and always go up.
      
      This invariant was broken though, because relay on restart didn't
      nullify the current applier status (status_msg). It could break
      if the replica would loose its xlog files or its ID would be
      taken by another instance - then its vclock would go down, making
      last_recv_ack.vclock < status_msg.vclock. But that is not right
      and is fixed in this patch.
      
      In scope of #9748
      
      NO_DOC=bugfix
      NO_TEST=test 5158 already covers it
      NO_CHANGELOG=bugfix
      
      (cherry picked from commit 71dbb47c)
      aeaca11e
    • Vladislav Shpilevoy's avatar
      relay: move ack handling into new function · 68fdba93
      Vladislav Shpilevoy authored
      To reduce the insane indentation level. And to isolate the further
      changes in next commits more.
      
      Part of #9748
      
      NO_DOC=refactoring
      NO_TEST=refactoring
      NO_CHANGELOG=refactoring
      
      (cherry picked from commit d6f15a10)
      68fdba93
    • Vladislav Shpilevoy's avatar
      applier: drop lag to zero on reconnect · 9ce12d45
      Vladislav Shpilevoy authored
      Before the patch if the applier was reconnected, the master would
      see downstream lag equal to the time since it replicated the last
      txn to this applier.
      
      This happened because applier between reconnects kept the txn
      timestamp used for acks. On the master's side the relay was
      recreated, received the ack, thought the applier just applied this
      txn, and displayed this as a lag.
      
      The test makes a master restart because this is the easiest way to
      reproduce it. Most importantly, the applier shouldn't be
      re-created, and relay should restart.
      
      Part of #9748
      
      NO_DOC=bugfix
      NO_CHANGELOG=later
      
      (cherry picked from commit dda42035)
      9ce12d45
    • Vladislav Shpilevoy's avatar
      applier: move applier_txn_last_tm into applier · bda619c2
      Vladislav Shpilevoy authored
      It was stored in struct replica, now is in struct applier. The
      motivation is that applier-specific data must be inside the
      applier.
      
      Also it makes the next commits look more logical. They are going
      to change this timestamp when applier progresses through its state
      machine. It looks strange when the applier is changing the replica
      object. Replica is on an upper level in the hierarchy. It owns the
      applier and the applier ideally mustn't know about struct replica
      (hardly possible to achieve), or at least not change it (this is
      feasible).
      
      In scope of #9748
      
      NO_DOC=internal
      NO_TEST=refactoring
      NO_CHANGELOG=refactoring
      
      (cherry picked from commit 8e5d9f2a)
      bda619c2
    • Vladimir Davydov's avatar
      vinyl: fix bug when tuple not committed to unique nullable index · 4db12994
      Vladimir Davydov authored
      A unique nullable key definition extended with primary key parts
      (cmp_def) assumes that two tuples are equal *without* comparing
      primary key fields if all secondary key fields are equal and not
      nulls, see tuple_compare_slowpath(). This is a hack required to
      ignore the uniqueness constraint for nulls in memtx. The memtx
      engine can't use the secondary key definition as is (key_def) for
      comparing tuples in the index tree, as it does for a non-nullable
      unique index, because this wouldn't allow insertion of any
      duplicates, including nulls. It couldn't use cmp_def without this
      hack, either, because then conflicting tuples with the same
      secondary key fields would always compare as not equal due to
      different primary key parts.
      
      For Vinyl, this hack isn't required because it explicitly skips
      the uniqueness check if any of the indexed fields are nulls, see
      vy_check_is_unique_secondary(). Furthermore, this hack is harmful
      because Vinyl relies on the fact that two tuples compare as equal by
      cmp_def if and only if *all* key fields (both secondary and primary)
      are equal. For example, this is used in the transaction manager,
      which overwrites statements equal by cmp_def, see vy_tx_set_entry().
      
      Let's disable this hack by resetting unique_part_count in cmp_def.
      
      Closes #9769
      
      NO_DOC=bug fix
      
      (cherry picked from commit 2e689063)
      4db12994
  12. May 16, 2024
    • Vladimir Davydov's avatar
      vinyl: fix use-after-free of LSM tree in scheduler · 660c355f
      Vladimir Davydov authored
      Between picking an LSM tree from a heap and taking a reference to it in
      vy_task_new() there are a few places where the scheduler may yield:
       - in vy_worker_pool_get() to start a worker pool;
       - in vy_task_dump_new() to wait for a memory tree to be unpinned;
       - in vy_task_compaction_new() to commit an entry to the metadata log
         after splitting or coalescing a range.
      
      If a concurrent fiber drops and deletes the LSM tree in the meanwhile,
      the scheduler will crash. To avoid that, let's take a reference to
      the LSM tree.
      
      It's quite difficult to write a functional test for it without a bunch
      of ugly error injections so we rely on fuzzing tests.
      
      Closes #9995
      
      NO_DOC=bug fix
      NO_TEST=fuzzing
      
      (cherry picked from commit 1c4605bb)
      660c355f
  13. May 14, 2024
    • Alexander Turenko's avatar
      test/interactive: add connect() function · ba66508f
      Alexander Turenko authored
      It encapsulates all the needed actions to connect to a remote console
      using a Unix socket.
      
      Part of #9985
      
      NO_DOC=testing helper change
      NO_CHANGELOG=see NO_DOC
      
      (cherry picked from commit bb430c55)
      ba66508f
    • Alexander Turenko's avatar
      test/interactive: disable hide/show prompt feature · c9d5b345
      Alexander Turenko authored
      See #7169 for details about the hide/show prompt feature. In short, it
      hides readline's prompt before `print()` or `log.<level>()` calls and
      restores the prompt afterwards.
      
      This feature sometimes badly interferes with
      `test.interactive_tarantool` heuristics about readline's command
      echoing.
      
      This commit disables the feature in `test.interactive_tarantool` by
      default and enables it explicitly where needed.
      
      Part of #9985
      
      NO_DOC=testing helper change
      NO_CHANGELOG=see NO_DOC
      
      (cherry picked from commit 23094b6f)
      c9d5b345
    • Alexander Turenko's avatar
      test/interactive: allow to assert for nil response · 73e03318
      Alexander Turenko authored
      Before this patch the `:roundtrip()` method in the
      `test.interative_tarantool` instance considered the following calls as
      equivalent:
      
      ```lua
      g.it = it.new()
      
      -- Doesn't check the response.
      g.it:roundtrip('x')
      
      -- Before the patch it was the same as above.
      --
      -- Now it checks that the response is nil.
      local expected = nil
      g.it:roundtrip('x', expected)
      
      -- It is the same as previous.
      g.it:roundtrip('x', nil)
      ```
      
      Now the response is checked against the provided expected value if the
      value is passed to arguments, even if it is `nil`.
      
      Also, a command's response is now returned from the method. It may be
      useful if the response returns some dynamic information (such as a TCP
      port number or a file descriptor) that is used later in the test or if
      the response should be verified in some non-trivial way, not just a deep
      compare.
      
      The `:roundtrip()` method is just `:execute_command()` plus
      `:read_response()` plus `luatest.assert_equals()`. However, I like using
      `:roundtrip()` even when the assertion is not needed, because it is
      shorter and because using the same method brings less context to a
      reader.
      
      For example,
      
      ```lua
      g.it = it.new()
      g.it:roundtrip('x = 2')
      g.it:roundtrip('y = 3')
      g.it:roundtrip('x + y', 6)
      ```
      
      Part of #9985
      
      NO_DOC=testing helper change
      NO_CHANGELOG=see NO_DOC
      NO_TEST=see NO_DOC
      
      (cherry picked from commit 7d9e8569)
      73e03318
  14. May 08, 2024
    • Vladislav Shpilevoy's avatar
      relay: fix timeout on large xlog subscribe start · df6988e6
      Vladislav Shpilevoy authored
      When a replica subscribes, it might in the beginning try to
      position its reader cursor to the end of a large xlog file.
      
      Positioning inside of this file can take significant time during
      which the WAL reader yielded and tried to send heartbeats, but
      couldn't, because the relay thread wasn't communicating with the
      TX thread.
      
      When there are no messages from TX for too long time, the
      heartbeats to the replica are not being sent
      (commit 56571d83 ("raft: make
      followers notice leader hang")).
      
      The relay must communicate with the TX thread even when subscribe
      is just being started and opens a large xlog file.
      
      This isn't the first time when the missing heartbeats result into
      timeouts. See more here:
      
      - commit 30ad4a55 ("relay: yield
          explicitly every N sent rows").
      
      - commit 17289440 ("recovery: make
          it yield when positioning in a WAL").
      
      - commit ee6de025 ("relay: send
          heartbeats while reading a WAL").
      
      Given that this is fixed fourth time, it might suggest that the
      relay has not the best architecture having some slight drawbacks.
      See more in #9968.
      
      Closes #9094
      
      NO_DOC=bugfix
      
      (cherry picked from commit f7e6686a)
      df6988e6
  15. Apr 25, 2024
    • Sergey Vorontsov's avatar
      ci: add timeouts for workflow jobs · b30a5ad8
      Sergey Vorontsov authored
      By default, each job in a workflow can run for up to 6 hours of the
      execution time. If a job reaches this limit, the job is terminated by
      GitHub automatically and fails to complete. This patch sets job timeouts
      to 60 minutes to avoid waiting for jobs to complete for 6 hours.
      
      NO_DOC=ci
      NO_TEST=ci
      NO_CHANGELOG=ci
      b30a5ad8
  16. Apr 23, 2024
    • Georgiy Lebedev's avatar
      netbox: close transport after stopping worker loop and wait for the stop · c4c022e2
      Georgiy Lebedev authored
      
      Currently, we close the transport from transport from
      `luaT_netbox_transport_stop`, and we do not wait for the worker fiber to
      stop. This causes several problems.
      
      Firstly, the worker can switch context by yielding (`coio_wait`) or
      entering the Lua VM (`netbox_on_state_change`). During a context switch,
      the connection can get closed. When the connection is closed, its receive
      buffer is reset. If there was some pending response that was partially
      retrieved (e.g., a large select), then after resetting the buffer we will
      read some inconsistent data. We must not allow this to happen, so let's
      check for this case after returning from places where the worker can switch
      context. In between closing the connection and cancelling the connection's
      worker, an `on_disconnect` trigger can be called, which, in turn, can
      also yield, returning control to the worker before it gets cancelled.
      
      Secondly, when the worker enters the Lua VM, garbage collection can be
      triggered and the connection owning the worker could get closed
      unexpectedly to the worker.
      
      The fundamental source of these problems is that we close the transport
      before the worker's loop stops. Instead, we should close it after the
      worker's loop stops. In `luaT_netbox_transport_stop`, we should only cancel
      the worker, and either wait for the worker to stop, if we are not executing
      on it, or otherwise throw an exception (`luaL_testcancel`) to stop the
      worker's loop. The user will still have the opportunity to catch this
      exception and prevent stoppage of the worker at his own risk. To safeguard
      from this scenario, we will now keep the `is_closing` flag enabled once
      `luaT_netbox_transport_stop` is called and never disable it.
      
      There also still remains a special case of the connection getting garbage
      collected, when it is impossible to stop the worker's loop, since we cannot
      join the worker (yielding is forbidden from finalizers), and an exception
      will not go past the finalizer. However, this case is safe, since the
      connection is not going to be used by this point, so the worker can simply
      stop on its own at some point. The only thing we need to account for is
      that we cannot wait for the worker to stop: we can reuse the `wait` option
      of `luaT_netbox_transport_stop` for this.
      
      Closes #9621
      Closes #9826
      
      NO_DOC=<bugfix>
      
      Co-authored-by: default avatarVladimir Davydov <vdavydov@tarantool.org>
      (cherry picked from commit fcf7f5c4)
      
      Cherry pick note: Dropped gh_9621_netbox_worker_crash_test because
      box.iproto.encode helpers aren't available on 2.11.
      c4c022e2
  17. Apr 19, 2024
  18. Apr 17, 2024
  19. Apr 16, 2024
    • Aleksandr Lyapunov's avatar
      memtx: fix a bug with mvcc and exclude_null option · 364af9bf
      Aleksandr Lyapunov authored
      Before this patch MVCC engine expected that if index_replace
      sets `result` to NULL then index_replace sets `successor` to
      something (NULL or existing tuple, depending on index type).
      That looked fine because by contract `successor` is set when
      true insertion was happened.
      
      Unfortunately it was not considered that in case of part with
      `exclude_null` option in index the insertion can be silently
      skipped and thus `successor` can be not set. The latter access
      of it was actually an UB.
      
      Fix it by explicit check of tuple_key_is_excluded and work on
      this case correctly.
      
      Note that logically `index_replace` should return a flag whether
      the new tuple was filtered (excluded) by key_def. But on the other
      hand this flag is required only for mvcc while the function is
      already has lots of arguments and it's very cheap to determine
      this flag right from memtx_tx, so I decided to make the most
      simple patch.
      
      NO_DOC=bugfix
      
      (cherry picked from commit 14e21297)
      364af9bf
Loading