github-actions[bot] commented on code in PR #43011:
URL: https://github.com/apache/doris/pull/43011#discussion_r1824157007
##########
be/src/olap/base_tablet.cpp:
##########
@@ -1344,7 +1394,10 @@
}
Status BaseTablet::update_delete_bitmap(const BaseTabletSPtr& self,
TabletTxnInfo* txn_info,
Review Comment:
warning: function 'update_delete_bitmap' has cognitive complexity of 86
(threshold 50) [readability-function-cognitive-complexity]
```cpp
Status BaseTablet::update_delete_bitmap(const BaseTabletSPtr& self,
TabletTxnInfo* txn_info,
^
```
<details>
<summary>Additional context</summary>
**be/src/olap/base_tablet.cpp:1410:** +1
```cpp
txn_info->partial_update_info &&
txn_info->partial_update_info->is_partial_update();
^
```
**be/src/olap/base_tablet.cpp:1411:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (is_partial_update) {
^
```
**be/src/olap/base_tablet.cpp:1412:** nesting level increased to 2
```cpp
transient_rs_writer = DORIS_TRY(self->create_transient_rowset_writer(
^
```
**be/src/common/status.h:708:** expanded from macro 'DORIS_TRY'
```cpp
({ \
^
```
**be/src/olap/base_tablet.cpp:1412:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
transient_rs_writer = DORIS_TRY(self->create_transient_rowset_writer(
^
```
**be/src/common/status.h:711:** expanded from macro 'DORIS_TRY'
```cpp
if (!res.has_value()) [[unlikely]] { \
^
```
**be/src/olap/base_tablet.cpp:1424:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
RETURN_IF_ERROR(std::dynamic_pointer_cast<BetaRowset>(rowset)->load_segments(&segments));
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1424:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(std::dynamic_pointer_cast<BetaRowset>(rowset)->load_segments(&segments));
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1430:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (self->tablet_state() == TABLET_NOTREADY) {
^
```
**be/src/olap/base_tablet.cpp:1435:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
RETURN_IF_ERROR(self->get_all_rs_id_unlocked(next_visible_version -
1, &cur_rowset_ids));
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1435:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(self->get_all_rs_id_unlocked(next_visible_version -
1, &cur_rowset_ids));
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1450:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (non_visible_rowsets != nullptr) {
^
```
**be/src/olap/base_tablet.cpp:1455:** nesting level increased to 2
```cpp
[](RowsetSharedPtr& lhs, RowsetSharedPtr& rhs) {
^
```
**be/src/olap/base_tablet.cpp:1460:** nesting level increased to 1
```cpp
[](RowsetSharedPtr& lhs, RowsetSharedPtr& rhs) {
^
```
**be/src/olap/base_tablet.cpp:1470:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (is_partial_update) {
^
```
**be/src/olap/base_tablet.cpp:1483:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (!rowsets_skip_alignment.empty()) {
^
```
**be/src/olap/base_tablet.cpp:1488:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
DBUG_EXECUTE_IF("BaseTablet::update_delete_bitmap.enable_spin_wait", {
^
```
**be/src/util/debug_points.h:36:** expanded from macro 'DBUG_EXECUTE_IF'
```cpp
if (UNLIKELY(config::enable_debug_points)) {
\
^
```
**be/src/olap/base_tablet.cpp:1488:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
DBUG_EXECUTE_IF("BaseTablet::update_delete_bitmap.enable_spin_wait", {
^
```
**be/src/util/debug_points.h:38:** expanded from macro 'DBUG_EXECUTE_IF'
```cpp
if (dp) {
\
^
```
**be/src/olap/base_tablet.cpp:1490:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
while
(DebugPoints::instance()->is_enable("BaseTablet::update_delete_bitmap.block")) {
^
```
**be/src/olap/base_tablet.cpp:1493:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (block_dp) {
^
```
**be/src/olap/base_tablet.cpp:1495:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if (wait_token != token) {
^
```
**be/src/olap/base_tablet.cpp:1503:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (!rowsets_skip_alignment.empty()) {
^
```
**be/src/olap/base_tablet.cpp:1506:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
rowsets_skip_alignment,
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1506:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
rowsets_skip_alignment,
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1509:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(token->wait());
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1509:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(token->wait());
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1514:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (segments.size() <= 1) {
^
```
**be/src/olap/base_tablet.cpp:1515:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
specified_rowsets, delete_bitmap,
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1515:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
specified_rowsets, delete_bitmap,
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1519:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/olap/base_tablet.cpp:1521:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
specified_rowsets, delete_bitmap,
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1521:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(calc_delete_bitmap(self, rowset, segments,
specified_rowsets, delete_bitmap,
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1524:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(token->wait());
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1524:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(token->wait());
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1532:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (config::enable_merge_on_write_correctness_check &&
rowset->num_rows() != 0) {
^
```
**be/src/olap/base_tablet.cpp:1532:** +1
```cpp
if (config::enable_merge_on_write_correctness_check &&
rowset->num_rows() != 0) {
^
```
**be/src/olap/base_tablet.cpp:1537:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (!st.ok()) {
^
```
**be/src/olap/base_tablet.cpp:1542:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (transient_rs_writer) {
^
```
**be/src/olap/base_tablet.cpp:1544:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
DBUG_EXECUTE_IF("Tablet.update_delete_bitmap.partial_update_write_rowset_fail",
{
^
```
**be/src/util/debug_points.h:36:** expanded from macro 'DBUG_EXECUTE_IF'
```cpp
if (UNLIKELY(config::enable_debug_points)) {
\
^
```
**be/src/olap/base_tablet.cpp:1544:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
DBUG_EXECUTE_IF("Tablet.update_delete_bitmap.partial_update_write_rowset_fail",
{
^
```
**be/src/util/debug_points.h:38:** expanded from macro 'DBUG_EXECUTE_IF'
```cpp
if (dp) {
\
^
```
**be/src/olap/base_tablet.cpp:1545:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (rand() % 100 < (100 * dp->param("percent", 0.5))) {
^
```
**be/src/olap/base_tablet.cpp:1553:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(transient_rs_writer->flush());
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1553:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(transient_rs_writer->flush());
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1555:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(transient_rs_writer->build(transient_rowset));
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1555:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(transient_rs_writer->build(transient_rowset));
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/base_tablet.cpp:1572:** nesting level increased to 1
```cpp
[](size_t sum, const segment_v2::SegmentSharedPtr& s) { return
sum += s->num_rows(); });
^
```
**be/src/olap/base_tablet.cpp:1574:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
RETURN_IF_ERROR(self->save_delete_bitmap(txn_info, txn_id, delete_bitmap,
^
```
**be/src/common/status.h:629:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/base_tablet.cpp:1574:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(self->save_delete_bitmap(txn_info, txn_id, delete_bitmap,
^
```
**be/src/common/status.h:631:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
##########
be/src/olap/base_tablet.cpp:
##########
@@ -128,6 +128,44 @@ BaseTablet::~BaseTablet() {
g_total_tablet_num << -1;
}
+Status BaseTablet::capture_sub_txn_rs_readers(
Review Comment:
warning: method 'capture_sub_txn_rs_readers' can be made const
[readability-make-member-function-const]
be/src/olap/base_tablet.cpp:133:
```diff
- std::vector<std::shared_ptr<TabletTxnInfo>>* tablet_txn_infos) {
+ std::vector<std::shared_ptr<TabletTxnInfo>>* tablet_txn_infos)
const {
```
##########
be/src/olap/base_tablet.cpp:
##########
@@ -1505,7 +1573,210 @@
[](size_t sum, const segment_v2::SegmentSharedPtr& s) { return sum
+= s->num_rows(); });
auto t5 = watch.get_elapse_time_us();
RETURN_IF_ERROR(self->save_delete_bitmap(txn_info, txn_id, delete_bitmap,
- transient_rs_writer.get(),
cur_rowset_ids));
+ transient_rs_writer.get(),
cur_rowset_ids,
+ base_txn_id /* lock_id */));
+ LOG(INFO) << "[Publish] construct delete bitmap tablet: " <<
self->tablet_id()
+ << ", rowset_ids to add: " << rowset_ids_to_add.size()
+ << ", rowset_ids to del: " << rowset_ids_to_del.size()
+ << ", cur version: " << cur_version << ", transaction_id: " <<
txn_id << ","
+ << ss.str() << " , total rows: " << total_rows
+ << ", update delete_bitmap cost: " << watch.get_elapse_time_us()
- t5 << "(us)";
+ return Status::OK();
+}
+
+Status BaseTablet::update_delete_bitmap(const BaseTabletSPtr& self,
TabletTxnInfo* txn_info,
+ int64_t txn_id, int64_t
txn_expiration) {
+ return update_delete_bitmap(self, txn_info, txn_id, txn_expiration,
nullptr, -1,
+ txn_info->rowset->start_version(), nullptr);
+}
+
+Status BaseTablet::txn_load_update_delete_bitmap(
+ const BaseTabletSPtr& self, const std::vector<RowsetSharedPtr>&
visible_rowsets,
+ const std::vector<RowsetSharedPtr>& all_non_visible_rowsets, int64_t
visible_version,
+ const std::vector<int64_t>& sub_txn_ids,
+ std::vector<std::shared_ptr<TabletTxnInfo>>& tablet_txn_infos,
+ DeleteBitmapPtr tablet_delete_bitmap) {
+ // calculate delete bitmap of sub txn rowsets
+ for (auto i = 0; i < tablet_txn_infos.size(); ++i) {
+ auto& tablet_txn_info = tablet_txn_infos[i];
+ auto sub_txn_id = sub_txn_ids[i];
+ if (tablet_txn_info->rowset->rowset_meta()->has_delete_predicate()) {
+ continue;
+ }
+ std::vector<RowsetSharedPtr> non_visible_rowsets;
+ for (auto j = 0; j < i; ++j) {
+ auto rowset = all_non_visible_rowsets[j];
+ non_visible_rowsets.push_back(rowset);
+ }
+ tablet_txn_info->delete_bitmap->delete_bitmap.clear();
+ tablet_txn_info->rowset_ids.clear();
+ // TODO the version is set when capture?
+ int64_t tmp_version = visible_version + i + 1;
+ RETURN_IF_ERROR(self->txn_load_update_delete_bitmap(
+ self, tablet_txn_info.get(), sub_txn_id, -1, visible_rowsets,
non_visible_rowsets,
+ tablet_delete_bitmap));
+ // merge delete bitmap of sub txn rowsets
+ auto& dm = tablet_txn_info->delete_bitmap->delete_bitmap;
+ for (auto it = dm.begin(); it != dm.end(); ++it) {
+ if (std::get<1>(it->first) != DeleteBitmap::INVALID_SEGMENT_ID) {
+ tablet_delete_bitmap->merge(
+ {std::get<0>(it->first), std::get<1>(it->first),
tmp_version}, it->second);
+ }
+ }
+ }
+ return Status::OK();
+}
+
+Status BaseTablet::txn_load_update_delete_bitmap(
+ const BaseTabletSPtr& self, TabletTxnInfo* txn_info, int64_t txn_id,
int64_t txn_expiration,
+ const std::vector<RowsetSharedPtr>& visible_rowsets,
+ const std::vector<RowsetSharedPtr>& non_visible_rowsets,
Review Comment:
warning: function 'txn_load_update_delete_bitmap' exceeds recommended
size/complexity thresholds [readability-function-size]
```cpp
Status BaseTablet::txn_load_update_delete_bitmap(
^
```
<details>
<summary>Additional context</summary>
**be/src/olap/base_tablet.cpp:1632:** 154 lines including whitespace and
comments (threshold 80)
```cpp
Status BaseTablet::txn_load_update_delete_bitmap(
^
```
</details>
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -1410,6 +1410,108 @@ static bool try_fetch_and_parse_schema(Transaction*
txn, RowsetMetaCloudPB& rows
return true;
}
+void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController*
controller,
Review Comment:
warning: function 'get_tmp_rowset' exceeds recommended size/complexity
thresholds [readability-function-size]
```cpp
void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController*
controller,
^
```
<details>
<summary>Additional context</summary>
**cloud/src/meta-service/meta_service.cpp:1412:** 97 lines including
whitespace and comments (threshold 80)
```cpp
void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController*
controller,
^
```
</details>
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -1410,6 +1410,108 @@
return true;
}
+void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController*
controller,
+ const GetTmpRowsetRequest* request,
+ GetTmpRowsetResponse* response,
+ ::google::protobuf::Closure* done) {
+ RPC_PREPROCESS(get_tmp_rowset);
+ instance_id = get_instance_id(resource_mgr_, request->cloud_unique_id());
+ if (instance_id.empty()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty instance_id";
+ LOG(INFO) << msg << ", cloud_unique_id=" << request->cloud_unique_id();
+ return;
+ }
+ RPC_RATE_LIMIT(get_tmp_rowset)
+ if (!request->has_index_id()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty index id";
+ return;
+ }
+ if (!request->has_tablet_id()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty tablet id";
+ return;
+ }
+ if (request->txn_ids().empty()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty txn ids";
+ return;
+ }
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv_->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ code = cast_as<ErrCategory::CREATE>(err);
+ msg = "failed to create txn";
+ return;
+ }
+ std::vector<std::string> tmp_rowset_keys;
+ std::vector<std::optional<std::string>> tmp_rowset_values;
+ tmp_rowset_keys.reserve(request->txn_ids().size());
+ tmp_rowset_values.reserve(request->txn_ids().size());
+ int64_t tablet_id = request->tablet_id();
+ int64_t index_id = request->index_id();
+ // TODO avoid too many txn ids, should limit in fe
+ for (const auto& txn_id : request->txn_ids()) {
+ tmp_rowset_keys.push_back(meta_rowset_tmp_key({instance_id, txn_id,
tablet_id}));
+ }
+ err = txn->batch_get(&tmp_rowset_values, tmp_rowset_keys);
+ if (err != TxnErrorCode::TXN_OK) {
+ code = cast_as<ErrCategory::READ>(err);
+ ss << "failed to batch get tmp rowset, instance_id=" << instance_id
+ << " tablet_id=" << tablet_id << " err=" << err;
+ msg = ss.str();
+ LOG(WARNING) << msg;
+ return;
+ }
+ // get referenced schema
+ std::unordered_map<int32_t, doris::TabletSchemaCloudPB*> version_to_schema;
+ for (size_t i = 0; i < tmp_rowset_keys.size(); ++i) {
+ if (!tmp_rowset_values[i].has_value()) [[unlikely]] {
+ code = MetaServiceCode::KV_TXN_GET_ERR;
+ ss << "failed to get tmp rowset, err=not found, instance_id=" <<
instance_id
+ << " tablet_id=" << tablet_id << " txn_id=" <<
request->txn_ids(i)
+ << " key=" << hex(tmp_rowset_keys[i]);
+ msg = ss.str();
+ LOG(WARNING) << msg;
+ return;
+ }
+ auto rowset_meta = response->add_rowset_meta();
+ if (!rowset_meta->ParseFromArray(tmp_rowset_values[i]->data(),
+ tmp_rowset_values[i]->size())) {
+ code = MetaServiceCode::PROTOBUF_PARSE_ERR;
+ ss << "malformed rowset meta value, instance_id=" << instance_id
+ << " tablet_id=" << tablet_id << " txn_id=" <<
request->txn_ids(i)
+ << " key=" << hex(tmp_rowset_keys[i]);
+ msg = ss.str();
+ LOG(WARNING) << msg;
+ return;
+ }
+ // set tablet schema
+ if (rowset_meta->has_tablet_schema()) continue;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (rowset_meta->has_tablet_schema()) { continue;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
