Skip to content

Commit

Permalink
Log specialization should return whether a trim actually happened or it
Browse files Browse the repository at this point in the history
was a no-op

Summary: This is valuable, because the Raft Application (e.g. Mysql Raft
plugin) can call TruncateCallbackWithRaftLock by passing in -1 to  do
the postponed trimming. However it also benefits from the knowledge that
actual trimming took place. If trim happens certain side-effects will
have to be taken care of like restarting state machine appliers to get
rid of cached transactions.

Test Plan: The testing happened via Dead Primary Promotion tests.
With and Without we were able to mitigate a dataloss situation in which
the Dead Primary (instance which was erstwhile primary and stepped
down and then had a trim), had some transactions cached in its applier
cache waiting for confirmation from Raft. Without removing the entries
from the cache, extra rows were present in the database.

Reviewers: vinaybhat, abhinavsharma, yashtc

Subscribers:

Tasks:

Tags:
  • Loading branch information
anirbanr-fb committed Mar 2, 2021
1 parent 6cf6d37 commit cff169f
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/kudu/consensus/log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ Status Log::WaitUntilAllFlushed() {
return s.Wait();
}

Status Log::TruncateOpsAfter(int64_t index) {
Status Log::TruncateOpsAfter(int64_t index, int64_t *index_if_truncated) {
// In base implementation, truncation is not needed
// as next_sequential_op_index_ is updated, as an alternative
// to actual trimming
Expand Down
4 changes: 3 additions & 1 deletion src/kudu/consensus/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ class Log : public RefCountedThreadSafe<Log> {
// are flushed and fsynced (if fsync of log entries is enabled).
virtual Status WaitUntilAllFlushed();

virtual Status TruncateOpsAfter(int64_t index);
// index_if_truncated - if caller e.g. Log Cache passes in index_if_truncated,
// the log specialization is expected to return the index of truncation
virtual Status TruncateOpsAfter(int64_t index, int64_t *index_if_truncated = nullptr);

// Kick off an asynchronous task that pre-allocates a new
// log-segment, setting 'allocation_status_'. To wait for the
Expand Down
4 changes: 2 additions & 2 deletions src/kudu/consensus/raft_consensus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ Status RaftConsensus::Replicate(const scoped_refptr<ConsensusRound>& round) {
return Status::OK();
}

Status RaftConsensus::TruncateCallbackWithRaftLock() {
Status RaftConsensus::TruncateCallbackWithRaftLock(int64_t *index_if_truncated) {
DCHECK(FLAGS_raft_derived_log_mode);
ThreadRestrictions::AssertWaitAllowed();
LockGuard l(lock_);
Expand All @@ -914,7 +914,7 @@ Status RaftConsensus::TruncateCallbackWithRaftLock() {
// We pass -1 to TruncateOpsAfter in the log abstraction
// It is the responsibility of the derived log to truncate from
// the cached truncation index and clear it.
return log_->TruncateOpsAfter(-1);
return log_->TruncateOpsAfter(-1, index_if_truncated);
}

Status RaftConsensus::CheckLeadershipAndBindTerm(const scoped_refptr<ConsensusRound>& round) {
Expand Down
4 changes: 3 additions & 1 deletion src/kudu/consensus/raft_consensus.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,9 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
// while holding Raft Consensus lock. This is to serialize
// the operation with Raft Consensus lock, which is the same locking
// pattern that is used by UpdateReplica while invoking TruncateOpsAfter
Status TruncateCallbackWithRaftLock();
// @param index_if_truncated - the log specialization will return the
// truncated index if truncation happened.
Status TruncateCallbackWithRaftLock(int64_t *index_if_truncated);

// Returns the last OpId (either received or committed, depending on the
// 'type' argument) that the Consensus implementation knows about.
Expand Down

0 comments on commit cff169f

Please sign in to comment.