Georgy Moshkin authored 2 years ago and Georgy Moshkin committed 2 years ago

since 1.61 rust has added a `whole-archive` modifier for link targets
which is unset by default.

Previously to this version this was probably undefined behavior, because
the linking would work fine when building an executable (like picodata)
but would fail with undefined symbols when building a static library.

But now with the advent of this option, we can probably refactor
picodata into a library which can be useful for testing, i.e. we can
exclude testing stuff from the binary but still have a test binary with
all the necessary runtime symbols.

Closes #81

Pytest has a feature to segregate setup, test, and teardown logs. The
setup phase is considered to be an intialization of the fixtures. In
order to split logs properly `cluster.deploy()` is now called inside a
fixture.

Picodata already assigns `replicaset_id` to an instance when it joins,
but it wasn't used in Tarantool `box.cfg` yet. Now it is.

It's also important to set up listen port in `start_join` immediately.
Without it Tarantool will stuck waiting for connection to self.

Part of https://git.picodata.io/picodata/picodata/picodata/-/issues/52

It's already formatted in conformity to usual `cargo test`.
Also, remove unused command-line arguments from `picodata test` command.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/61

The intention is to eliminate ambiguities in the `Instance` API. Make
it more like `subprocess` module (as regards `kill` and `terminate`
functions).

Behavior of `killpg` slightly differs in Mac and Linux. For some reason,
`killpg` returns error EPERM when sending a signal to a zomibie process.
And that is the reason of `test_process_management` failure on mac -
there's a small gap between killing child and and subreaper calls
`waitpid`.

Now pytest handles this exception properly.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/70

See also:

- Stackoverflow: Why would `killpg` return "not permitted" when ownership is correct?
  https://stackoverflow.com/questions/12521705/why-would-killpg-return-not-permitted-when-ownership-is-correct

- Linux `man 2 killpg`:
  https://linux.die.net/man/2/killpg#Notes

  > Notes
  >
  > There are various differences between the permission checking in
  > BSD-type systems and System V-type systems. See the POSIX rationale
  > for kill(). A difference not mentioned by POSIX concerns the return
  > value EPERM: BSD documents that no signal is sent and EPERM returned
  > when the permission check failed for **at least one** target process,
  > while POSIX documents EPERM only when the permission check failed for
  > **all** target processes.

- MacOS `man 2 killpg`:
  https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/killpg.2.html

  > [EPERM] The sending process is not the super-user and
  >         **one or more** of the target processes has an effective
  >         user ID different from that of the sending process.

- Linux `man 2 kill`:
  https://linux.die.net/man/2/kill

  >  EPERM  The process does not have permission to send the signal
  >         *to any* of the target processes.
  >

- Process states in Linux:
  https://kerneltalks.com/linux/process-states-in-linux/

- Reproduce killpg returning EPERM on MacOS:
  https://git.picodata.io/picodata/picodata/picodata/-/snippets/7

Pytest supports running tests in parallel using the `xdist` plugin. In
order to support it in Picodata, one should avoid ports collision. It
assigns each worker a dedicated IP address `127.7.n.1`, where
`n = xdist_worker_number`. Unfortunately, it doesn't work on MacOS,
because Mac doesn't provide any loopback aliases except `127.0.0.1`
by default.

This patch provides another address generation logics. The `subnet`
parameter is superseeded with a `base_port`, that is `3300 + n * 100`.
In this way, every pytest (xdist) worker gets a dedicated port range
`[3301, 3399]`, `[3401, 3499]` and so on.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/65

When bootstrapping an instance, there're two possible execution paths -
`start_boot` and `start_join`. While `start_join` takes all uuids from
JoinResponse, `start_boot` already deals with a bootstrapped `box.cfg`
(it's done in `start_discover`, refer to [1]). In order to make uuids
consistent across `box.cfg` and topology module, `start_boot` stage is
preceded with rebootstrap.

This case is also covered with a pytest.

- [1] doc/clustering.md

- Add corresponding field to the Peer struct.
- Generate it in the topology module.
- Use it in `box.cfg`.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/51

Address `replication_factor` when choosing `relicaset_id` for a new
instance. It dosn't consider `failure_domain` yet, but takes into
account the number of instances.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/68

- Choose it in the topology module if it's not provided in a
  `JoinRequest`.
- Persist in `raft_group` space.
- Respond with an error if `JoinRequest` contains different
  `replicaset_id`.
- In `JoinResponse` it's transferred automatically.

Part of https://git.picodata.io/picodata/picodata/picodata/-/issues/51

Follow-up for https://git.picodata.io/picodata/picodata/picodata/-/issues/50

- Generate it in the topology module
- Persist it in `raft_group` space
- Transfer it in `JoinResponse`
- Use it in `box.cfg`

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/50

Alexander Tolstoy authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

Part of https://git.picodata.io/picodata/picodata/docs/-/issues/1

It's necessary to incapsulate topology management logics away from main.

It encapsulates the logics of a JoinRequest batch processing.

Topology module will be quite important in picodata. This first version
misses a lot of features, but a few commits later it's going to
implement quite a lot of logics.

When a new instance is joined - there's one complex thing: raft leader
has to decide where this new instance is going to be emplaced, i.e. what
replicaset should it join. There're many different parameters have an
influence - `repliction_factor`, `failure-domain`, and of course the
existing topology. So, this new `topology` module must make the decision.

This patch only refactors the current Picodata behavior, and doesn't
bring new features for its users. Instead, it opens the door to a future
development.

Also, this patch provides a unit-testing basis for the future features.

Both JoinRequest and JoinResponse are going to be used in other modules.
Move them one level above from `traft::node::*` to `traft::*`.

One of the most tricky Raft cases is a so-called ABA problem [1].
In that case it's important to protect a batch of join requests with a
term number. Since the whole batch is supplied with atomicity-sensitive
uuids, applying it on a different term may cause an inconsistency, which
is very, veeeery bad.

[1] https://en.wikipedia.org/wiki/ABA_problem

Implementation of `net_box` in `tarantool-module` resolves hostnames
with a `to_socket_addrs` function that is blocking. Pytest uses
fake addresses in one test, and sometimes it results in 5-second
blockage and consequent test failure.

This patch only provides a workaround. It makes a connection to fail
even before the blocking DNS request.

See also:
- https://git.picodata.io/picodata/picodata/tarantool-module/-/issues/81

Before this patch, pytest used to launch all instances in a clean
environment. It prevented running with `PICODATA_LOG_LEVEL=verbose`.

Specifying the particular directory with tests
significantly speeds up their collection and execution.

Before:

```console
$ time pytest -k nothing
12 deselected in 1.36s
```

After:

```
12 deselected in 0.04s
```

Yaroslav Dynnikov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

This patch covers one more case when discovery request is handled
by an instance that has the discovery module unitialized.

By default cargo runs tests in parallel in multiple threads.  Both
`test_log_level` and `test_parse` access environment variables which are
shared across threads. Consequently, their concurrent modification
results in the test failure.

This patch unites these two tests making it linear.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/28

Commit 1a3b5233 missed a bug. Iteration over instances could be aborted
by an exception during teardown. It resulted in garbage process
remaining alive after pytest termination.

1. Lower log level of connection errors in `netork.rs`.
2. Give raft fibers a name.