This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 368225e87 KUDU-3535 Clear log cache while tombstoning a tablet replica.
368225e87 is described below
commit 368225e87f77851f8cdf98fc4b7670aaac7a773e
Author: 宋家成 <[email protected]>
AuthorDate: Wed Dec 20 14:57:01 2023 +0800
KUDU-3535 Clear log cache while tombstoning a tablet replica.
The log cache of a replica still exists even if the replica has been
already tombstoned. This problem might take place if we decrease
the replication factor of a table with high throughput.
So we should clear the log cache while deleting the replica with
delete type TABLET_DATA_TOMBSTONED.
Change-Id: I6cf545e604f80d41e7ebd9660acfd2e928cd27a9
Reviewed-on: http://gerrit.cloudera.org:8080/20822
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
src/kudu/consensus/consensus_queue-test.cc | 13 +++++++++++++
src/kudu/consensus/consensus_queue.cc | 8 +++++---
src/kudu/consensus/consensus_queue.h | 2 ++
src/kudu/consensus/log_cache.cc | 11 ++++++++---
src/kudu/consensus/log_cache.h | 3 +++
5 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/src/kudu/consensus/consensus_queue-test.cc
b/src/kudu/consensus/consensus_queue-test.cc
index e219b651e..d21a37783 100644
--- a/src/kudu/consensus/consensus_queue-test.cc
+++ b/src/kudu/consensus/consensus_queue-test.cc
@@ -44,6 +44,7 @@
#include "kudu/consensus/log-test-base.h"
#include "kudu/consensus/log.h"
#include "kudu/consensus/log_anchor_registry.h"
+#include "kudu/consensus/log_cache.h"
#include "kudu/consensus/log_util.h"
#include "kudu/consensus/metadata.pb.h"
#include "kudu/consensus/opid.pb.h"
@@ -1189,5 +1190,17 @@ TEST(ConsensusQueueUnitTest, PeerHealthStatus) {
EXPECT_EQ(HealthReportPB::FAILED_UNRECOVERABLE,
PeerMessageQueue::PeerHealthStatus(peer));
}
+// Test the log cache will be cleared if Close() is called. The method is
called while
+// tombstoning a replica, so the log cache will be cleared if a replica is
tombstoned.
+TEST_F(ConsensusQueueTest, ClearLogCacheWhileClosing) {
+ queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm,
BuildRaftConfigPBForTests(3));
+ AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 100);
+
+ ASSERT_LT(0, queue_->log_cache_.BytesUsed());
+ queue_->Close();
+
+ ASSERT_EQ(0, queue_->log_cache_.BytesUsed());
+}
+
} // namespace consensus
} // namespace kudu
diff --git a/src/kudu/consensus/consensus_queue.cc
b/src/kudu/consensus/consensus_queue.cc
index bda268183..86a7a3eda 100644
--- a/src/kudu/consensus/consensus_queue.cc
+++ b/src/kudu/consensus/consensus_queue.cc
@@ -1428,9 +1428,11 @@ void PeerMessageQueue::ClearUnlocked() {
void PeerMessageQueue::Close() {
raft_pool_observers_token_->Shutdown();
-
- std::lock_guard<simple_spinlock> lock(queue_lock_);
- ClearUnlocked();
+ {
+ std::lock_guard<simple_spinlock> lock(queue_lock_);
+ ClearUnlocked();
+ }
+ log_cache_.Clear();
}
int64_t PeerMessageQueue::GetQueuedOperationsSizeBytesForTests() const {
diff --git a/src/kudu/consensus/consensus_queue.h
b/src/kudu/consensus/consensus_queue.h
index 68cae6351..c20429dcc 100644
--- a/src/kudu/consensus/consensus_queue.h
+++ b/src/kudu/consensus/consensus_queue.h
@@ -371,6 +371,7 @@ class PeerMessageQueue {
void EndWatchForSuccessor();
private:
+ FRIEND_TEST(ConsensusQueueTest, ClearLogCacheWhileClosing);
FRIEND_TEST(ConsensusQueueTest, TestQueueAdvancesCommittedIndex);
FRIEND_TEST(ConsensusQueueTest, TestQueueMovesWatermarksBackward);
FRIEND_TEST(ConsensusQueueTest, TestFollowerCommittedIndexAndMetrics);
@@ -378,6 +379,7 @@ class PeerMessageQueue {
FRIEND_TEST(ConsensusQueueUnitTest, PeerHealthStatus);
FRIEND_TEST(RaftConsensusQuorumTest,
TestReplicasEnforceTheLogMatchingProperty);
+
// Mode specifies how the queue currently behaves:
// LEADER - Means the queue tracks remote peers and replicates whatever
messages
// are appended. Observers are notified of changes.
diff --git a/src/kudu/consensus/log_cache.cc b/src/kudu/consensus/log_cache.cc
index 24281cf23..03f67fc7a 100644
--- a/src/kudu/consensus/log_cache.cc
+++ b/src/kudu/consensus/log_cache.cc
@@ -22,6 +22,7 @@
#include <mutex>
#include <ostream>
#include <string>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -31,7 +32,7 @@
#include "kudu/consensus/consensus.pb.h"
#include "kudu/consensus/log.h"
-#include "kudu/consensus/log_reader.h"
+#include "kudu/consensus/log_reader.h" // IWYU pragma: keep
#include "kudu/consensus/opid.pb.h"
#include "kudu/consensus/opid_util.h"
#include "kudu/consensus/ref_counted_replicate.h"
@@ -113,8 +114,7 @@ LogCache::LogCache(const scoped_refptr<MetricEntity>&
metric_entity,
}
LogCache::~LogCache() {
- tracker_->Release(tracker_->consumption());
- cache_.clear();
+ Clear();
}
void LogCache::Init(const OpId& preceding_op) {
@@ -477,6 +477,11 @@ void LogCache::DumpToHtml(std::ostream& out) const {
out << "</table>";
}
+void LogCache::Clear() {
+ tracker_->Release(tracker_->consumption());
+ cache_.clear();
+}
+
#define INSTANTIATE_METRIC(x) \
x.Instantiate(metric_entity, 0)
LogCache::Metrics::Metrics(const scoped_refptr<MetricEntity>& metric_entity)
diff --git a/src/kudu/consensus/log_cache.h b/src/kudu/consensus/log_cache.h
index 812f50ec2..c80b32cea 100644
--- a/src/kudu/consensus/log_cache.h
+++ b/src/kudu/consensus/log_cache.h
@@ -139,6 +139,9 @@ class LogCache {
// Returns another bad Status if the log index fails to load (eg. due to an
IO error).
Status LookupOpId(int64_t op_index, OpId* op_id) const;
+ // Clear the log cache.
+ void Clear();
+
private:
FRIEND_TEST(LogCacheTest, TestAppendAndGetMessages);
FRIEND_TEST(LogCacheTest, TestGlobalMemoryLimit);
