Skip to content
Snippets Groups Projects
  1. May 27, 2020
    • Vladislav Shpilevoy's avatar
      build: link bit library to vclock library · 74ac4263
      Vladislav Shpilevoy authored
      Vclock depends on bit library, but wasn't linked to it.
      74ac4263
    • Nikita Pettik's avatar
      vinyl: unthrottle scheduler on manual checkpoint · 7305fe5a
      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
      7305fe5a
    • Nikita Pettik's avatar
      engine: add is_scheduled arg to engine->begin_checkpoint · 00c582f0
      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
      00c582f0
    • Nikita Pettik's avatar
      vinyl: fix assert in vy_tx_write() · f6cb9289
      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
      f6cb9289
  2. May 25, 2020
    • Vladislav Shpilevoy's avatar
      vinyl: add missing diag_set in space creation · 2cf4450f
      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.
      2cf4450f
    • Vladislav Shpilevoy's avatar
      build: turn off LTO for exports.c · 36927e54
      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
      36927e54
  3. May 21, 2020
  4. May 20, 2020
    • Nikita Pettik's avatar
      vinyl: fix comment to vy_read_iterator_cmp_stmt() · 4a98a9bb
      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.
      4a98a9bb
    • Kirill Yukhin's avatar
      small: bump new version · bb7c3d16
      Kirill Yukhin authored
      Add MAP_STACK to mmap() flags.
      bb7c3d16
    • Sergey Bronnikov's avatar
      say: fix compilation on OpenBSD · 64308484
      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
      64308484
    • Sergey Bronnikov's avatar
      Include libgen.h when building on OpenBSD · fd653200
      Sergey Bronnikov authored
      Part of #4967
      fd653200
    • Sergey Bronnikov's avatar
      sql: use mremap() on OpenBSD · 43ffbba6
      Sergey Bronnikov authored
      Part of #4967
      43ffbba6
    • Sergey Bronnikov's avatar
      Fix building of tt_pthread_attr_getstack() on OpenBSD · 15601e09
      Sergey Bronnikov authored
      Part of #4967
      15601e09
    • Sergey Bronnikov's avatar
      build: skip linking with -ldl on OpenBSD · 291c392a
      Sergey Bronnikov authored
      OpenBSD includes DL library in a base system
      
      Part of #4967
      291c392a
    • Alexander Turenko's avatar
      test: popen: fix popen test 'hang' under test-run · 0afba959
      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
      0afba959
    • Alexander Turenko's avatar
      popen: fix access to freed memory after :close() · 01211e8e
      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
      01211e8e
  5. May 19, 2020
    • Vladislav Shpilevoy's avatar
      session: remove box.session.push() 'sync' · a38c861e
      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.
      a38c861e
    • Kirill Yukhin's avatar
      small: revert previous bump · 61d2cb64
      Kirill Yukhin authored
      Revert 03790ac5 commit in small library part.
      61d2cb64
  6. May 18, 2020
    • Vladislav Shpilevoy's avatar
      cmake: remove dynamic-list linker option · 03790ac5
      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
      03790ac5
    • Vladislav Shpilevoy's avatar
      cmake: remove double usage of some source files · 55171895
      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.
      55171895
    • Cyrill Gorcunov's avatar
      box/console: switch to new lua serializer · ae7e2103
      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: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      ae7e2103
    • Cyrill Gorcunov's avatar
      box/console: implement lua serializer · 35058d22
      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: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      35058d22
    • Cyrill Gorcunov's avatar
      box/console: rename luaL_yaml_default to serializer_yaml · ec34fcdc
      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: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      ec34fcdc
    • Cyrill Gorcunov's avatar
      box/console: rename format to format_yaml · acc83768
      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: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      acc83768
    • Cyrill Gorcunov's avatar
      box/console: use tabs instead of spaces in consolelib · 7225ba71
      Cyrill Gorcunov authored
      
      For some reason we're using spaces here to adjust code.
      
      Signed-off-by: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      7225ba71
    • Cyrill Gorcunov's avatar
      box/console: console_session_vtab -- use designated initialization · a2e34240
      Cyrill Gorcunov authored
      
      For better maintainance.
      
      Signed-off-by: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      a2e34240
    • Cyrill Gorcunov's avatar
      box/memtx: use uint32_t for number of indices · 15b34012
      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: default avatarCyrill Gorcunov <gorcunov@gmail.com>
      15b34012
  7. May 15, 2020
    • Serge Petrenko's avatar
      replication: remove unnecessary errors on replicating from an anonymous instance · caf73913
      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
      caf73913
    • Serge Petrenko's avatar
      replication: add box.info.replication_anon · 4e5d60d8
      Serge Petrenko authored
      Closes #4900
      
      @TarantoolBot document
      Title: add new field to box.info: replication_anon
      
      It is now possible to list all the anonymous replicas following the
      instance with a call to `box.info.replication_anon()`
      The output is similar to the one produced by `box.info.replication` with
      an exception that anonymous replicas are indexed by their uuid strings
      rather then server ids, since server ids have no meaning for anonymous
      replicas.
      
      Note, that when you issue a plain `box.info.replication_anon`, the only
      info returned is the number of anonymous replicas following the current
      instance. In order to see the full stats, you have to call
      `box.info.replication_anon()`. This is done to not overload the `box.info`
      output with excess info, since there may be lots of anonymous replicas.
      Example:
      
      ```
      tarantool> box.info.replication_anon
      ---
      - count: 2
      ...
      
      tarantool> box.info.replication_anon()
      ---
      - 3a6a2cfb-7e47-42f6-8309-7a25c37feea1:
          id: 0
          uuid: 3a6a2cfb-7e47-42f6-8309-7a25c37feea1
          lsn: 0
          downstream:
            status: follow
            idle: 0.76203499999974
            vclock: {1: 1}
        f58e4cb0-e0a8-42a1-b439-591dd36c8e5e:
          id: 0
          uuid: f58e4cb0-e0a8-42a1-b439-591dd36c8e5e
          lsn: 0
          downstream:
            status: follow
            idle: 0.0041349999992235
            vclock: {1: 1}
      ...
      
      ```
      
      Note, that anonymous replicas hide their lsn from the others, so
      anonymous replica lsn will always be reported as zero, even if anonymous
      replicas perform some local space operations.
      To know the anonymous replica's lsn, you have to issue `box.info.lsn` on
      it.
      4e5d60d8
  8. May 12, 2020
    • Nikita Pettik's avatar
      vinyl: drop wasted runs in case range recovery fails · 32f59756
      Nikita Pettik authored
      
      If recovery process fails during range restoration, range itself is
      deleted and recovery is assumed to be finished as failed (in case of
      casual i.e. not forced recovery). During recovery of particular range,
      runs to be restored are refed twice: once when they are created at
      vy_run_new() and once when they are attached to slice. This fact is
      taken into consideration and after all ranges are recovered: all runs of
      lsm tree are unrefed so that slices own run resources (as a result, when
      slice is to be deleted its runs unrefed and deleted as well). However, if
      range recovery fails, range is dropped alongside with already recovered
      slices. This leads to unrefing runs - this is not accounted. To sum up
      recovery process below is a brief schema:
      
      foreach range in lsm.ranges {
        vy_lsm_recover_range(range) {
          foreach slice in range.slices {
            // inside recover_slice() each run is refed twice
            if vy_lsm_recover_slice() != 0 {
              // here all already restored slices are deleted and
              // corresponding runs are unrefed, so now they have 1 ref.
              range_delete()
            }
          }
        }
      }
      foreach run in lsm.runs {
        assert(run->refs > 1)
        vy_run_unref(run)
      }
      
      In this case, unrefing such runs one more time would lead to their
      destruction. On the other hand, iteration over runs may turn out to
      be unsafe, so we should use rlist_foreach_entry_safe(). Moreover, we
      should explicitly clean-up these runs calling vy_lsm_remove_run().
      
      Reviewed-by: default avatarVladislav Shpilevoy <vshpilevoi@mail.ru>
      
      Closes #4805
      32f59756
    • Nikita Pettik's avatar
      errinj: introduce delayed injection · 9d4ac029
      Nikita Pettik authored
      
      With new macro ERROR_INJECT_COUNTDOWN it is possible to delay error
      injection by iparam value: injection will be set only after iparam
      times the path is executed. For instance:
      
      void
      foo(int i)
      {
      	/* 2 is delay counter. */
      	ERROR_INJECT_COUNTDOWN(ERRINJ_FOO, {
      		 printf("Error injection on %d cycle!\n", i);
      		});
      }
      
      void
      boo(void)
      {
      	for (int i = 0; i < 10; ++i)
      		foo(i);
      }
      
      box.error.injection.set('ERRINJ_FOO', 2)
      
      The result is "Error injection on 2 cycle!". This type of error
      injection can turn out to be useful to set injection in the middle of
      query processing. Imagine following scenario:
      
      void
      foo(void)
      {
      	int *fds[10];
      	for (int i = 0; i < 10; ++i) {
      		fds[i] = malloc(sizeof(int));
      		if (fds[i] == NULL)
      			goto cleanup;
      	}
      cleanup:
      	free(fds[0]);
      }
      
      "cleanup" section obviously contains error and leads to memory leak.
      But using means of casual error injection without delay such situation
      can't be detected: OOM can be set only for first cycle iteration and in
      this particular case no leaks take place.
      
      Reviewed-by: default avatarVladislav Shpilevoy <vshpilevoi@mail.ru>
      9d4ac029
    • Nikita Pettik's avatar
      vinyl: add test on failed write iterator during compaction · cb062017
      Nikita Pettik authored
      vy_task_write_run() is executed in auxiliary thread (dump or
      compaction). Write iterator is created and used inside this function.
      Meanwhile, creating/destroying tuples in these threads does not change
      reference counter of corresponding  tuple formats (see vy_tuple_delete()
      and vy_stmt_alloc()). Without cleaning up write iterator right in
      write_iterator_start() after fail, this procedure takes place in
      vy_task_compaction_abort() or vy_task_dump_abort().  These *_abort()
      functions in turn are executed in the main thread.  Taking this into
      consideration, tuple might be allocated in aux. thread and deleted in
      the main thread. As a result, format reference counter might decrease,
      whereas it shouldn't change (otherwise tuple format will be destroyed
      before all tuples of this format are gone).
      
      Fortunately, clean-up of write iterator in another thread was found only
      on 1.10 branch, master branch already contains fix but lacks test
      (2f17c929). So let's introduce test with following scenario:
      
      1. run compaction process;
      2. add one or more slice sources in vy_write_iterator_start():
      corresponding slice_stream structures obtain newly created tuples
      in vy_slice_stream_next();
      3. the next call of vy_write_iterator_add_src() fails due to OOM,
      invalid run file or whatever;
      4. if write_iterator_start() didn't provide clean-up of sources, it
      would take place in vy_task_dump_abort() which would be executed in
      the main thread;
      5. now format reference counter would be less than it was before
      compaction.
      
      Closes #4864
      cb062017
    • Nikita Pettik's avatar
      vinyl: clean-up unprocessed read views in *_build_read_views() · 521a6fbd
      Nikita Pettik authored
      vy_write_iterator->read_views[i].history objects are allocated on
      region (see vy_write_iterator_push_rv()) during building history of the
      given key. However, in case of fail of vy_write_iterator_build_history()
      region is truncated but pointers to vy_write_history objects are not
      nullified. As a result, they may be accessed (for instance while
      finalizing write_iterator object in  vy_write_iterator_stop) which in
      turn may lead to crash, segfaul or disk formatting. The same may happen
      if vy_read_view_merge() fails during processing of read view array.
      Let's clean-up those objects in case of error takes place.
      
      Part of #4864
      521a6fbd
  9. May 08, 2020
    • Alexander V. Tikhonov's avatar
      gitlab-ci: add Catalina OSX 10.15 · 76157ef6
      Alexander V. Tikhonov authored
      Added Catalina OSX 10.15 to gitlab-ci testing and removed OSX 10.13,
      due to decided to have only 2 last major releases, for now it is
      10.14 and 10.15 OSX versions. Also changed the commit job for branches
      from 10.14 to 10.15 OSX version.
      
      Additional cleanup for 'box_return_mp' and 'box_session_push',
      added API_EXPORT which defines nothrow, compiler warns or errors
      depending on the build options.
      
      Part of #4885
      Close #4873
      76157ef6
Loading