Lukas Jezek authored 4 years ago and Vladimír Čunát committed 4 years ago

vcunat change some parts of this, mainly around memory allocation.

ci: re-try OBS distrotests

See merge request !1066

Often, the VM fails to boot (even twice in the row, as already handled
by the test itself) which leads to false negative result.

These nightly tests fail far too often (at least on of the ~7 tests) and
generate annoying notifications.

Adding yet another layer of retries should reduce the number of false
negatives without diminishing the value of the test (since the real
packaging issues are 100 % reproducible).

ci: fix Deckard integration so it throws errors up the stack

Closes #611

See merge request !1065

Fixes: #611

ci: use DAG & clenaup

See merge request !1064

Use DAG dependencies to allow more precise control when a test
suite should be executed. It also enabled use of more descriptive
pipeline stages.

These tests are currently failing and since they're sort of
duplicate, and we want to get rid of them anyway, I'm removing
them from CI.

Related #612

The turris build was never migrated to support meson and these
files have been obsolete for far too long now.

This test fails far too often due to conditions inside the CI
environment. This test is already executed in test:valgrind without
paralelism, so let's keep it there instead.

daemon/worker and daemon/session cleanup

See merge request !1058

Rename "s" to "session" to be consistent with all the other
session_*() functions to make it easier to read.

If pkt was NULL, the function would return error from parse_packet(),
which would then have no effect besides returning an error code from
worker_submit(). There were also needless checks that pkt is indeed
not NULL in cases where it was no longer possible for it to be so.

This also removes assert(false) statements and simply return an error.

The variable name query is misleading, since the passed in packet
can actually be a response as well (or a malformed packet).

The action doesn't increase any counter or do any error handling.
It would simply SEGFAULT. Even if it didn't worker_submit() would
just return an error code.

daemon: handle IO error when processing wire buffer

See merge request !1054

Ensure both tasklist and waitinglist is always cleared when tearing down
connection (otherwise the session close will fail on assert).

The previous assert could be triggered when the while loop in the code
above would successfuly perform qr_task_send() for one of the
tasks in waitinglist and then fail on a subsequent one.

This fixes the following assert:
daemon/worker.c:1157: qr_task_finalize: Assertion `!session_flags(source_session)->closing' failed.

Scenario which leads to the above error:
1. We're using a stateful protocol.
2. Enough data arrive in a single tcp_recv() call to put at least
   two queries into the session's wire buffer.
3. session_wirebuf_process() calls worker_submit() which calls
   qr_task_step().
4. In the qr_task_step() the query state changes to KR_STATE_DONE,
   then qr_task_finalize() is called.
5. qr_task_send() is called, but it fails. This is where
   qr_task_finalize() closes the session, but used to return no error.
6. When the next query is processed in session_wirebuf_process(),
   steps 3 and 4 are followed and qr_task_finalize() is called.
7. Since the session is already closed, the assert is triggered.

Debugging this was a lot of fun... All hail the rr debugger!

CI: update knot3.0

See merge request !1062

It's one of the places still using Knot's void* and I managed to pass
the wrong `db` :-/

improve precision of UDP timeout measurement

See merge request !1047

Štěpán Balážik authored 4 years ago and Tomas Krizek committed 4 years ago

Previously this was done *before* calling uv_udp_send which lead to many
early retransmits (significant amount of time might pass between calling
uv_udp_send and the moment the packet is actually send to the wire).

lib/cache: type safety of the cache API pointers

See merge request !1049

Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

- The malloc-free pair could be avoided without difficulty,
  but it seemed like premature optimization.
- The libknot functions make error handling a bit difficult
  (zero is theoretically valid and doesn't show error type),
  but writing this properly without libknot would need 10-20
  additional lines of code and the risk of encountering errors
  in this function seems very low anyway.

Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

See the definition of kr_cdb_pt for details.

(Tiny nitpicks addressed by vcunat.)

Štěpán Balážik authored 4 years ago and Vladimír Čunát committed 4 years ago

The Internet has changed, turktel ones are fixed, edatel one does not
work at all.

release 5.1.3

See merge request !1059

cache-forking fixes

See merge request !1042

This change fixes race condition in assert_right_version(). Racy
situation:
- Two instances have the (empty) cache open: New binary and old binary.
- New binary executes count() inside assert_right_version(), which
  internally starts RO transaction. Returned count is 0.
- Old binary does some writes (RW transaction parallel to RO in the first
  process).
- New binary skips cache clear because cache was empty at the time of check.
- Result: The old binary wrote data with an old format into cache which
  was not cleared and silenty changed version number to a new one.

This is not complete fix because we lack mechanism to detect cache format
change at run-time, but at least it removes one nasty corner case and
cost of this change seems to be minimal.