This is an automated email from the ASF dual-hosted git repository. adar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit e11f0151a2d22d2658995e27c8dcde4d3441f561 Author: Adar Dembo <[email protected]> AuthorDate: Thu Jan 9 17:17:51 2020 -0800 clock: remove shared ownership I don't think this was ever necessary as the Clock instance is always guaranteed to be in scope (as it is part of the server itself). Along the way I modernized some stuff. Change-Id: I3e75a539c5de1c367d05784f544b96197249ec49 Reviewed-on: http://gerrit.cloudera.org:8080/15005 Reviewed-by: Alexey Serbin <[email protected]> Reviewed-by: Andrew Wong <[email protected]> Tested-by: Kudu Jenkins --- src/kudu/clock/clock.h | 2 +- src/kudu/clock/hybrid_clock-test.cc | 9 +-- src/kudu/clock/logical_clock-test.cc | 7 ++- src/kudu/clock/logical_clock.cc | 12 ++-- src/kudu/clock/logical_clock.h | 3 +- src/kudu/common/timestamp.h | 6 +- src/kudu/consensus/consensus-test-util.h | 4 +- src/kudu/consensus/consensus_peers-test.cc | 20 +++---- src/kudu/consensus/consensus_queue-test.cc | 30 +++++----- src/kudu/consensus/consensus_queue.h | 6 +- src/kudu/consensus/log-test-base.h | 17 +++--- src/kudu/consensus/log-test.cc | 4 +- src/kudu/consensus/log_cache-test.cc | 11 ++-- src/kudu/consensus/raft_consensus_quorum-test.cc | 4 +- src/kudu/consensus/time_manager-test.cc | 18 +++--- src/kudu/consensus/time_manager.cc | 5 +- src/kudu/consensus/time_manager.h | 8 ++- src/kudu/integration-tests/fuzz-itest.cc | 3 +- .../integration-tests/tablet_history_gc-itest.cc | 11 ++-- src/kudu/integration-tests/ts_recovery-itest.cc | 65 +++++++++++----------- src/kudu/master/sys_catalog.cc | 5 +- src/kudu/server/server_base.cc | 2 +- src/kudu/server/server_base.h | 9 ++- src/kudu/tablet/compaction-test.cc | 3 +- src/kudu/tablet/deltamemstore-test.cc | 4 +- src/kudu/tablet/diskrowset-test-base.h | 7 +-- src/kudu/tablet/diskrowset-test.cc | 4 +- src/kudu/tablet/memrowset-test.cc | 4 +- src/kudu/tablet/mvcc-test.cc | 16 +++--- src/kudu/tablet/tablet-harness.h | 10 ++-- src/kudu/tablet/tablet-test-util.h | 1 - src/kudu/tablet/tablet.cc | 5 +- src/kudu/tablet/tablet.h | 14 +++-- src/kudu/tablet/tablet_bootstrap-test.cc | 33 +++++------ src/kudu/tablet/tablet_bootstrap.cc | 14 ++--- src/kudu/tablet/tablet_bootstrap.h | 6 +- src/kudu/tablet/tablet_replica.cc | 5 +- src/kudu/tablet/tablet_replica.h | 9 +-- src/kudu/tools/tool_action_perf.cc | 5 +- src/kudu/tserver/ts_tablet_manager.cc | 5 +- 40 files changed, 195 insertions(+), 211 deletions(-) diff --git a/src/kudu/clock/clock.h b/src/kudu/clock/clock.h index e9a0404..79ac84e 100644 --- a/src/kudu/clock/clock.h +++ b/src/kudu/clock/clock.h @@ -40,7 +40,7 @@ namespace clock { // i.e. for any two calls, i.e. Now returns timestamp1 and timestamp2, it must // hold that timestamp1 < timestamp2. // 2 - Update() must never set the clock backwards (corollary of 1) -class Clock : public RefCountedThreadSafe<Clock> { +class Clock { public: virtual ~Clock() = default; diff --git a/src/kudu/clock/hybrid_clock-test.cc b/src/kudu/clock/hybrid_clock-test.cc index d5e5641..3873b59 100644 --- a/src/kudu/clock/hybrid_clock-test.cc +++ b/src/kudu/clock/hybrid_clock-test.cc @@ -19,11 +19,11 @@ #include <algorithm> #include <cstdint> +#include <memory> #include <string> #include <vector> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <gtest/gtest.h> @@ -48,6 +48,7 @@ DECLARE_bool(inject_unsync_time_errors); DECLARE_string(time_source); using std::string; +using std::unique_ptr; using std::vector; namespace kudu { @@ -65,7 +66,7 @@ class HybridClockTest : public KuduTest { } protected: - scoped_refptr<HybridClock> clock_; + unique_ptr<HybridClock> clock_; }; clock::MockNtp* mock_ntp(HybridClock* clock) { @@ -75,7 +76,7 @@ clock::MockNtp* mock_ntp(HybridClock* clock) { TEST(MockHybridClockTest, TestMockedSystemClock) { google::FlagSaver saver; FLAGS_time_source = "mock"; - scoped_refptr<HybridClock> clock(new HybridClock); + unique_ptr<HybridClock> clock(new HybridClock); ASSERT_OK(clock->Init()); Timestamp timestamp; uint64_t max_error_usec; @@ -109,7 +110,7 @@ TEST(MockHybridClockTest, TestMockedSystemClock) { TEST(MockHybridClockTest, TestClockDealsWithWrapping) { google::FlagSaver saver; FLAGS_time_source = "mock"; - scoped_refptr<HybridClock> clock(new HybridClock); + unique_ptr<HybridClock> clock(new HybridClock); ASSERT_OK(clock->Init()); mock_ntp(clock.get())->SetMockClockWallTimeForTests(1000); diff --git a/src/kudu/clock/logical_clock-test.cc b/src/kudu/clock/logical_clock-test.cc index 2dcad70..a6b5cb4 100644 --- a/src/kudu/clock/logical_clock-test.cc +++ b/src/kudu/clock/logical_clock-test.cc @@ -15,16 +15,19 @@ // specific language governing permissions and limitations // under the License. +#include <memory> + #include <gtest/gtest.h> #include "kudu/clock/logical_clock.h" #include "kudu/common/timestamp.h" -#include "kudu/gutil/ref_counted.h" #include "kudu/util/monotime.h" #include "kudu/util/status.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" +using std::unique_ptr; + namespace kudu { namespace clock { @@ -35,7 +38,7 @@ class LogicalClockTest : public KuduTest { } protected: - scoped_refptr<LogicalClock> clock_; + unique_ptr<LogicalClock> clock_; }; // Test that two subsequent time reads are monotonically increasing. diff --git a/src/kudu/clock/logical_clock.cc b/src/kudu/clock/logical_clock.cc index 4e81bcf..6fa861e 100644 --- a/src/kudu/clock/logical_clock.cc +++ b/src/kudu/clock/logical_clock.cc @@ -17,6 +17,7 @@ #include "kudu/clock/logical_clock.h" +#include <memory> #include <ostream> #include <string> @@ -30,9 +31,6 @@ #include "kudu/util/metrics.h" #include "kudu/util/status.h" -namespace kudu { -namespace clock { - METRIC_DEFINE_gauge_uint64(server, logical_clock_timestamp, "Logical Clock Timestamp", kudu::MetricUnit::kUnits, @@ -43,6 +41,10 @@ using base::subtle::Atomic64; using base::subtle::Barrier_AtomicIncrement; using base::subtle::NoBarrier_CompareAndSwap; using base::subtle::NoBarrier_Load; +using std::unique_ptr; + +namespace kudu { +namespace clock { Timestamp LogicalClock::Now() { return Timestamp(Barrier_AtomicIncrement(&now_, 1)); @@ -88,9 +90,9 @@ bool LogicalClock::IsAfter(Timestamp t) { return base::subtle::Acquire_Load(&now_) >= t.value(); } -LogicalClock* LogicalClock::CreateStartingAt(const Timestamp& timestamp) { +unique_ptr<LogicalClock> LogicalClock::CreateStartingAt(const Timestamp& timestamp) { // initialize at 'timestamp' - 1 so that the first output value is 'timestamp'. - return new LogicalClock(timestamp.value() - 1); + return unique_ptr<LogicalClock>(new LogicalClock(timestamp.value() - 1)); } uint64_t LogicalClock::GetCurrentTime() { diff --git a/src/kudu/clock/logical_clock.h b/src/kudu/clock/logical_clock.h index 091f617..0a74a51 100644 --- a/src/kudu/clock/logical_clock.h +++ b/src/kudu/clock/logical_clock.h @@ -17,6 +17,7 @@ #pragma once #include <cstdint> +#include <memory> #include <string> #include "kudu/clock/clock.h" @@ -78,7 +79,7 @@ class LogicalClock : public Clock { } // Creates a logical clock whose first output value on a Now() call is 'timestamp'. - static LogicalClock* CreateStartingAt(const Timestamp& timestamp); + static std::unique_ptr<LogicalClock> CreateStartingAt(const Timestamp& timestamp); private: // Should use LogicalClock::CreatingStartingAt() diff --git a/src/kudu/common/timestamp.h b/src/kudu/common/timestamp.h index 7777b64..ac2cfa7 100644 --- a/src/kudu/common/timestamp.h +++ b/src/kudu/common/timestamp.h @@ -14,9 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - -#ifndef KUDU_COMMON_TIMESTAMP_H_ -#define KUDU_COMMON_TIMESTAMP_H_ +#pragma once #include <cstdint> #include <iosfwd> @@ -137,5 +135,3 @@ inline bool operator>=(const Timestamp& lhs, const Timestamp& rhs) { } } // namespace kudu - -#endif /* KUDU_COMMON_TIMESTAMP_H_ */ diff --git a/src/kudu/consensus/consensus-test-util.h b/src/kudu/consensus/consensus-test-util.h index ff5aa87..a315028 100644 --- a/src/kudu/consensus/consensus-test-util.h +++ b/src/kudu/consensus/consensus-test-util.h @@ -14,6 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. +#pragma once #include <functional> #include <map> @@ -95,7 +96,7 @@ inline RaftPeerPB FakeRaftPeerPB(const std::string& uuid) { // TestOperationStatus::AckPeer(). inline void AppendReplicateMessagesToQueue( PeerMessageQueue* queue, - const scoped_refptr<clock::Clock>& clock, + clock::Clock* clock, int64_t first, int64_t count, int64_t payload_size = 0) { @@ -741,4 +742,3 @@ class TestTransactionFactory : public ConsensusRoundHandler { } // namespace consensus } // namespace kudu - diff --git a/src/kudu/consensus/consensus_peers-test.cc b/src/kudu/consensus/consensus_peers-test.cc index 08797c6..4a380b8 100644 --- a/src/kudu/consensus/consensus_peers-test.cc +++ b/src/kudu/consensus/consensus_peers-test.cc @@ -94,7 +94,7 @@ class ConsensusPeersTest : public KuduTest { clock_.reset(new clock::HybridClock()); ASSERT_OK(clock_->Init()); - scoped_refptr<TimeManager> time_manager(new TimeManager(clock_, Timestamp::kMin)); + scoped_refptr<TimeManager> time_manager(new TimeManager(clock_.get(), Timestamp::kMin)); message_queue_.reset(new PeerMessageQueue( metric_entity_, @@ -163,14 +163,14 @@ class ConsensusPeersTest : public KuduTest { protected: MetricRegistry metric_registry_; scoped_refptr<MetricEntity> metric_entity_; - gscoped_ptr<FsManager> fs_manager_; + unique_ptr<FsManager> fs_manager_; scoped_refptr<Log> log_; unique_ptr<ThreadPool> raft_pool_; - gscoped_ptr<PeerMessageQueue> message_queue_; + unique_ptr<PeerMessageQueue> message_queue_; const Schema schema_; LogOptions options_; unique_ptr<ThreadPoolToken> raft_pool_token_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; shared_ptr<Messenger> messenger_; }; @@ -192,7 +192,7 @@ TEST_F(ConsensusPeersTest, TestRemotePeer) { NewRemotePeer(kFollowerUuid, &remote_peer); // Append a bunch of messages to the queue - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, 1, 20); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), 1, 20); // signal the peer there are requests pending. remote_peer->SignalRequest(); @@ -223,7 +223,7 @@ TEST_F(ConsensusPeersTest, TestRemotePeers) { remote_peer2_proxy->DelayResponse(); // Append one message to the queue. - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, 1, 1); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), 1, 1); OpId first = MakeOpId(0, 1); @@ -247,7 +247,7 @@ TEST_F(ConsensusPeersTest, TestRemotePeers) { } // Now append another message to the queue - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, 2, 1); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), 2, 1); // We should not see it committed, even after 10ms, // since only the local peer replicates the message. @@ -294,7 +294,7 @@ TEST_F(ConsensusPeersTest, TestCloseWhenRemotePeerDoesntMakeProgress) { mock_proxy->set_update_response(peer_resp); // Add an op to the queue and start sending requests to the peer. - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, 1, 1); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), 1, 1); peer->SignalRequest(true); // We should be able to close the peer even though it has more data pending. @@ -332,7 +332,7 @@ TEST_F(ConsensusPeersTest, TestDontSendOneRpcPerWriteWhenPeerIsDown) { initial_resp.mutable_status()->set_last_committed_idx(1); mock_proxy->set_update_response(initial_resp); - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, 1, 1); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), 1, 1); peer->SignalRequest(true); // Now wait for the message to be replicated, this should succeed since @@ -347,7 +347,7 @@ TEST_F(ConsensusPeersTest, TestDontSendOneRpcPerWriteWhenPeerIsDown) { // Add a bunch of messages to the queue. for (int i = 2; i <= 100; i++) { - AppendReplicateMessagesToQueue(message_queue_.get(), clock_, i, 1); + AppendReplicateMessagesToQueue(message_queue_.get(), clock_.get(), i, 1); peer->SignalRequest(false); SleepFor(MonoDelta::FromMilliseconds(2)); } diff --git a/src/kudu/consensus/consensus_queue-test.cc b/src/kudu/consensus/consensus_queue-test.cc index 7bb7a22..f6e6c1a 100644 --- a/src/kudu/consensus/consensus_queue-test.cc +++ b/src/kudu/consensus/consensus_queue-test.cc @@ -101,9 +101,7 @@ class ConsensusQueueTest : public KuduTest { } void CloseAndReopenQueue(const OpId& replicated_opid, const OpId& committed_opid) { - scoped_refptr<clock::Clock> clock(new clock::HybridClock()); - ASSERT_OK(clock->Init()); - scoped_refptr<TimeManager> time_manager(new TimeManager(clock, Timestamp::kMin)); + scoped_refptr<TimeManager> time_manager(new TimeManager(clock_.get(), Timestamp::kMin)); queue_.reset(new PeerMessageQueue( metric_entity_, log_.get(), @@ -233,7 +231,7 @@ class ConsensusQueueTest : public KuduTest { unique_ptr<ThreadPool> raft_pool_; gscoped_ptr<PeerMessageQueue> queue_; scoped_refptr<log::LogAnchorRegistry> registry_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; }; // Tests that the queue is able to track a peer when it starts tracking a peer @@ -242,7 +240,7 @@ class ConsensusQueueTest : public KuduTest { // falls in the middle of the current messages in the queue. TEST_F(ConsensusQueueTest, TestStartTrackingAfterStart) { queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(2)); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 100); ConsensusRequestPB request; ConsensusResponsePB response; @@ -318,7 +316,7 @@ TEST_F(ConsensusQueueTest, TestGetPagedMessages) { // Append the messages after the queue is tracked. Otherwise the ops might // get evicted from the cache immediately and the requests below would // result in async log reads instead of cache hits. - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 100); OpId last; for (int i = 0; i < 11; i++) { @@ -350,7 +348,7 @@ TEST_F(ConsensusQueueTest, TestGetPagedMessages) { TEST_F(ConsensusQueueTest, TestPeersDontAckBeyondWatermarks) { queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(3)); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 100); // Wait for the local peer to append all messages WaitForLocalPeerToAckIndex(100); @@ -382,7 +380,7 @@ TEST_F(ConsensusQueueTest, TestPeersDontAckBeyondWatermarks) { ASSERT_FALSE(needs_tablet_copy); ASSERT_EQ(50, request.ops_size()); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 101, 100); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 101, 100); SetLastReceivedAndLastCommitted(&response, request.ops(49).id()); response.set_responder_term(28); @@ -424,7 +422,7 @@ TEST_F(ConsensusQueueTest, TestQueueAdvancesCommittedIndex) { // Append 10 messages to the queue. // This should add messages 0.1 -> 0.7, 1.8 -> 1.10 to the queue. - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 10); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 10); WaitForLocalPeerToAckIndex(10); // Since only the local log has ACKed at this point, @@ -509,7 +507,7 @@ TEST_F(ConsensusQueueTest, TestNonVoterAcksDontCountTowardMajority) { // Append 10 messages to the queue. // This should add messages 0.1 -> 0.7, 1.8 -> 1.10 to the queue. const int kNumMessages = 10; - AppendReplicateMessagesToQueue(queue_.get(), clock_, + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), /*first=*/ 1, /*count=*/ kNumMessages); WaitForLocalPeerToAckIndex(kNumMessages); @@ -560,7 +558,7 @@ TEST_F(ConsensusQueueTest, TestQueueLoadsOperationsForPeer) { const int kOpsToAppend = 100; for (int i = 1; i <= kOpsToAppend; i++) { - ASSERT_OK(log::AppendNoOpToLogSync(clock_, log_.get(), &opid)); + ASSERT_OK(log::AppendNoOpToLogSync(clock_.get(), log_.get(), &opid)); // Roll the log every 10 ops if (i % 10 == 0) { ASSERT_OK(log_->AllocateSegmentAndRollOverForTests()); @@ -623,7 +621,7 @@ TEST_F(ConsensusQueueTest, TestQueueHandlesOperationOverwriting) { OpId opid = MakeOpId(1, 1); // Append 10 messages in term 1 to the log. for (int i = 1; i <= 10; i++) { - ASSERT_OK(log::AppendNoOpToLogSync(clock_, log_.get(), &opid)); + ASSERT_OK(log::AppendNoOpToLogSync(clock_.get(), log_.get(), &opid)); // Roll the log every 3 ops if (i % 3 == 0) { ASSERT_OK(log_->AllocateSegmentAndRollOverForTests()); @@ -633,7 +631,7 @@ TEST_F(ConsensusQueueTest, TestQueueHandlesOperationOverwriting) { opid = MakeOpId(2, 11); // Now append 10 more messages in term 2. for (int i = 11; i <= 20; i++) { - ASSERT_OK(log::AppendNoOpToLogSync(clock_, log_.get(), &opid)); + ASSERT_OK(log::AppendNoOpToLogSync(clock_.get(), log_.get(), &opid)); // Roll the log every 3 ops if (i % 3 == 0) { ASSERT_OK(log_->AllocateSegmentAndRollOverForTests()); @@ -729,7 +727,7 @@ TEST_F(ConsensusQueueTest, TestQueueMovesWatermarksBackward) { queue_->SetNonLeaderMode(BuildRaftConfigPBForTests(3)); // Append a bunch of messages and update as if they were also appeneded to the leader. queue_->UpdateLastIndexAppendedToLeader(10); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 10); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 10); log_->WaitUntilAllFlushed(); // Now rewrite some of the operations and wait for the log to append. @@ -889,7 +887,7 @@ TEST_F(ConsensusQueueTest, TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog) // Test that Tablet Copy is triggered when a "tablet not found" error occurs. TEST_F(ConsensusQueueTest, TestTriggerTabletCopyIfTabletNotFound) { queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(3)); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 100); ConsensusRequestPB request; ConsensusResponsePB response; @@ -926,7 +924,7 @@ TEST_F(ConsensusQueueTest, TestFollowerCommittedIndexAndMetrics) { // Emulate a follower sending a request to replicate 10 messages. queue_->UpdateLastIndexAppendedToLeader(10); - AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 10); + AppendReplicateMessagesToQueue(queue_.get(), clock_.get(), 1, 10); WaitForLocalPeerToAckIndex(10); // The committed_index should be MinimumOpId() since UpdateFollowerCommittedIndex diff --git a/src/kudu/consensus/consensus_queue.h b/src/kudu/consensus/consensus_queue.h index 69b10b5..2b9e546 100644 --- a/src/kudu/consensus/consensus_queue.h +++ b/src/kudu/consensus/consensus_queue.h @@ -14,9 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - -#ifndef KUDU_CONSENSUS_CONSENSUS_QUEUE_H_ -#define KUDU_CONSENSUS_CONSENSUS_QUEUE_H_ +#pragma once #include <cstdint> #include <functional> @@ -606,5 +604,3 @@ class PeerMessageQueueObserver { } // namespace consensus } // namespace kudu - -#endif /* KUDU_CONSENSUS_CONSENSUS_QUEUE_H_ */ diff --git a/src/kudu/consensus/log-test-base.h b/src/kudu/consensus/log-test-base.h index 1fcbad0..a843e77 100644 --- a/src/kudu/consensus/log-test-base.h +++ b/src/kudu/consensus/log-test-base.h @@ -14,8 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. -#ifndef KUDU_CONSENSUS_LOG_TEST_BASE_H -#define KUDU_CONSENSUS_LOG_TEST_BASE_H +#pragma once #include "kudu/consensus/log.h" @@ -66,7 +65,7 @@ constexpr bool APPEND_ASYNC = false; // Append a single batch of 'count' NoOps to the log. // If 'size' is not NULL, increments it by the expected increase in log size. // Increments 'op_id''s index once for each operation logged. -inline Status AppendNoOpsToLogSync(const scoped_refptr<clock::Clock>& clock, +inline Status AppendNoOpsToLogSync(clock::Clock* clock, Log* log, consensus::OpId* op_id, int count, @@ -104,7 +103,7 @@ inline Status AppendNoOpsToLogSync(const scoped_refptr<clock::Clock>& clock, return s.Wait(); } -inline Status AppendNoOpToLogSync(const scoped_refptr<clock::Clock>& clock, +inline Status AppendNoOpToLogSync(clock::Clock* clock, Log* log, consensus::OpId* op_id, int* size = nullptr) { @@ -332,7 +331,7 @@ class LogTestBase : public KuduTest { // If non-NULL, and if the write is successful, 'size' is incremented // by the size of the written operation. Status AppendNoOp(consensus::OpId* op_id, int* size = nullptr) { - return AppendNoOpToLogSync(clock_, log_.get(), op_id, size); + return AppendNoOpToLogSync(clock_.get(), log_.get(), op_id, size); } // Append a number of no-op entries to the log. @@ -374,8 +373,8 @@ class LogTestBase : public KuduTest { }; const Schema schema_; - gscoped_ptr<FsManager> fs_manager_; - gscoped_ptr<MetricRegistry> metric_registry_; + std::unique_ptr<FsManager> fs_manager_; + std::unique_ptr<MetricRegistry> metric_registry_; scoped_refptr<MetricEntity> metric_entity_; scoped_refptr<Log> log_; int64_t current_index_; @@ -383,10 +382,8 @@ class LogTestBase : public KuduTest { // Reusable entries vector that deletes the entries on destruction. LogEntries entries_; scoped_refptr<LogAnchorRegistry> log_anchor_registry_; - scoped_refptr<clock::Clock> clock_; + std::unique_ptr<clock::Clock> clock_; }; } // namespace log } // namespace kudu - -#endif diff --git a/src/kudu/consensus/log-test.cc b/src/kudu/consensus/log-test.cc index b50c5d2..4a68189 100644 --- a/src/kudu/consensus/log-test.cc +++ b/src/kudu/consensus/log-test.cc @@ -191,7 +191,7 @@ TEST_P(LogTestOptionalCompression, TestMultipleEntriesInABatch) { opid.set_term(1); opid.set_index(1); - AppendNoOpsToLogSync(clock_, log_.get(), &opid, 2); + AppendNoOpsToLogSync(clock_.get(), log_.get(), &opid, 2); // RollOver() the batch so that we have a properly formed footer. ASSERT_OK(log_->AllocateSegmentAndRollOverForTests()); @@ -1138,7 +1138,7 @@ TEST_F(LogTest, TestAutoStopIdleAppendThread) { // after the append long enough for the append thread to shut itself down // again. ASSERT_EVENTUALLY([&]() { - AppendNoOpsToLogSync(clock_, log_.get(), &opid, 2); + AppendNoOpsToLogSync(clock_.get(), log_.get(), &opid, 2); ASSERT_TRUE(log_->append_thread_active_for_tests()); debug::ScopedTSANIgnoreReadsAndWrites ignore_tsan; ASSERT_GT(log_->segment_allocator_.active_segment_->compress_buf_.capacity(), diff --git a/src/kudu/consensus/log_cache-test.cc b/src/kudu/consensus/log_cache-test.cc index b72544f..fbc1c8f 100644 --- a/src/kudu/consensus/log_cache-test.cc +++ b/src/kudu/consensus/log_cache-test.cc @@ -15,9 +15,12 @@ // specific language governing permissions and limitations // under the License. +#include "kudu/consensus/log_cache.h" + #include <atomic> #include <cstddef> #include <cstdint> +#include <initializer_list> #include <memory> #include <ostream> #include <string> @@ -35,7 +38,6 @@ #include "kudu/consensus/consensus-test-util.h" #include "kudu/consensus/consensus.pb.h" #include "kudu/consensus/log.h" -#include "kudu/consensus/log_cache.h" #include "kudu/consensus/log_util.h" #include "kudu/consensus/opid.pb.h" #include "kudu/consensus/opid_util.h" @@ -57,6 +59,7 @@ using std::atomic; using std::shared_ptr; using std::thread; +using std::unique_ptr; using std::vector; using strings::Substitute; @@ -130,10 +133,10 @@ class LogCacheTest : public KuduTest { const Schema schema_; MetricRegistry metric_registry_; scoped_refptr<MetricEntity> metric_entity_; - gscoped_ptr<FsManager> fs_manager_; - gscoped_ptr<LogCache> cache_; + unique_ptr<FsManager> fs_manager_; + unique_ptr<LogCache> cache_; scoped_refptr<log::Log> log_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; }; diff --git a/src/kudu/consensus/raft_consensus_quorum-test.cc b/src/kudu/consensus/raft_consensus_quorum-test.cc index 83c6d80..d2676f8 100644 --- a/src/kudu/consensus/raft_consensus_quorum-test.cc +++ b/src/kudu/consensus/raft_consensus_quorum-test.cc @@ -216,7 +216,7 @@ class RaftConsensusQuorumTest : public KuduTest { unique_ptr<PeerProxyFactory> proxy_factory( new LocalTestPeerProxyFactory(peers_.get())); scoped_refptr<TimeManager> time_manager( - new TimeManager(clock_, Timestamp::kMin)); + new TimeManager(clock_.get(), Timestamp::kMin)); unique_ptr<TestTransactionFactory> txn_factory( new TestTransactionFactory(logs_[i].get())); txn_factory->SetConsensus(peer.get()); @@ -582,7 +582,7 @@ class RaftConsensusQuorumTest : public KuduTest { unique_ptr<ThreadPool> raft_pool_; unique_ptr<TestPeerMapManager> peers_; vector<unique_ptr<TestTransactionFactory>> txn_factories_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; MetricRegistry metric_registry_; scoped_refptr<MetricEntity> metric_entity_; const Schema schema_; diff --git a/src/kudu/consensus/time_manager-test.cc b/src/kudu/consensus/time_manager-test.cc index 66075d6..f5f27e9 100644 --- a/src/kudu/consensus/time_manager-test.cc +++ b/src/kudu/consensus/time_manager-test.cc @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include "kudu/consensus/time_manager.h" + #include <memory> #include <thread> #include <vector> @@ -22,11 +24,9 @@ #include <glog/logging.h> #include <gtest/gtest.h> -#include "kudu/clock/clock.h" #include "kudu/clock/hybrid_clock.h" #include "kudu/common/timestamp.h" #include "kudu/consensus/consensus.pb.h" -#include "kudu/consensus/time_manager.h" #include "kudu/gutil/ref_counted.h" #include "kudu/util/countdown_latch.h" #include "kudu/util/monotime.h" @@ -34,11 +34,13 @@ #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" +using std::thread; +using std::unique_ptr; +using std::vector; + namespace kudu { namespace consensus { -using std::unique_ptr; - class TimeManagerTest : public KuduTest { public: TimeManagerTest() : clock_(new clock::HybridClock()) {} @@ -55,7 +57,7 @@ class TimeManagerTest : public KuduTest { protected: void InitTimeManager(Timestamp initial_safe_time = Timestamp::kMin) { - time_manager_.reset(new TimeManager(clock_, initial_safe_time)); + time_manager_.reset(new TimeManager(clock_.get(), initial_safe_time)); } // Returns a latch that allows to wait for TimeManager to consider 'safe_time' safe. @@ -71,10 +73,10 @@ class TimeManagerTest : public KuduTest { return latch; } - scoped_refptr<clock::HybridClock> clock_; + unique_ptr<clock::HybridClock> clock_; scoped_refptr<TimeManager> time_manager_; - std::vector<unique_ptr<CountDownLatch>> latches_; - std::vector<std::thread> threads_; + vector<unique_ptr<CountDownLatch>> latches_; + vector<thread> threads_; }; // Tests TimeManager's functionality in non-leader mode and the transition to leader mode. diff --git a/src/kudu/consensus/time_manager.cc b/src/kudu/consensus/time_manager.cc index de48fab..5428e51 100644 --- a/src/kudu/consensus/time_manager.cc +++ b/src/kudu/consensus/time_manager.cc @@ -25,6 +25,7 @@ #include <gflags/gflags.h> #include <glog/logging.h> +#include "kudu/clock/clock.h" #include "kudu/consensus/consensus.pb.h" #include "kudu/gutil/macros.h" #include "kudu/gutil/port.h" @@ -72,12 +73,12 @@ ExternalConsistencyMode TimeManager::GetMessageConsistencyMode(const ReplicateMs return CLIENT_PROPAGATED; } -TimeManager::TimeManager(scoped_refptr<Clock> clock, Timestamp initial_safe_time) +TimeManager::TimeManager(Clock* clock, Timestamp initial_safe_time) : last_serial_ts_assigned_(initial_safe_time), last_safe_ts_(initial_safe_time), last_advanced_safe_time_(MonoTime::Now()), mode_(NON_LEADER), - clock_(std::move(clock)) {} + clock_(clock) {} void TimeManager::SetLeaderMode() { Lock l(lock_); diff --git a/src/kudu/consensus/time_manager.h b/src/kudu/consensus/time_manager.h index b431b4c..45304c1 100644 --- a/src/kudu/consensus/time_manager.h +++ b/src/kudu/consensus/time_manager.h @@ -21,7 +21,6 @@ #include <gtest/gtest_prod.h> -#include "kudu/clock/clock.h" #include "kudu/common/common.pb.h" #include "kudu/common/timestamp.h" #include "kudu/gutil/ref_counted.h" @@ -32,6 +31,9 @@ namespace kudu { class CountDownLatch; +namespace clock { +class Clock; +} // namespace clock namespace consensus { class ReplicateMsg; @@ -73,7 +75,7 @@ class TimeManager : public RefCountedThreadSafe<TimeManager> { public: // Constructs a TimeManager in non-leader mode. - TimeManager(scoped_refptr<clock::Clock> clock, Timestamp initial_safe_time); + TimeManager(clock::Clock* clock, Timestamp initial_safe_time); // Sets this TimeManager to leader mode. void SetLeaderMode(); @@ -211,7 +213,7 @@ class TimeManager : public RefCountedThreadSafe<TimeManager> { // The current mode of the TimeManager. Mode mode_; - const scoped_refptr<clock::Clock> clock_; + clock::Clock* clock_; const std::string local_peer_uuid_; }; diff --git a/src/kudu/integration-tests/fuzz-itest.cc b/src/kudu/integration-tests/fuzz-itest.cc index 2ee1a24..5f0e0de 100644 --- a/src/kudu/integration-tests/fuzz-itest.cc +++ b/src/kudu/integration-tests/fuzz-itest.cc @@ -30,7 +30,6 @@ #include <boost/optional/optional.hpp> // IWYU pragma: keep #include <boost/optional/optional_io.hpp> // IWYU pragma: keep #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <glog/stl_logging.h> #include <gtest/gtest.h> @@ -916,7 +915,7 @@ void FuzzTest::RunFuzzCase(const vector<TestOp>& test_ops, FlushSessionOrDie(session_); cur_val = pending_val; int current_time = down_cast<kudu::clock::LogicalClock*>( - tablet()->clock().get())->GetCurrentTime(); + tablet()->clock())->GetCurrentTime(); VLOG(1) << "Current time: " << current_time; saved_values_[current_time] = cur_val; saved_redos_[current_time] = pending_redos; diff --git a/src/kudu/integration-tests/tablet_history_gc-itest.cc b/src/kudu/integration-tests/tablet_history_gc-itest.cc index e2e2371..6f78057 100644 --- a/src/kudu/integration-tests/tablet_history_gc-itest.cc +++ b/src/kudu/integration-tests/tablet_history_gc-itest.cc @@ -28,19 +28,18 @@ #include <vector> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <gtest/gtest.h> -#include "kudu/clock/clock.h" -#include "kudu/clock/hybrid_clock.h" -#include "kudu/clock/mock_ntp.h" -#include "kudu/clock/time_service.h" #include "kudu/client/client-test-util.h" #include "kudu/client/client.h" #include "kudu/client/scan_batch.h" #include "kudu/client/shared_ptr.h" #include "kudu/client/write_op.h" +#include "kudu/clock/clock.h" +#include "kudu/clock/hybrid_clock.h" +#include "kudu/clock/mock_ntp.h" +#include "kudu/clock/time_service.h" #include "kudu/common/partial_row.h" #include "kudu/common/schema.h" #include "kudu/common/timestamp.h" @@ -230,7 +229,7 @@ TEST_F(TabletHistoryGcITest, TestUndoDeltaBlockGc) { // Move the clock so all operations are in the past. Then wait until we have // no more undo deltas. - HybridClock* c = down_cast<HybridClock*>(tablet->clock().get()); + HybridClock* c = down_cast<HybridClock*>(tablet->clock()); AddTimeToHybridClock(c, MonoDelta::FromSeconds(FLAGS_tablet_history_max_age_sec)); ASSERT_EVENTUALLY([&] { ASSERT_EQ(0, tablet->CountUndoDeltasForTests()); diff --git a/src/kudu/integration-tests/ts_recovery-itest.cc b/src/kudu/integration-tests/ts_recovery-itest.cc index 8c41956..6be498e 100644 --- a/src/kudu/integration-tests/ts_recovery-itest.cc +++ b/src/kudu/integration-tests/ts_recovery-itest.cc @@ -23,6 +23,7 @@ #include <string> #include <unordered_map> #include <unordered_set> +#include <utility> #include <vector> #include <glog/logging.h> @@ -52,7 +53,6 @@ #include "kudu/fs/fs.pb.h" #include "kudu/fs/fs_manager.h" #include "kudu/gutil/basictypes.h" -#include "kudu/gutil/gscoped_ptr.h" #include "kudu/gutil/ref_counted.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/gutil/strings/util.h" @@ -80,38 +80,37 @@ METRIC_DECLARE_gauge_uint64(tablets_num_failed); +using kudu::client::KuduClient; +using kudu::client::KuduInsert; +using kudu::client::KuduSession; +using kudu::client::KuduTable; +using kudu::client::KuduUpdate; +using kudu::client::sp::shared_ptr; +using kudu::cluster::ExternalTabletServer; +using kudu::cluster::ExternalMiniClusterOptions; +using kudu::clock::Clock; +using kudu::clock::HybridClock; +using kudu::consensus::ConsensusMetadata; +using kudu::consensus::ConsensusMetadataManager; +using kudu::consensus::ConsensusStatePB; +using kudu::consensus::EXCLUDE_HEALTH_REPORT; +using kudu::consensus::OpId; +using kudu::consensus::RECEIVED_OPID; +using kudu::fs::BlockManager; +using kudu::itest::MiniClusterFsInspector; +using kudu::itest::TServerDetails; +using kudu::log::AppendNoOpsToLogSync; +using kudu::log::Log; +using kudu::log::LogOptions; +using kudu::tablet::TabletMetadata; +using kudu::tablet::TabletSuperBlockPB; using std::string; using std::unique_ptr; using std::vector; +using strings::Substitute; namespace kudu { -using client::KuduClient; -using client::KuduInsert; -using client::KuduSession; -using client::KuduTable; -using client::KuduUpdate; -using client::sp::shared_ptr; -using cluster::ExternalTabletServer; -using cluster::ExternalMiniClusterOptions; -using clock::Clock; -using clock::HybridClock; -using consensus::ConsensusMetadata; -using consensus::ConsensusMetadataManager; -using consensus::ConsensusStatePB; -using consensus::EXCLUDE_HEALTH_REPORT; -using consensus::OpId; -using consensus::RECEIVED_OPID; -using fs::BlockManager; -using itest::MiniClusterFsInspector; -using kudu::itest::TServerDetails; -using log::AppendNoOpsToLogSync; -using log::Log; -using log::LogOptions; -using strings::Substitute; -using tablet::TabletMetadata; -using tablet::TabletSuperBlockPB; - namespace { // Generate a row key such that an increasing sequence (0...N) ends up spreading writes // across the key space as several sequential streams rather than a single sequential @@ -203,7 +202,7 @@ TEST_F(TsRecoveryITest, TestTabletRecoveryAfterSegmentDelete) { opts.wal_root = ets->wal_dir(); opts.data_roots = ets->data_dirs(); - gscoped_ptr<FsManager> fs_manager(new FsManager(env_, opts)); + unique_ptr<FsManager> fs_manager(new FsManager(env_, opts)); ASSERT_OK(fs_manager->Open()); @@ -587,11 +586,11 @@ TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) { FsManagerOpts opts; opts.wal_root = ets->wal_dir(); opts.data_roots = ets->data_dirs(); - gscoped_ptr<FsManager> fs_manager(new FsManager(env_, opts)); + unique_ptr<FsManager> fs_manager(new FsManager(env_, opts)); ASSERT_OK(fs_manager->Open()); scoped_refptr<ConsensusMetadataManager> cmeta_manager( new ConsensusMetadataManager(fs_manager.get())); - scoped_refptr<Clock> clock(new HybridClock()); + unique_ptr<Clock> clock(new HybridClock); ASSERT_OK(clock->Init()); OpId opid; @@ -610,7 +609,7 @@ TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) { // Write a series of negative OpIds. // This will cause a crash, but only after they have been written to disk. - ASSERT_OK(AppendNoOpsToLogSync(clock, log.get(), &opid, kNumOverflowedEntriesToWrite)); + ASSERT_OK(AppendNoOpsToLogSync(clock.get(), log.get(), &opid, kNumOverflowedEntriesToWrite)); }, "Check failed: log_index > 0"); // Before restarting the tablet server, delete the initial log segment from @@ -745,7 +744,7 @@ class UpdaterThreads { int i = inserted_->Load(); if (i == 0) continue; - gscoped_ptr<KuduUpdate> up(table_->NewUpdate()); + unique_ptr<KuduUpdate> up(table_->NewUpdate()); CHECK_OK(up->mutable_row()->SetInt32("key", IntToKey(rng.Uniform(i) + 1))); CHECK_OK(up->mutable_row()->SetInt32("int_val", rng.Next32())); CHECK_OK(session->Apply(up.release())); @@ -831,7 +830,7 @@ TEST_P(Kudu969Test, Test) { session->SetTimeoutMillis(1000); CHECK_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH)); for (int i = 1; ts->IsProcessAlive(); i++) { - gscoped_ptr<KuduInsert> ins(table->NewInsert()); + unique_ptr<KuduInsert> ins(table->NewInsert()); ASSERT_OK(ins->mutable_row()->SetInt32("key", IntToKey(i))); ASSERT_OK(ins->mutable_row()->SetInt32("int_val", i)); ASSERT_OK(ins->mutable_row()->SetNull("string_val")); diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc index a70bb91..fb5e47a 100644 --- a/src/kudu/master/sys_catalog.cc +++ b/src/kudu/master/sys_catalog.cc @@ -34,7 +34,6 @@ #include <glog/logging.h> #include <google/protobuf/util/message_differencer.h> -#include "kudu/clock/clock.h" #include "kudu/common/column_predicate.h" #include "kudu/common/common.pb.h" #include "kudu/common/iterator.h" @@ -392,7 +391,7 @@ Status SysCatalogTable::SetupTablet( RETURN_NOT_OK_SHUTDOWN(BootstrapTablet( metadata, cmeta->CommittedConfig(), - scoped_refptr<clock::Clock>(master_->clock()), + master_->clock(), master_->mem_tracker(), scoped_refptr<rpc::ResultTracker>(), metric_registry_, @@ -408,7 +407,7 @@ Status SysCatalogTable::SetupTablet( RETURN_NOT_OK_SHUTDOWN(tablet_replica_->Start( consensus_info, tablet, - scoped_refptr<clock::Clock>(master_->clock()), + master_->clock(), master_->messenger(), scoped_refptr<rpc::ResultTracker>(), log, diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc index 08fcfd7..e040ca3 100644 --- a/src/kudu/server/server_base.cc +++ b/src/kudu/server/server_base.cc @@ -384,7 +384,7 @@ ServerBase::ServerBase(string name, const ServerBaseOptions& options, fs_manager_.reset(new FsManager(options.env, std::move(fs_opts))); if (FLAGS_use_hybrid_clock) { - clock_ = new clock::HybridClock(); + clock_.reset(new clock::HybridClock); } else { clock_ = clock::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp); } diff --git a/src/kudu/server/server_base.h b/src/kudu/server/server_base.h index 6859e3d..f97214c 100644 --- a/src/kudu/server/server_base.h +++ b/src/kudu/server/server_base.h @@ -97,14 +97,13 @@ class ServerBase { const scoped_refptr<MetricEntity>& metric_entity() const { return metric_entity_; } - MetricRegistry* metric_registry() { return metric_registry_.get(); } + MetricRegistry* metric_registry() const { return metric_registry_.get(); } const scoped_refptr<rpc::ResultTracker>& result_tracker() const { return result_tracker_; } - // Returns this server's clock. - clock::Clock* clock() { return clock_.get(); } + clock::Clock* clock() const { return clock_.get(); } - DnsResolver* dns_resolver() { return dns_resolver_.get(); } + DnsResolver* dns_resolver() const { return dns_resolver_.get(); } // Return a PB describing the status of the server (version info, bound ports, etc) Status GetStatusPB(ServerStatusPB* status) const; @@ -185,7 +184,7 @@ class ServerBase { scoped_refptr<rpc::ResultTracker> result_tracker_; bool is_first_run_; - scoped_refptr<clock::Clock> clock_; + std::unique_ptr<clock::Clock> clock_; // The instance identifier of this server. gscoped_ptr<NodeInstancePB> instance_pb_; diff --git a/src/kudu/tablet/compaction-test.cc b/src/kudu/tablet/compaction-test.cc index 4f81f95..82765f2 100644 --- a/src/kudu/tablet/compaction-test.cc +++ b/src/kudu/tablet/compaction-test.cc @@ -30,7 +30,6 @@ #include <vector> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <gtest/gtest.h> @@ -485,7 +484,7 @@ class TestCompaction : public KuduRowSetTest { RowBuilder row_builder_; char key_buf_[256]; Arena arena_; - scoped_refptr<clock::LogicalClock> clock_; + unique_ptr<clock::LogicalClock> clock_; MvccManager mvcc_; scoped_refptr<LogAnchorRegistry> log_anchor_registry_; diff --git a/src/kudu/tablet/deltamemstore-test.cc b/src/kudu/tablet/deltamemstore-test.cc index 5ea72a6..fce37c0 100644 --- a/src/kudu/tablet/deltamemstore-test.cc +++ b/src/kudu/tablet/deltamemstore-test.cc @@ -22,6 +22,7 @@ #include <cstdio> #include <cstdlib> #include <cstring> +#include <map> #include <memory> #include <ostream> #include <string> @@ -49,7 +50,6 @@ #include "kudu/fs/block_manager.h" #include "kudu/fs/fs_manager.h" #include "kudu/gutil/gscoped_ptr.h" -#include "kudu/gutil/ref_counted.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/tablet/delta_key.h" #include "kudu/tablet/delta_stats.h" @@ -161,7 +161,7 @@ class TestDeltaMemStore : public KuduTest { const Schema schema_; shared_ptr<DeltaMemStore> dms_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; MvccManager mvcc_; }; diff --git a/src/kudu/tablet/diskrowset-test-base.h b/src/kudu/tablet/diskrowset-test-base.h index 787dcf3..0c7190e 100644 --- a/src/kudu/tablet/diskrowset-test-base.h +++ b/src/kudu/tablet/diskrowset-test-base.h @@ -14,8 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. -#ifndef KUDU_TABLET_LAYER_TEST_BASE_H -#define KUDU_TABLET_LAYER_TEST_BASE_H +#pragma once #include <unistd.h> @@ -340,12 +339,10 @@ class TestRowSet : public KuduRowSetTest { size_t n_rows_; consensus::OpId op_id_; // Generally a "fake" OpId for these tests. - scoped_refptr<clock::Clock> clock_; + std::unique_ptr<clock::Clock> clock_; MvccManager mvcc_; scoped_refptr<log::LogAnchorRegistry> log_anchor_registry_; }; } // namespace tablet } // namespace kudu - -#endif diff --git a/src/kudu/tablet/diskrowset-test.cc b/src/kudu/tablet/diskrowset-test.cc index ecb6016..3437f7f 100644 --- a/src/kudu/tablet/diskrowset-test.cc +++ b/src/kudu/tablet/diskrowset-test.cc @@ -24,11 +24,11 @@ #include <ostream> #include <string> #include <unordered_set> +#include <utility> #include <vector> #include <boost/optional/optional.hpp> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <glog/stl_logging.h> #include <gtest/gtest.h> @@ -723,7 +723,7 @@ class DiffScanRowSetTest : public KuduRowSetTest, } consensus::OpId op_id_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; MvccManager mvcc_; }; diff --git a/src/kudu/tablet/memrowset-test.cc b/src/kudu/tablet/memrowset-test.cc index 29d8733..f3a5c39 100644 --- a/src/kudu/tablet/memrowset-test.cc +++ b/src/kudu/tablet/memrowset-test.cc @@ -19,10 +19,12 @@ #include <cstdint> #include <cstdio> +#include <initializer_list> #include <memory> #include <ostream> #include <string> #include <unordered_set> +#include <utility> #include <vector> #include <boost/optional/optional.hpp> @@ -259,7 +261,7 @@ class TestMemRowSet : public KuduTest { faststring mutation_buf_; const Schema schema_; const Schema key_schema_; - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; MvccManager mvcc_; }; diff --git a/src/kudu/tablet/mvcc-test.cc b/src/kudu/tablet/mvcc-test.cc index 8458922..d7cf1a5 100644 --- a/src/kudu/tablet/mvcc-test.cc +++ b/src/kudu/tablet/mvcc-test.cc @@ -15,6 +15,9 @@ // specific language governing permissions and limitations // under the License. +#include "kudu/tablet/mvcc.h" + +#include <memory> #include <mutex> #include <ostream> #include <string> @@ -28,9 +31,6 @@ #include "kudu/clock/hybrid_clock.h" #include "kudu/clock/logical_clock.h" #include "kudu/common/timestamp.h" -#include "kudu/gutil/gscoped_ptr.h" -#include "kudu/gutil/ref_counted.h" -#include "kudu/tablet/mvcc.h" #include "kudu/util/locks.h" #include "kudu/util/monotime.h" #include "kudu/util/status.h" @@ -38,6 +38,7 @@ #include "kudu/util/test_util.h" using std::thread; +using std::unique_ptr; namespace kudu { namespace tablet { @@ -48,8 +49,7 @@ using clock::HybridClock; class MvccTest : public KuduTest { public: MvccTest() - : clock_( - clock::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)) { + : clock_(clock::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)) { } void WaitForSnapshotAtTSThread(MvccManager* mgr, Timestamp ts) { @@ -66,10 +66,10 @@ class MvccTest : public KuduTest { } protected: - scoped_refptr<clock::Clock> clock_; + unique_ptr<clock::Clock> clock_; mutable simple_spinlock lock_; - gscoped_ptr<MvccSnapshot> result_snapshot_; + unique_ptr<MvccSnapshot> result_snapshot_; }; TEST_F(MvccTest, TestMvccBasic) { @@ -182,7 +182,7 @@ TEST_F(MvccTest, TestMvccMultipleInFlight) { } TEST_F(MvccTest, TestOutOfOrderTxns) { - scoped_refptr<Clock> hybrid_clock(new HybridClock()); + unique_ptr<Clock> hybrid_clock(new HybridClock); ASSERT_OK(hybrid_clock->Init()); MvccManager mgr; diff --git a/src/kudu/tablet/tablet-harness.h b/src/kudu/tablet/tablet-harness.h index 83ffc3b..f714d46 100644 --- a/src/kudu/tablet/tablet-harness.h +++ b/src/kudu/tablet/tablet-harness.h @@ -14,8 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. -#ifndef KUDU_TABLET_TABLET_REPLICA_HARNESS_H -#define KUDU_TABLET_TABLET_REPLICA_HARNESS_H +#pragma once #include <memory> #include <string> @@ -109,13 +108,13 @@ class TabletHarness { } if (options_.clock_type == Options::LOGICAL_CLOCK) { - clock_.reset(clock::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)); + clock_ = clock::LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp); } else { clock_.reset(new clock::HybridClock()); RETURN_NOT_OK(clock_->Init()); } tablet_.reset(new Tablet(metadata, - clock_, + clock_.get(), std::shared_ptr<MemTracker>(), metrics_registry_.get(), make_scoped_refptr(new log::LogAnchorRegistry()))); @@ -152,7 +151,7 @@ class TabletHarness { gscoped_ptr<MetricRegistry> metrics_registry_; - scoped_refptr<clock::Clock> clock_; + std::unique_ptr<clock::Clock> clock_; Schema schema_; gscoped_ptr<FsManager> fs_manager_; std::shared_ptr<Tablet> tablet_; @@ -160,4 +159,3 @@ class TabletHarness { } // namespace tablet } // namespace kudu -#endif /* KUDU_TABLET_TABLET_REPLICA_HARNESS_H */ diff --git a/src/kudu/tablet/tablet-test-util.h b/src/kudu/tablet/tablet-test-util.h index 3215254..3198b3d 100644 --- a/src/kudu/tablet/tablet-test-util.h +++ b/src/kudu/tablet/tablet-test-util.h @@ -14,7 +14,6 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - #pragma once #include <limits.h> diff --git a/src/kudu/tablet/tablet.cc b/src/kudu/tablet/tablet.cc index 032ceb8..f20f2a3 100644 --- a/src/kudu/tablet/tablet.cc +++ b/src/kudu/tablet/tablet.cc @@ -32,6 +32,7 @@ #include <gflags/gflags.h> #include <glog/logging.h> +#include "kudu/clock/clock.h" #include "kudu/clock/hybrid_clock.h" #include "kudu/common/common.pb.h" #include "kudu/common/encoded_key.h" @@ -214,7 +215,7 @@ TabletComponents::TabletComponents(shared_ptr<MemRowSet> mrs, //////////////////////////////////////////////////////////// Tablet::Tablet(scoped_refptr<TabletMetadata> metadata, - scoped_refptr<clock::Clock> clock, + clock::Clock* clock, shared_ptr<MemTracker> parent_mem_tracker, MetricRegistry* metric_registry, scoped_refptr<LogAnchorRegistry> log_anchor_registry) @@ -223,7 +224,7 @@ Tablet::Tablet(scoped_refptr<TabletMetadata> metadata, log_anchor_registry_(std::move(log_anchor_registry)), mem_trackers_(tablet_id(), std::move(parent_mem_tracker)), next_mrs_id_(0), - clock_(std::move(clock)), + clock_(clock), rowsets_flush_sem_(1), state_(kInitialized), last_write_time_(MonoTime::Now()), diff --git a/src/kudu/tablet/tablet.h b/src/kudu/tablet/tablet.h index d4997be..c419e01 100644 --- a/src/kudu/tablet/tablet.h +++ b/src/kudu/tablet/tablet.h @@ -14,7 +14,6 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - #pragma once #include <cstddef> @@ -30,7 +29,6 @@ #include <glog/logging.h> #include <gtest/gtest_prod.h> -#include "kudu/clock/clock.h" #include "kudu/common/iterator.h" #include "kudu/common/schema.h" #include "kudu/fs/io_context.h" @@ -69,9 +67,13 @@ class Timestamp; struct IterWithBounds; struct IteratorStats; +namespace clock { +class Clock; +} // namespace clock + namespace log { class LogAnchorRegistry; -} +} // namespace log namespace tablet { @@ -102,7 +104,7 @@ class Tablet { // If 'metric_registry' is non-NULL, then this tablet will create a 'tablet' entity // within the provided registry. Otherwise, no metrics are collected. Tablet(scoped_refptr<TabletMetadata> metadata, - scoped_refptr<clock::Clock> clock, + clock::Clock* clock, std::shared_ptr<MemTracker> parent_mem_tracker, MetricRegistry* metric_registry, scoped_refptr<log::LogAnchorRegistry> log_anchor_registry); @@ -440,7 +442,7 @@ class Tablet { // Return true if this RPC is allowed. bool ShouldThrottleAllow(int64_t bytes); - scoped_refptr<clock::Clock> clock() const { return clock_; } + clock::Clock* clock() const { return clock_; } std::string LogPrefix() const; @@ -727,7 +729,7 @@ class Tablet { int64_t next_mrs_id_; // A pointer to the server's clock. - scoped_refptr<clock::Clock> clock_; + clock::Clock* clock_; MvccManager mvcc_; LockManager lock_manager_; diff --git a/src/kudu/tablet/tablet_bootstrap-test.cc b/src/kudu/tablet/tablet_bootstrap-test.cc index 8c72817..fd29b29 100644 --- a/src/kudu/tablet/tablet_bootstrap-test.cc +++ b/src/kudu/tablet/tablet_bootstrap-test.cc @@ -31,7 +31,6 @@ #include <gtest/gtest.h> #include "kudu/clock/clock.h" -#include "kudu/clock/logical_clock.h" #include "kudu/common/common.pb.h" #include "kudu/common/iterator.h" #include "kudu/common/partial_row.h" @@ -78,6 +77,19 @@ #include "kudu/util/status.h" #include "kudu/util/test_macros.h" +using kudu::consensus::ConsensusBootstrapInfo; +using kudu::consensus::ConsensusMetadata; +using kudu::consensus::ConsensusMetadataManager; +using kudu::consensus::MakeOpId; +using kudu::consensus::OpId; +using kudu::consensus::ReplicateMsg; +using kudu::consensus::ReplicateRefPtr; +using kudu::consensus::kMinimumTerm; +using kudu::consensus::make_scoped_refptr_replicate; +using kudu::log::LogAnchorRegistry; +using kudu::log::LogTestBase; +using kudu::pb_util::SecureShortDebugString; +using kudu::tserver::WriteRequestPB; using std::shared_ptr; using std::string; using std::unique_ptr; @@ -89,23 +101,6 @@ class MemTracker; namespace tablet { -using clock::Clock; -using clock::LogicalClock; -using consensus::ConsensusBootstrapInfo; -using consensus::ConsensusMetadata; -using consensus::ConsensusMetadataManager; -using consensus::MakeOpId; -using consensus::OpId; -using consensus::ReplicateMsg; -using consensus::ReplicateRefPtr; -using consensus::kMinimumTerm; -using consensus::make_scoped_refptr_replicate; -using log::Log; -using log::LogAnchorRegistry; -using log::LogTestBase; -using pb_util::SecureShortDebugString; -using tserver::WriteRequestPB; - class BootstrapTest : public LogTestBase { protected: @@ -170,7 +165,7 @@ class BootstrapTest : public LogTestBase { RETURN_NOT_OK(BootstrapTablet( meta, cmeta->CommittedConfig(), - scoped_refptr<Clock>(LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)), + clock_.get(), shared_ptr<MemTracker>(), scoped_refptr<rpc::ResultTracker>(), nullptr, diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc index dd10f3b..baf1537 100644 --- a/src/kudu/tablet/tablet_bootstrap.cc +++ b/src/kudu/tablet/tablet_bootstrap.cc @@ -29,7 +29,6 @@ #include <boost/optional/optional.hpp> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <glog/logging.h> #include "kudu/clock/clock.h" @@ -194,7 +193,7 @@ class TabletBootstrap { public: TabletBootstrap(scoped_refptr<TabletMetadata> tablet_meta, RaftConfigPB committed_raft_config, - scoped_refptr<Clock> clock, + Clock* clock, shared_ptr<MemTracker> mem_tracker, scoped_refptr<ResultTracker> result_tracker, MetricRegistry* metric_registry, @@ -374,7 +373,7 @@ class TabletBootstrap { const scoped_refptr<TabletMetadata> tablet_meta_; const RaftConfigPB committed_raft_config_; - const scoped_refptr<Clock> clock_; + Clock* clock_; shared_ptr<MemTracker> mem_tracker_; scoped_refptr<rpc::ResultTracker> result_tracker_; MetricRegistry* metric_registry_; @@ -442,7 +441,7 @@ void TabletBootstrap::SetStatusMessage(const string& status) { Status BootstrapTablet(scoped_refptr<TabletMetadata> tablet_meta, RaftConfigPB committed_raft_config, - scoped_refptr<Clock> clock, + Clock* clock, shared_ptr<MemTracker> mem_tracker, scoped_refptr<ResultTracker> result_tracker, MetricRegistry* metric_registry, @@ -455,7 +454,7 @@ Status BootstrapTablet(scoped_refptr<TabletMetadata> tablet_meta, "tablet_id", tablet_meta->tablet_id()); TabletBootstrap bootstrap(std::move(tablet_meta), std::move(committed_raft_config), - std::move(clock), + clock, std::move(mem_tracker), std::move(result_tracker), metric_registry, @@ -491,14 +490,15 @@ static string DebugInfo(const string& tablet_id, TabletBootstrap::TabletBootstrap( scoped_refptr<TabletMetadata> tablet_meta, RaftConfigPB committed_raft_config, - scoped_refptr<Clock> clock, shared_ptr<MemTracker> mem_tracker, + Clock* clock, + shared_ptr<MemTracker> mem_tracker, scoped_refptr<ResultTracker> result_tracker, MetricRegistry* metric_registry, scoped_refptr<TabletReplica> tablet_replica, scoped_refptr<LogAnchorRegistry> log_anchor_registry) : tablet_meta_(std::move(tablet_meta)), committed_raft_config_(std::move(committed_raft_config)), - clock_(std::move(clock)), + clock_(clock), mem_tracker_(std::move(mem_tracker)), result_tracker_(std::move(result_tracker)), metric_registry_(metric_registry), diff --git a/src/kudu/tablet/tablet_bootstrap.h b/src/kudu/tablet/tablet_bootstrap.h index 6dace64..5bfa5a5 100644 --- a/src/kudu/tablet/tablet_bootstrap.h +++ b/src/kudu/tablet/tablet_bootstrap.h @@ -29,7 +29,7 @@ class MetricRegistry; namespace log { class Log; class LogAnchorRegistry; -} +} // namespace log namespace consensus { class RaftConfigPB; @@ -42,7 +42,7 @@ class ResultTracker; namespace clock { class Clock; -} +} // namespace clock namespace tablet { class Tablet; @@ -59,7 +59,7 @@ extern const char* kLogRecoveryDir; // TSTabletManager. Status BootstrapTablet(scoped_refptr<TabletMetadata> tablet_meta, consensus::RaftConfigPB committed_raft_config, - scoped_refptr<clock::Clock> clock, + clock::Clock* clock, std::shared_ptr<MemTracker> mem_tracker, scoped_refptr<rpc::ResultTracker> result_tracker, MetricRegistry* metric_registry, diff --git a/src/kudu/tablet/tablet_replica.cc b/src/kudu/tablet/tablet_replica.cc index fafc28f..b990ef8 100644 --- a/src/kudu/tablet/tablet_replica.cc +++ b/src/kudu/tablet/tablet_replica.cc @@ -28,7 +28,6 @@ #include <glog/logging.h> -#include "kudu/clock/clock.h" #include "kudu/common/partition.h" #include "kudu/common/timestamp.h" #include "kudu/consensus/consensus.pb.h" @@ -170,7 +169,7 @@ Status TabletReplica::Init(ServerContext server_ctx) { Status TabletReplica::Start(const ConsensusBootstrapInfo& bootstrap_info, shared_ptr<Tablet> tablet, - scoped_refptr<clock::Clock> clock, + clock::Clock* clock, shared_ptr<Messenger> messenger, scoped_refptr<ResultTracker> result_tracker, scoped_refptr<Log> log, @@ -190,7 +189,7 @@ Status TabletReplica::Start(const ConsensusBootstrapInfo& bootstrap_info, CHECK_EQ(BOOTSTRAPPING, state_); tablet_ = DCHECK_NOTNULL(std::move(tablet)); - clock_ = DCHECK_NOTNULL(std::move(clock)); + clock_ = DCHECK_NOTNULL(clock); messenger_ = DCHECK_NOTNULL(std::move(messenger)); result_tracker_ = std::move(result_tracker); // Passed null in tablet_replica-test log_ = DCHECK_NOTNULL(log); // Not moved because it's passed to RaftConsensus::Start() below. diff --git a/src/kudu/tablet/tablet_replica.h b/src/kudu/tablet/tablet_replica.h index 9cc38ec..915fad7 100644 --- a/src/kudu/tablet/tablet_replica.h +++ b/src/kudu/tablet/tablet_replica.h @@ -14,7 +14,6 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - #pragma once #include <cstddef> @@ -106,7 +105,7 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>, // in the consensus configuration. Status Start(const consensus::ConsensusBootstrapInfo& bootstrap_info, std::shared_ptr<tablet::Tablet> tablet, - scoped_refptr<clock::Clock> clock, + clock::Clock* clock, std::shared_ptr<rpc::Messenger> messenger, scoped_refptr<rpc::ResultTracker> result_tracker, scoped_refptr<log::Log> log, @@ -249,9 +248,7 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>, return log_.get(); } - clock::Clock* clock() { - return clock_.get(); - } + clock::Clock* clock() const { return clock_; } const scoped_refptr<log::LogAnchorRegistry>& log_anchor_registry() const { return log_anchor_registry_; @@ -382,7 +379,7 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>, // Token for serial task submission to the server-wide transaction prepare pool. std::unique_ptr<ThreadPoolToken> prepare_pool_token_; - scoped_refptr<clock::Clock> clock_; + clock::Clock* clock_; // List of maintenance operations for the tablet that need information that only the peer // can provide. diff --git a/src/kudu/tools/tool_action_perf.cc b/src/kudu/tools/tool_action_perf.cc index c48bb08..845f79f 100644 --- a/src/kudu/tools/tool_action_perf.cc +++ b/src/kudu/tools/tool_action_perf.cc @@ -775,8 +775,7 @@ Status TabletScan(const RunnerContext& context) { scoped_refptr<ConsensusMetadata> cmeta; RETURN_NOT_OK(cmeta_manager->Load(tablet_id, &cmeta)); - scoped_refptr<Clock> clock( - LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)); + unique_ptr<Clock> clock(LogicalClock::CreateStartingAt(Timestamp::kInitialTimestamp)); RETURN_NOT_OK(clock->Init()); scoped_refptr<LogAnchorRegistry> registry(new LogAnchorRegistry()); @@ -787,7 +786,7 @@ Status TabletScan(const RunnerContext& context) { ConsensusBootstrapInfo cbi; RETURN_NOT_OK(tablet::BootstrapTablet(std::move(tmeta), cmeta->CommittedConfig(), - std::move(clock), + clock.get(), /*mem_tracker=*/ nullptr, /*result_tracker=*/ nullptr, /*metric_registry=*/ nullptr, diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc index 4c5d240..8b2a54d 100644 --- a/src/kudu/tserver/ts_tablet_manager.cc +++ b/src/kudu/tserver/ts_tablet_manager.cc @@ -31,7 +31,6 @@ #include <gflags/gflags.h> #include <glog/logging.h> -#include "kudu/clock/clock.h" #include "kudu/common/common.pb.h" #include "kudu/common/wire_protocol.h" #include "kudu/common/wire_protocol.pb.h" @@ -1113,7 +1112,7 @@ void TSTabletManager::OpenTablet(const scoped_refptr<TabletReplica>& replica, replica->SetBootstrapping(); s = BootstrapTablet(replica->tablet_metadata(), replica->consensus()->CommittedConfig(), - scoped_refptr<clock::Clock>(server_->clock()), + server_->clock(), server_->mem_tracker(), server_->result_tracker(), metric_registry_, @@ -1134,7 +1133,7 @@ void TSTabletManager::OpenTablet(const scoped_refptr<TabletReplica>& replica, TRACE("Starting tablet replica"); s = replica->Start(bootstrap_info, tablet, - scoped_refptr<clock::Clock>(server_->clock()), + server_->clock(), server_->messenger(), server_->result_tracker(), log,
