Skip to content

Commit

Permalink
r/consensus: do not advance last visible offset beyond majority repli…
Browse files Browse the repository at this point in the history
…cated

When Raft handles replicate requests of mixed consistency levels it has
to separately track the visibility of both type of replicate requests.
i.e. `quorum_ack` requests should not be visible to readers until they
are successfully replicated and flushed on the majority of nodes while
for `leader_ack` requests it is enough to have them acknowledged and not
flushed by majority of nodes. When `quorum_ack` batch is committed then
all subsequent majority replicated `leader_ack` batches may become
immediately visible to consumers if they are majority confirmed. Fixed
bug in Raft where the majority replication wasn't validated correctly
and 'dirty' batches were made visible to the consumer. It might lead to
situation in which consumer read batches which were then not available
on newly elected leader.

Signed-off-by: Michal Maslanka <michal@redpanda.com>
  • Loading branch information
mmaslankaprv committed Nov 7, 2023
1 parent 785d6a9 commit 09917c7
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/v/raft/consensus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2904,7 +2904,6 @@ ss::future<> consensus::maybe_commit_configuration(ssx::semaphore_units u) {

ss::future<>
consensus::do_maybe_update_leader_commit_idx(ssx::semaphore_units u) {
auto lstats = _log->offsets();
// Raft paper:
//
// If there exists an N such that N > commitIndex, a majority
Expand Down Expand Up @@ -2947,7 +2946,7 @@ consensus::do_maybe_update_leader_commit_idx(ssx::semaphore_units u) {
// if we successfully acknowledged all quorum writes we can make pending
// relaxed consistency requests visible
if (_commit_index >= _last_quorum_replicated_index) {
maybe_update_last_visible_index(lstats.dirty_offset);
maybe_update_last_visible_index(_majority_replicated_index);
} else {
// still have to wait for some quorum consistency requests to be
// committed
Expand Down

0 comments on commit 09917c7

Please sign in to comment.