From 3b082e42b8b33253c22b1f4f95809983edd6af89 Mon Sep 17 00:00:00 2001 From: adiholden Date: Thu, 2 Jan 2025 09:39:31 +0200 Subject: [PATCH] fix(replication): do not log to journal on callback fail (#4392) fix replication: do not log to journal on callback fail Signed-off-by: adi_holden --- src/server/transaction.cc | 10 +++++++--- src/server/transaction.h | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/server/transaction.cc b/src/server/transaction.cc index 3e180f871..36372a2a4 100644 --- a/src/server/transaction.cc +++ b/src/server/transaction.cc @@ -704,7 +704,7 @@ void Transaction::RunCallback(EngineShard* shard) { // Log to journal only once the command finished running if ((coordinator_state_ & COORD_CONCLUDING) || (multi_ && multi_->concluding)) { - LogAutoJournalOnShard(shard); + LogAutoJournalOnShard(shard, result); MaybeInvokeTrackingCb(); } } @@ -1346,7 +1346,7 @@ OpStatus Transaction::RunSquashedMultiCb(RunnableType cb) { auto result = cb(this, shard); db_slice.OnCbFinish(); - LogAutoJournalOnShard(shard); + LogAutoJournalOnShard(shard, result); MaybeInvokeTrackingCb(); DCHECK_EQ(result.flags, 0); // if it's sophisticated, we shouldn't squash it @@ -1438,7 +1438,7 @@ optional Transaction::GetWakeKey(ShardId sid) const { return ArgS(full_args_, sd.wake_key_pos); } -void Transaction::LogAutoJournalOnShard(EngineShard* shard) { +void Transaction::LogAutoJournalOnShard(EngineShard* shard, RunnableResult result) { // TODO: For now, we ignore non shard coordination. if (shard == nullptr) return; @@ -1455,6 +1455,10 @@ void Transaction::LogAutoJournalOnShard(EngineShard* shard) { if (journal == nullptr) return; + if (result.status != OpStatus::OK) { + return; // Do not log to journal if command execution failed. + } + // If autojournaling was disabled and not re-enabled the callback is writing to journal. if ((cid_->opt_mask() & CO::NO_AUTOJOURNAL) && !re_enabled_auto_journal_) { return; diff --git a/src/server/transaction.h b/src/server/transaction.h index c0eb37ff8..5e209eb96 100644 --- a/src/server/transaction.h +++ b/src/server/transaction.h @@ -559,7 +559,7 @@ class Transaction { // Log command in shard's journal, if this is a write command with auto-journaling enabled. // Should be called immediately after the last hop. - void LogAutoJournalOnShard(EngineShard* shard); + void LogAutoJournalOnShard(EngineShard* shard, RunnableResult shard_result); // Whether the callback can be run directly on this thread without dispatching on the shard queue bool CanRunInlined() const;