There was also a bug in storage::Plugins::get_all_versions.

Close #1283

We used to automatically truncate the index in compact_log if the caller
requested to compact too many entries. This made it so that the
requirement of not compacting any un-applied entries was implicit in our
code base, which is not good as it allows for some bugs to creep in
(like the one we fix a couple commits ago).

Now this is changed and instead of silently adjusting the index of last
compacted entry, we just assert that it's no greater than the applied
index. As a consequence there's a minor improvement in do_raft_log_auto_compaction
function.

There was a hard-to-reproduce bug in our snapshot application code.
We always compact the raft log before applying the snapshot, because
the snapshot replaces the entries and some of the logic in raft-rs
seems to rely on this. The problem was, that our compact_log function
would not remove any unapplied entries, which makes sense for compaction
triggered automatically by raft log size, but doesn't make sense for
raft snapshot, as the snapshot contains the state corresponding to the
newer entries. The fix is simple: don't guard from unapplied entry
compaction in case the compaction is for raft snapshot.

We don't add any regression tests for this, because the implementation
would be too difficult and would need us to pollute the code with error
injection logic, which is not a worthy trade off in this case. But also
the logic will still be tested, because this bug was responsible for a
large amount of flaky tests, so we should see a significant reduction in
flakiness from now on in tests concerning raft snapshots.

godzie44 authored 6 months ago and Georgy Moshkin committed 6 months ago

Closes #664

Move vshard related fields from _pico_property to _pico_tier
and adapt it in governor algorithm

We only use Eq ranges

CAS predicates should always contain the current raft term, because this
is what's explicitly checked in proc_cas.

Note that this will sometimes result in a EntryTermMismatch error
because the latest applied entry may have a different term in case the
election has started but not finished yet.

We could add this check to all the client cas precondition checks, but
it's not a big deal, because we handle the situation correctly anyways.