You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2021/05/25 21:29:30 UTC
[kudu] branch master updated (0569c70 -> 725c8b7)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 0569c70 KUDU-1884: set sasl_protocol_name for the TxnSystemClient
new 8ffa181 [consensus] a small optimisation on log prefix
new 725c8b7 [tests] TsRecoveryITestDeathTest isn't for for TSAN buidls
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/consensus/pending_rounds.cc | 8 +++-----
src/kudu/consensus/pending_rounds.h | 8 +++++---
src/kudu/consensus/raft_consensus.cc | 10 ++--------
src/kudu/consensus/raft_consensus.h | 7 ++++++-
src/kudu/integration-tests/ts_recovery-itest.cc | 12 ++++++------
5 files changed, 22 insertions(+), 23 deletions(-)
[kudu] 02/02: [tests] TsRecoveryITestDeathTest isn't for for TSAN
buidls
Posted by al...@apache.org.
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
commit 725c8b728a4c9fd582114aa529d7f8fba186bc9d
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Sun May 23 14:42:00 2021 -0700
[tests] TsRecoveryITestDeathTest isn't for for TSAN buidls
Prior to this patch, the TestRecoverFromOpIdOverflow scenario of the
TsRecoveryITestDeathTest hasn't actually run for TSAN builds, but always
reported success. A proper way of dealing with such conditions is to
either skip a scenario or don't compile it at all as we do for others:
* TestCFileBothCacheMemoryTypes.TestReadWriteLargeStrings
* ThreadPoolTest.TestDeadlocks
* DebugUtilTest.TestUnwindWhileUnsafe
Change-Id: Id4ff9cd8943c780d4fe8bf6afb2ef540c07a5c8d
Reviewed-on: http://gerrit.cloudera.org:8080/17491
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/integration-tests/ts_recovery-itest.cc | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/kudu/integration-tests/ts_recovery-itest.cc b/src/kudu/integration-tests/ts_recovery-itest.cc
index 94ae9cc..35b93d8 100644
--- a/src/kudu/integration-tests/ts_recovery-itest.cc
+++ b/src/kudu/integration-tests/ts_recovery-itest.cc
@@ -547,18 +547,17 @@ TEST_P(TsRecoveryITest, TestChangeMaxCellSize) {
work.rows_inserted()));
}
-class TsRecoveryITestDeathTest : public TsRecoveryITest {};
+#if !defined(THREAD_SANITIZER)
+// TSAN cannot handle spawning threads after fork(), so this class and its
+// test scenario isn't present for TSAN builds.
+class TsRecoveryITestDeathTest : public TsRecoveryITest {
+};
// Test that tablet bootstrap can automatically repair itself if it finds an
// overflowed OpId index written to the log caused by KUDU-1933.
// Also serves as a regression itest for KUDU-1933 by writing ops with a high
// term and index.
TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) {
-#if defined(THREAD_SANITIZER)
- // TSAN cannot handle spawning threads after fork().
- return;
-#endif
-
// Create the initial tablet files on disk, then shut down the cluster so we
// can meddle with the WAL.
NO_FATALS(StartClusterOneTs());
@@ -696,6 +695,7 @@ TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) {
});
NO_FATALS(cluster_->AssertNoCrashes());
}
+#endif // #if !defined(THREAD_SANITIZER) ...
// A set of threads which pick rows which are known to exist in the table
// and issue random updates against them.
[kudu] 01/02: [consensus] a small optimisation on log prefix
Posted by al...@apache.org.
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
commit 8ffa18196dcd3917ae624f9dc9679040e65967cb
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Sun May 23 09:00:15 2021 -0700
[consensus] a small optimisation on log prefix
This patch introduces a small optimisation into the implementation of
the RaftConsensus::LogPrefixThreadSafe() method: since the prefix
it produces doesn't change, it's possible to construct the string once
in the constructor and then return a constant reference.
I also updated PendingRound class to store a reference to the prefix as
well since instances of the PendingRound class have shorter live span
than RaftConsensus objects which create PendingRound objects.
Change-Id: I6a5082fe07449ef24827696638ddd8e925a08264
Reviewed-on: http://gerrit.cloudera.org:8080/17489
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/consensus/pending_rounds.cc | 8 +++-----
src/kudu/consensus/pending_rounds.h | 8 +++++---
src/kudu/consensus/raft_consensus.cc | 10 ++--------
src/kudu/consensus/raft_consensus.h | 7 ++++++-
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/kudu/consensus/pending_rounds.cc b/src/kudu/consensus/pending_rounds.cc
index 9200b07..cd275dc 100644
--- a/src/kudu/consensus/pending_rounds.cc
+++ b/src/kudu/consensus/pending_rounds.cc
@@ -47,14 +47,12 @@ namespace consensus {
// PendingRounds
//------------------------------------------------------------
-PendingRounds::PendingRounds(string log_prefix, TimeManager* time_manager)
- : log_prefix_(std::move(log_prefix)),
+PendingRounds::PendingRounds(const string& log_prefix,
+ TimeManager* time_manager)
+ : log_prefix_(log_prefix),
last_committed_op_id_(MinimumOpId()),
time_manager_(time_manager) {}
-PendingRounds::~PendingRounds() {
-}
-
Status PendingRounds::CancelPendingOps() {
ThreadRestrictions::AssertWaitAllowed();
if (pending_ops_.empty()) {
diff --git a/src/kudu/consensus/pending_rounds.h b/src/kudu/consensus/pending_rounds.h
index 742dbeb..530db2f 100644
--- a/src/kudu/consensus/pending_rounds.h
+++ b/src/kudu/consensus/pending_rounds.h
@@ -42,8 +42,8 @@ class TimeManager;
// operations.
class PendingRounds {
public:
- PendingRounds(std::string log_prefix, TimeManager* time_manager);
- ~PendingRounds();
+ PendingRounds(const std::string& log_prefix, TimeManager* time_manager);
+ ~PendingRounds() = default;
// Set the committed op during startup. This should be done after appending
// any of the pending ops, and will take care of triggering any that are now
@@ -99,7 +99,9 @@ class PendingRounds {
private:
const std::string& LogPrefix() const { return log_prefix_; }
- const std::string log_prefix_;
+ // Storing just a reference here: the RaftConsensus class which instantiates
+ // a PendingRound object keeps the ownership of the referenced string.
+ const std::string& log_prefix_;
// Index=>Round map that manages pending ops, i.e. operations for which we've
// received a replicate message from the leader but have yet to be committed.
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 0daf78a..7849c6d 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -183,6 +183,7 @@ RaftConsensus::RaftConsensus(
ServerContext server_ctx)
: options_(std::move(options)),
local_peer_pb_(std::move(local_peer_pb)),
+ log_prefix_(Substitute("T $0 P $1: ", options_.tablet_id, peer_uuid())),
cmeta_manager_(DCHECK_NOTNULL(std::move(cmeta_manager))),
server_ctx_(std::move(server_ctx)),
state_(kNew),
@@ -289,7 +290,6 @@ Status RaftConsensus::Start(const ConsensusBootstrapInfo& info,
queue.get(),
raft_pool_token_.get(),
log_));
-
unique_ptr<PendingRounds> pending(new PendingRounds(
LogPrefixThreadSafe(), time_manager_.get()));
@@ -2662,7 +2662,7 @@ void RaftConsensus::ElectionCallback(ElectionReason reason, const ElectionResult
auto self = shared_from_this();
Status s = raft_pool_token_->Submit([=]() { self->DoElectionCallback(reason, result); });
if (!s.ok()) {
- static const char* msg = "unable to run election callback";
+ static const char* const msg = "unable to run election callback";
CHECK(s.IsServiceUnavailable()) << LogPrefixThreadSafe() << msg;
WARN_NOT_OK(s, LogPrefixThreadSafe() + msg);
}
@@ -3226,12 +3226,6 @@ string RaftConsensus::LogPrefixUnlocked() const {
return Substitute("T $0 P $1$2: ", options_.tablet_id, peer_uuid(), cmeta_info);
}
-string RaftConsensus::LogPrefixThreadSafe() const {
- return Substitute("T $0 P $1: ",
- options_.tablet_id,
- peer_uuid());
-}
-
string RaftConsensus::ToString() const {
ThreadRestrictions::AssertWaitAllowed();
LockGuard l(lock_);
diff --git a/src/kudu/consensus/raft_consensus.h b/src/kudu/consensus/raft_consensus.h
index 743789e..adb0bad 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -838,7 +838,9 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
// A variant of LogPrefix which does not take the lock. This is a slightly
// less thorough prefix which only includes immutable (and thus thread-safe)
// information, but does not require the lock.
- std::string LogPrefixThreadSafe() const;
+ const std::string& LogPrefixThreadSafe() const {
+ return log_prefix_;
+ }
std::string ToString() const;
std::string ToStringUnlocked() const;
@@ -850,6 +852,9 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
// Information about the local peer, including the local UUID.
const RaftPeerPB local_peer_pb_;
+ // Log prefix for this peer.
+ const std::string log_prefix_;
+
// Consensus metadata service.
const scoped_refptr<ConsensusMetadataManager> cmeta_manager_;