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 0498724d5 [RowSetMetadata] fix const-correctness of a few methods
0498724d5 is described below
commit 0498724d507cc88579cb672c279cabc42a07929a
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Sep 19 13:03:35 2024 -0700
[RowSetMetadata] fix const-correctness of a few methods
In review feedback for [1] it was mentioned to address const-correctness
of a few methods in the RowSetMetadata class. This patch does exactly
so, also bringing the code up to date with the current C++ style guide.
One method in FsManager was also updated because it's related to these
changes in RowSetMetadata.
This patch doesn't contain any functional modifications.
[1] https://gerrit.cloudera.org/#/c/21779/
Change-Id: Idf9fae19cf77795730b1e58b98617bb29ed3552e
Reviewed-on: http://gerrit.cloudera.org:8080/21826
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Yifan Zhang <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
src/kudu/fs/fs_manager.cc | 2 +-
src/kudu/fs/fs_manager.h | 2 +-
src/kudu/tablet/rowset_metadata.cc | 59 ++++++++++++++++++--------------------
src/kudu/tablet/rowset_metadata.h | 50 ++++++++++++++++----------------
4 files changed, 55 insertions(+), 58 deletions(-)
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index eda03085f..fdf3e582b 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -1498,7 +1498,7 @@ Status FsManager::OpenBlock(const BlockId& block_id,
return bm->OpenBlock(block_id, block);
}
-bool FsManager::BlockExists(const BlockId& block_id, const string& tenant_id) {
+bool FsManager::BlockExists(const BlockId& block_id, const string& tenant_id)
const {
if (!VertifyTenant(tenant_id)) {
return false;
}
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index f57de339b..435a4cbf1 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -264,7 +264,7 @@ class FsManager {
// If the tenant id is not specified, we treat it as the default tenant.
bool BlockExists(const BlockId& block_id,
- const std::string& tenant_id = fs::kDefaultTenantID);
+ const std::string& tenant_id = fs::kDefaultTenantID) const;
// ==========================================================================
// on-disk path
diff --git a/src/kudu/tablet/rowset_metadata.cc
b/src/kudu/tablet/rowset_metadata.cc
index 463c9489f..1db127c99 100644
--- a/src/kudu/tablet/rowset_metadata.cc
+++ b/src/kudu/tablet/rowset_metadata.cc
@@ -18,8 +18,10 @@
#include "kudu/tablet/rowset_metadata.h"
#include <algorithm>
+#include <mutex>
#include <ostream>
#include <string>
+#include <tuple>
#include <unordered_set>
#include <vector>
@@ -29,8 +31,11 @@
#include "kudu/gutil/strings/substitute.h"
#include "kudu/tablet/metadata.pb.h"
-using std::unique_ptr;
+using std::lock_guard;
+using std::map;
+using std::string;
using std::vector;
+using std::unique_ptr;
using strings::Substitute;
namespace kudu {
@@ -73,7 +78,7 @@ Status RowSetMetadata::InitFromPB(const RowSetDataPB& pb) {
}
void RowSetMetadata::LoadFromPB(const RowSetDataPB& pb) {
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
id_ = pb.id();
// Load the min/max keys.
@@ -124,16 +129,13 @@ void RowSetMetadata::LoadFromPB(const RowSetDataPB& pb) {
}
}
-void RowSetMetadata::ToProtobuf(RowSetDataPB *pb) {
- std::lock_guard l(lock_);
+void RowSetMetadata::ToProtobuf(RowSetDataPB* pb) const {
+ lock_guard l(lock_);
pb->set_id(id_);
// Write Column Files
- for (const ColumnIdToBlockIdMap::value_type& e : blocks_by_col_id_) {
- ColumnId col_id = e.first;
- const BlockId& block_id = e.second;
-
- ColumnDataPB *col_data = pb->add_columns();
+ for (const auto& [col_id, block_id] : blocks_by_col_id_) {
+ auto* col_data = pb->add_columns();
block_id.CopyToPB(col_data->mutable_block());
col_data->set_column_id(col_id);
}
@@ -142,12 +144,12 @@ void RowSetMetadata::ToProtobuf(RowSetDataPB *pb) {
pb->set_last_durable_dms_id(last_durable_redo_dms_id_);
for (const BlockId& redo_delta_block : redo_delta_blocks_) {
- DeltaDataPB *redo_delta_pb = pb->add_redo_deltas();
+ auto* redo_delta_pb = pb->add_redo_deltas();
redo_delta_block.CopyToPB(redo_delta_pb->mutable_block());
}
for (const BlockId& undo_delta_block : undo_delta_blocks_) {
- DeltaDataPB *undo_delta_pb = pb->add_undo_deltas();
+ auto* undo_delta_pb = pb->add_undo_deltas();
undo_delta_block.CopyToPB(undo_delta_pb->mutable_block());
}
@@ -173,33 +175,34 @@ void RowSetMetadata::ToProtobuf(RowSetDataPB *pb) {
}
}
-const std::string RowSetMetadata::ToString() const {
+string RowSetMetadata::ToString() const {
int64_t id = 0;
{
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
id = id_;
}
return Substitute("RowSet($0)", id);
}
-void RowSetMetadata::SetColumnDataBlocks(const std::map<ColumnId, BlockId>&
blocks_by_col_id) {
+void RowSetMetadata::SetColumnDataBlocks(const map<ColumnId, BlockId>&
blocks_by_col_id) {
ColumnIdToBlockIdMap new_map(blocks_by_col_id.begin(),
blocks_by_col_id.end());
new_map.shrink_to_fit();
- std::lock_guard l(lock_);
+
+ lock_guard l(lock_);
blocks_by_col_id_ = std::move(new_map);
}
void RowSetMetadata::CommitRedoDeltaDataBlock(int64_t dms_id,
int64_t num_deleted_rows,
const BlockId& block_id) {
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
last_durable_redo_dms_id_ = dms_id;
redo_delta_blocks_.push_back(block_id);
IncrementLiveRowsUnlocked(-num_deleted_rows);
}
void RowSetMetadata::CommitUndoDeltaDataBlock(const BlockId& block_id) {
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
undo_delta_blocks_.push_back(block_id);
}
@@ -207,7 +210,7 @@ void RowSetMetadata::CommitUpdate(const
RowSetMetadataUpdate& update,
BlockIdContainer* removed) {
removed->clear();
{
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
// Find the exact sequence of blocks to remove.
for (const auto& rep : update.replace_redo_blocks_) {
@@ -292,12 +295,12 @@ void RowSetMetadata::IncrementLiveRowsUnlocked(int64_t
row_count) {
}
void RowSetMetadata::IncrementLiveRows(int64_t row_count) {
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
IncrementLiveRowsUnlocked(row_count);
}
int64_t RowSetMetadata::live_row_count() const {
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
DCHECK_GE(live_row_count_, 0);
return live_row_count_;
}
@@ -305,7 +308,7 @@ int64_t RowSetMetadata::live_row_count() const {
BlockIdContainer RowSetMetadata::GetAllBlocks() const {
BlockIdContainer blocks;
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
blocks.reserve(blocks_by_col_id_.size() +
undo_delta_blocks_.size() +
redo_delta_blocks_.size() +
@@ -328,7 +331,7 @@ BlockIdContainer RowSetMetadata::GetAllBlocks() const {
BlockId RowSetMetadata::GetMaxLiveBlockId() const {
BlockId max_block_id;
- std::lock_guard l(lock_);
+ lock_guard l(lock_);
if (!adhoc_index_block_.IsNull()) {
max_block_id = std::max(max_block_id, adhoc_index_block_);
}
@@ -347,12 +350,6 @@ BlockId RowSetMetadata::GetMaxLiveBlockId() const {
return max_block_id;
}
-RowSetMetadataUpdate::RowSetMetadataUpdate() {
-}
-
-RowSetMetadataUpdate::~RowSetMetadataUpdate() {
-}
-
RowSetMetadataUpdate& RowSetMetadataUpdate::ReplaceColumnId(ColumnId col_id,
const BlockId&
block_id) {
InsertOrDie(&cols_to_replace_, col_id, block_id);
@@ -365,8 +362,8 @@ RowSetMetadataUpdate&
RowSetMetadataUpdate::RemoveColumnId(ColumnId col_id) {
}
RowSetMetadataUpdate& RowSetMetadataUpdate::ReplaceRedoDeltaBlocks(
- const std::vector<BlockId>& to_remove,
- const std::vector<BlockId>& to_add) {
+ const vector<BlockId>& to_remove,
+ const vector<BlockId>& to_add) {
ReplaceDeltaBlocks rdb = { to_remove, to_add };
replace_redo_blocks_.push_back(rdb);
@@ -374,7 +371,7 @@ RowSetMetadataUpdate&
RowSetMetadataUpdate::ReplaceRedoDeltaBlocks(
}
RowSetMetadataUpdate& RowSetMetadataUpdate::RemoveUndoDeltaBlocks(
- const std::vector<BlockId>& to_remove) {
+ const vector<BlockId>& to_remove) {
remove_undo_blocks_.insert(remove_undo_blocks_.end(), to_remove.begin(),
to_remove.end());
return *this;
}
diff --git a/src/kudu/tablet/rowset_metadata.h
b/src/kudu/tablet/rowset_metadata.h
index 922270cac..1d2d8e2de 100644
--- a/src/kudu/tablet/rowset_metadata.h
+++ b/src/kudu/tablet/rowset_metadata.h
@@ -68,7 +68,7 @@ class RowSetMetadataUpdate;
// 1) remove old data files from disk
// 2) remove log anchors corresponding to previously in-memory data
//
-class RowSetMetadata {
+class RowSetMetadata final {
public:
// We use a flat_map to save memory, since there are lots of these metadata
// objects.
@@ -95,14 +95,14 @@ class RowSetMetadata {
void AddOrphanedBlocks(const BlockIdContainer& blocks);
- const std::string ToString() const;
+ std::string ToString() const;
int64_t id() const {
std::lock_guard l(lock_);
return id_;
}
- const SchemaPtr tablet_schema() const {
+ SchemaPtr tablet_schema() const {
return tablet_metadata_->schema();
}
@@ -194,7 +194,7 @@ class RowSetMetadata {
return undo_delta_blocks_;
}
- TabletMetadata *tablet_metadata() const { return tablet_metadata_; }
+ const TabletMetadata* tablet_metadata() const { return tablet_metadata_; }
int64_t last_durable_redo_dms_id() const {
std::lock_guard l(lock_);
@@ -209,7 +209,9 @@ class RowSetMetadata {
bool HasDataForColumnIdForTests(ColumnId col_id) const {
BlockId b;
std::lock_guard l(lock_);
- if (!FindCopy(blocks_by_col_id_, col_id, &b)) return false;
+ if (!FindCopy(blocks_by_col_id_, col_id, &b)) {
+ return false;
+ }
return fs_manager()->BlockExists(b);
}
@@ -218,7 +220,8 @@ class RowSetMetadata {
return !bloom_block_.IsNull() && fs_manager()->BlockExists(bloom_block_);
}
- FsManager *fs_manager() const { return tablet_metadata_->fs_manager(); }
+ const FsManager* fs_manager() const { return tablet_metadata_->fs_manager();
}
+ FsManager* fs_manager() { return tablet_metadata_->fs_manager(); }
// Atomically commit a set of changes to this object.
//
@@ -227,7 +230,7 @@ class RowSetMetadata {
void CommitUpdate(const RowSetMetadataUpdate& update,
BlockIdContainer* removed);
- void ToProtobuf(RowSetDataPB *pb);
+ void ToProtobuf(RowSetDataPB* pb) const;
BlockIdContainer GetAllBlocks() const;
@@ -245,22 +248,19 @@ class RowSetMetadata {
private:
friend class TabletMetadata;
- typedef simple_spinlock LockType;
-
- explicit RowSetMetadata(TabletMetadata *tablet_metadata)
- : tablet_metadata_(tablet_metadata),
- initted_(false),
- last_durable_redo_dms_id_(kNoDurableMemStore),
- live_row_count_(0) {
+ explicit RowSetMetadata(TabletMetadata* tablet_metadata)
+ : tablet_metadata_(tablet_metadata),
+ initted_(false),
+ last_durable_redo_dms_id_(kNoDurableMemStore),
+ live_row_count_(0) {
}
- RowSetMetadata(TabletMetadata *tablet_metadata,
- int64_t id)
- : tablet_metadata_(DCHECK_NOTNULL(tablet_metadata)),
- initted_(true),
- id_(id),
- last_durable_redo_dms_id_(kNoDurableMemStore),
- live_row_count_(0) {
+ RowSetMetadata(TabletMetadata* tablet_metadata, int64_t id)
+ : tablet_metadata_(DCHECK_NOTNULL(tablet_metadata)),
+ initted_(true),
+ id_(id),
+ last_durable_redo_dms_id_(kNoDurableMemStore),
+ live_row_count_(0) {
}
Status InitFromPB(const RowSetDataPB& pb);
@@ -271,7 +271,7 @@ class RowSetMetadata {
bool initted_;
// Protects the below mutable fields.
- mutable LockType lock_;
+ mutable simple_spinlock lock_;
// Rowset identifier. Set in one of the constructors, and may be also set
// via LoadFromPB().
@@ -300,10 +300,10 @@ class RowSetMetadata {
// A set up of updates to be made to a RowSetMetadata object.
// Updates can be collected here, and then atomically applied to a
RowSetMetadata
// using the CommitUpdate() function.
-class RowSetMetadataUpdate {
+class RowSetMetadataUpdate final {
public:
- RowSetMetadataUpdate();
- ~RowSetMetadataUpdate();
+ RowSetMetadataUpdate() = default;
+ ~RowSetMetadataUpdate() = default;
// Replace the subsequence of redo delta blocks with the new (compacted)
delta blocks.
// The replaced blocks must be a contiguous subsequence of the the full list,
