github-actions[bot] commented on code in PR #41932:
URL: https://github.com/apache/doris/pull/41932#discussion_r1805784362


##########
be/src/olap/base_tablet.cpp:
##########
@@ -1344,7 +1356,10 @@ Status 
BaseTablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap
 }
 
 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:1372:** +1
   ```cpp
               txn_info->partial_update_info && 
txn_info->partial_update_info->is_partial_update();
                                             ^
   ```
   **be/src/olap/base_tablet.cpp:1373:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (is_partial_update) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1374:** 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:1374:** +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:1386:** +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:1386:** +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:1392:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
           if (self->tablet_state() == TABLET_NOTREADY) {
           ^
   ```
   **be/src/olap/base_tablet.cpp:1397:** +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:1397:** +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:1412:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (non_visible_rowsets != nullptr) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1424:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (is_partial_update) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1437:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           if (!rowsets_skip_alignment.empty()) {
           ^
   ```
   **be/src/olap/base_tablet.cpp:1442:** +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:1442:** +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:1444:** +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:1447:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               if (block_dp) {
               ^
   ```
   **be/src/olap/base_tablet.cpp:1449:** +5, including nesting penalty of 4, 
nesting level increased to 5
   ```cpp
                   if (wait_token != token) {
                   ^
   ```
   **be/src/olap/base_tablet.cpp:1457:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (!rowsets_skip_alignment.empty()) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1460:** +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:1460:** +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:1463:** +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:1463:** +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:1468:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (segments.size() <= 1) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1469:** +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:1469:** +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:1473:** +1, nesting level increased to 1
   ```cpp
       } else {
         ^
   ```
   **be/src/olap/base_tablet.cpp:1475:** +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:1475:** +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:1478:** +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:1478:** +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:1486:** +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:1486:** +1
   ```cpp
       if (config::enable_merge_on_write_correctness_check && 
rowset->num_rows() != 0) {
                                                           ^
   ```
   **be/src/olap/base_tablet.cpp:1491:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           if (!st.ok()) {
           ^
   ```
   **be/src/olap/base_tablet.cpp:1496:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (transient_rs_writer) {
       ^
   ```
   **be/src/olap/base_tablet.cpp:1498:** +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:1498:** +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:1499:** +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:1507:** +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:1507:** +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:1509:** +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:1509:** +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:1526:** 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:1528:** +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:1528:** +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/cloud/cloud_engine_calc_delete_bitmap_task.cpp:
##########
@@ -189,6 +190,60 @@ Status CloudTabletCalcDeleteBitmapTask::handle() const {
         return error_st;
     }
 
+    int64_t t3 = MonotonicMicros();
+    Status status;
+    if (_sub_txn_ids.empty()) {
+        status = _handle_rowset(tablet, _version);
+    } else {
+        std::stringstream ss;
+        for (const auto& sub_txn_id : _sub_txn_ids) {
+            ss << sub_txn_id << ", ";
+        }
+        LOG(INFO) << "start calc delete bitmap for txn_id=" << _transaction_id 
<< ", sub_txn_ids=["
+                  << ss.str() << "], table_id=" << tablet->table_id()
+                  << ", partition_id=" << tablet->partition_id() << ", 
tablet_id=" << _tablet_id
+                  << ", start_version=" << _version;
+        std::vector<RowsetSharedPtr> non_visible_rowsets;
+        DeleteBitmapPtr tablet_delete_bitmap =
+                
std::make_shared<DeleteBitmap>(tablet->tablet_meta()->delete_bitmap());
+        for (int i = 0; i < _sub_txn_ids.size(); ++i) {
+            int64_t sub_txn_id = _sub_txn_ids[i];
+            int64_t version = _version + i;
+            LOG(INFO) << "start calc delete bitmap for txn_id=" << 
_transaction_id
+                      << ", sub_txn_id=" << sub_txn_id << ", table_id=" << 
tablet->table_id()
+                      << ", partition_id=" << tablet->partition_id() << ", 
tablet_id=" << _tablet_id
+                      << ", start_version=" << _version << ", cur_version=" << 
version;
+            status = _handle_rowset(tablet, version, sub_txn_id, 
&non_visible_rowsets,
+                                    tablet_delete_bitmap);
+            if (!status.ok()) {
+                LOG(INFO) << "failed to calculate delete bitmap on tablet"
+                          << ", table_id=" << tablet->table_id()
+                          << ", transaction_id=" << _transaction_id << ", 
sub_txn_id=" << sub_txn_id
+                          << ", tablet_id=" << tablet->tablet_id() << ", start 
version=" << _version
+                          << ", cur_version=" << version << ", status=" << 
status;
+                return status;
+            }
+            DCHECK(non_visible_rowsets.size() == i + 1);
+        }
+    }
+    auto total_update_delete_bitmap_time_us = MonotonicMicros() - t3;
+    LOG(INFO) << "calculate delete bitmap successfully on tablet"
+              << ", table_id=" << tablet->table_id() << ", transaction_id=" << 
_transaction_id
+              << ", tablet_id=" << tablet->tablet_id()
+              << ", get_tablet_time_us=" << get_tablet_time_us
+              << ", sync_rowset_time_us=" << sync_rowset_time_us
+              << ", total_update_delete_bitmap_time_us=" << 
total_update_delete_bitmap_time_us
+              << ", res=" << status;
+    return status;
+}
+
+Status CloudTabletCalcDeleteBitmapTask::_handle_rowset(

Review Comment:
   warning: function '_handle_rowset' exceeds recommended size/complexity 
thresholds [readability-function-size]
   ```cpp
   Status CloudTabletCalcDeleteBitmapTask::_handle_rowset(
                                           ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/cloud/cloud_engine_calc_delete_bitmap_task.cpp:239:** 81 lines 
including whitespace and comments (threshold 80)
   ```cpp
   Status CloudTabletCalcDeleteBitmapTask::_handle_rowset(
                                           ^
   ```
   
   </details>
   



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to