(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.

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

This gives us correctness, especially on "staleness" detection.
For simplicity we now don't remove "stale" .cachelock on opening cache,
but it doesn't obstruct us in any way (and overflow will remove it).

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

- The switched order is documented not to make difference,
  but it seems much clearer this way.
- MDB_TXN_FULL wasn't handled correctly (a reversed condition)
  and current LMDB code indicates that such transaction is
  not recoverable anyway... so we give up on trying.

(This has shared authorship, basically, mostly from MR suggestions.)

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

This version seems to work OK.  Unfortunately we had to resort to
an extra write and cache reopening when attempting to set cache size.
And even so, decreasing the size can't really be done, so we only warn
about failing to do that.

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

For the usual use cases, whole milliseconds seem to make more sense
than seconds with 10ms precision.

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

TL;DR: tune the test - now it works quite reliably for me,
though it's perhaps not nice.

With 1 MiB cache it's not easy to avoid overflows, as the defaults are
meant for much larger sizes.  Normal GC target is to decrease usage
by 10% when above 80% in 100 records per transaction.  That just won't
work reliable due to 10% being only 25 pages.

This commit makes the test run GC with more suitable tuning and
frequently pauses kresd to give GC better chance to catch up.

GC should prevent cache from overflowing.

Resolvers must answer queries even if the shared cache overflown during query processing.

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

... in case of usage from kresd (GC does it a bit differently).

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

This is important for GC - otherwise the usage computation would be
wrong after another process changed size (without replacing the file).

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

As the code has been so far, there's no usable cache in that case
and some code just can't handle that.  Up to now we were getting
SIGSEGV from inside LMDB on the next attempted operation.

We might consider loosening preallocation in that case or even
retrying after a short sleep.  Systemd's restart after hold-off
timeout has an effect similar to the short sleep.

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

In the unlikely case that GC happens "too late", it could fail when
deleting, in which case it seems best to reopen the cache and try again,
as it will probably be deleted by a kresd instance by the next interval.

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

Until now the analyzing pass over full DB was taking place
in a single RO transaction.  For an unknown reason this caused kresd
processes to get MDB_MAP_FULL from mdb_put(), even though clearly there
were plenty free pages at that point.

Basic experiments show that 1k steps are OK and 10k steps are not.

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

though I've never seen it happening.

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

This apparently gets rid of MDB_BAD_TXN failures that we were getting
when cache overflows.  Unfortunately LMDB docs don't mention that
after operation failures one should abort the corresponding transaction.

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

Now it should keep working if the file has been replaced.

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

FIXME: review, testing, etc.

A couple functions got folded into cdb_open_env(), as the split was
complicating situation (mainly around error handling).

doc: upcoming changes

See merge request !1057

scripts, docs: specify lua version in `luarocks install`

Closes #601

See merge request !1052

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

On some systems luarocks defaults to other lua version (e.g. Fedora),
so the result would not be usable from kresd.  I didn't touch scripts
for older distro versions (Debian < 10, Ubuntu < 20.04, CentOS 7).

lib/dnssec: fix build against libdnssec 3.0

See merge request !1053

It hasn't been released yet, but this patch fixes build against
current Knot master already.