You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/26 03:06:25 UTC

[GitHub] [doris] yixiutt opened a new pull request, #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

yixiutt opened a new pull request, #11195:
URL: https://github.com/apache/doris/pull/11195

   …ublish version
   
   1.make version publish work in version order
   2.update delete bitmap while publish version, load current version rowset
   primary key and search in pre rowsets
   3.speed up publish version task by parallel tablet publish task
   
   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem Summary:
   
   Describe the overview of changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11195:
URL: https://github.com/apache/doris/pull/11195#issuecomment-1195616921

   PR approved by anyone and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] dataroaring merged pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
dataroaring merged PR #11195:
URL: https://github.com/apache/doris/pull/11195


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11195:
URL: https://github.com/apache/doris/pull/11195#issuecomment-1195616871

   PR approved by at least one committer and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] yixiutt commented on a diff in pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
yixiutt commented on code in PR #11195:
URL: https://github.com/apache/doris/pull/11195#discussion_r929819900


##########
be/src/olap/txn_manager.cpp:
##########
@@ -308,8 +309,108 @@ Status TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id,
                 _clear_txn_partition_map_unlocked(transaction_id, partition_id);
             }
         }
+    }
+    auto tablet = StorageEngine::instance()->tablet_manager()->get_tablet(tablet_id);
+#ifdef BE_TEST
+    if (tablet == nullptr) {
+        return Status::OK();
+    }
+#endif
+    // Check if have to build extra delete bitmap for table of UNIQUE_KEY model
+    if (!tablet->enable_unique_key_merge_on_write() ||
+        tablet->tablet_meta()->preferred_rowset_type() != RowsetTypePB::BETA_ROWSET ||
+        rowset_ptr->keys_type() != KeysType::UNIQUE_KEYS) {
         return Status::OK();
     }
+    CHECK(version.first == version.second) << "impossible: " << version;
+
+    // For each key in current set, check if it overwrites any previously
+    // written keys
+    OlapStopWatch watch;
+    std::vector<segment_v2::SegmentSharedPtr> segments;
+    std::vector<segment_v2::SegmentSharedPtr> pre_segments;
+    auto beta_rowset = reinterpret_cast<BetaRowset*>(rowset_ptr.get());
+    Status st = beta_rowset->load_segments(&segments);
+    if (!st.ok()) return st;
+    // lock tablet meta to modify delete bitmap
+    std::lock_guard<std::shared_mutex> meta_wrlock(tablet->get_header_lock());
+    for (auto& seg : segments) {
+        seg->load_index(); // We need index blocks to iterate
+        auto pk_idx = seg->get_primary_key_index();
+        int cnt = 0;
+        int total = pk_idx->num_rows();
+        int32_t remaining = total;
+        bool exact_match = false;
+        std::string last_key;
+        int batch_size = 1024;
+        MemPool pool;
+        while (remaining > 0) {
+            std::unique_ptr<segment_v2::IndexedColumnIterator> iter;
+            RETURN_IF_ERROR(pk_idx->new_iterator(&iter));
+
+            size_t num_to_read = std::min(batch_size, remaining);
+            std::unique_ptr<ColumnVectorBatch> cvb;
+            RETURN_IF_ERROR(ColumnVectorBatch::create(num_to_read, false, pk_idx->type_info(),
+                                                      nullptr, &cvb));
+            ColumnBlock block(cvb.get(), &pool);
+            ColumnBlockView column_block_view(&block);
+            Slice last_key_slice(last_key);
+            RETURN_IF_ERROR(iter->seek_at_or_after(&last_key_slice, &exact_match));
+
+            size_t num_read = num_to_read;
+            RETURN_IF_ERROR(iter->next_batch(&num_read, &column_block_view));
+            DCHECK(num_to_read == num_read);
+            last_key = (reinterpret_cast<const Slice*>(cvb->cell_ptr(num_read - 1)))->to_string();
+
+            // exclude last_key, last_key will be read in next batch.
+            if (num_read == batch_size && num_read != remaining) {
+                num_read -= 1;
+            }
+            for (size_t i = 0; i < num_read; i++) {
+                const Slice* key = reinterpret_cast<const Slice*>(cvb->cell_ptr(i));
+                // first check if exist in pre segment
+                bool find = _check_pk_in_pre_segments(pre_segments, *key, tablet, version);
+                if (find) {
+                    cnt++;

Review Comment:
   fixed



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] yixiutt commented on a diff in pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
yixiutt commented on code in PR #11195:
URL: https://github.com/apache/doris/pull/11195#discussion_r929817716


##########
be/src/olap/tablet.h:
##########
@@ -438,6 +438,9 @@ inline void Tablet::set_cumulative_layer_point(int64_t new_point) {
 }
 
 inline bool Tablet::enable_unique_key_merge_on_write() const {
+    if (_tablet_meta == nullptr) {

Review Comment:
   actually it's for test



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] zhannngchen commented on a diff in pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #11195:
URL: https://github.com/apache/doris/pull/11195#discussion_r929844943


##########
be/src/olap/tablet.h:
##########
@@ -438,6 +438,9 @@ inline void Tablet::set_cumulative_layer_point(int64_t new_point) {
 }
 
 inline bool Tablet::enable_unique_key_merge_on_write() const {
+    if (_tablet_meta == nullptr) {

Review Comment:
   ok



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] zhannngchen commented on a diff in pull request #11195: [feature-wip](unique-key-merge-on-write) update delete bitmap while p…

Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #11195:
URL: https://github.com/apache/doris/pull/11195#discussion_r929709212


##########
be/src/olap/task/engine_publish_version_task.cpp:
##########
@@ -86,39 +115,44 @@ Status EnginePublishVersionTask::finish() {
                 res = Status::OLAPInternalError(OLAP_ERR_PUSH_TABLE_NOT_EXIST);
                 continue;
             }
-
-            publish_status = StorageEngine::instance()->txn_manager()->publish_txn(
-                    partition_id, tablet, transaction_id, version);
-            if (publish_status != Status::OK()) {
-                LOG(WARNING) << "failed to publish version. rowset_id=" << rowset->rowset_id()
-                             << ", tablet_id=" << tablet_info.tablet_id
-                             << ", txn_id=" << transaction_id;
-                _error_tablet_ids->push_back(tablet_info.tablet_id);
-                res = publish_status;
+            Version max_version = tablet->max_version();
+            // in uniq key model with aux index, we should see all

Review Comment:
   in uniq key model with merge-on-write



##########
be/src/olap/txn_manager.cpp:
##########
@@ -308,8 +309,108 @@ Status TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id,
                 _clear_txn_partition_map_unlocked(transaction_id, partition_id);
             }
         }
+    }
+    auto tablet = StorageEngine::instance()->tablet_manager()->get_tablet(tablet_id);
+#ifdef BE_TEST
+    if (tablet == nullptr) {
+        return Status::OK();
+    }
+#endif
+    // Check if have to build extra delete bitmap for table of UNIQUE_KEY model
+    if (!tablet->enable_unique_key_merge_on_write() ||
+        tablet->tablet_meta()->preferred_rowset_type() != RowsetTypePB::BETA_ROWSET ||
+        rowset_ptr->keys_type() != KeysType::UNIQUE_KEYS) {
         return Status::OK();
     }
+    CHECK(version.first == version.second) << "impossible: " << version;
+
+    // For each key in current set, check if it overwrites any previously
+    // written keys
+    OlapStopWatch watch;
+    std::vector<segment_v2::SegmentSharedPtr> segments;
+    std::vector<segment_v2::SegmentSharedPtr> pre_segments;
+    auto beta_rowset = reinterpret_cast<BetaRowset*>(rowset_ptr.get());
+    Status st = beta_rowset->load_segments(&segments);
+    if (!st.ok()) return st;
+    // lock tablet meta to modify delete bitmap
+    std::lock_guard<std::shared_mutex> meta_wrlock(tablet->get_header_lock());
+    for (auto& seg : segments) {
+        seg->load_index(); // We need index blocks to iterate
+        auto pk_idx = seg->get_primary_key_index();
+        int cnt = 0;
+        int total = pk_idx->num_rows();
+        int32_t remaining = total;
+        bool exact_match = false;
+        std::string last_key;
+        int batch_size = 1024;
+        MemPool pool;
+        while (remaining > 0) {
+            std::unique_ptr<segment_v2::IndexedColumnIterator> iter;
+            RETURN_IF_ERROR(pk_idx->new_iterator(&iter));
+
+            size_t num_to_read = std::min(batch_size, remaining);
+            std::unique_ptr<ColumnVectorBatch> cvb;
+            RETURN_IF_ERROR(ColumnVectorBatch::create(num_to_read, false, pk_idx->type_info(),
+                                                      nullptr, &cvb));
+            ColumnBlock block(cvb.get(), &pool);
+            ColumnBlockView column_block_view(&block);
+            Slice last_key_slice(last_key);
+            RETURN_IF_ERROR(iter->seek_at_or_after(&last_key_slice, &exact_match));
+
+            size_t num_read = num_to_read;
+            RETURN_IF_ERROR(iter->next_batch(&num_read, &column_block_view));
+            DCHECK(num_to_read == num_read);
+            last_key = (reinterpret_cast<const Slice*>(cvb->cell_ptr(num_read - 1)))->to_string();
+
+            // exclude last_key, last_key will be read in next batch.
+            if (num_read == batch_size && num_read != remaining) {
+                num_read -= 1;
+            }
+            for (size_t i = 0; i < num_read; i++) {
+                const Slice* key = reinterpret_cast<const Slice*>(cvb->cell_ptr(i));
+                // first check if exist in pre segment
+                bool find = _check_pk_in_pre_segments(pre_segments, *key, tablet, version);
+                if (find) {
+                    cnt++;

Review Comment:
   we should `continue` here?



##########
be/src/olap/task/engine_publish_version_task.cpp:
##########
@@ -86,39 +115,44 @@ Status EnginePublishVersionTask::finish() {
                 res = Status::OLAPInternalError(OLAP_ERR_PUSH_TABLE_NOT_EXIST);
                 continue;
             }
-
-            publish_status = StorageEngine::instance()->txn_manager()->publish_txn(
-                    partition_id, tablet, transaction_id, version);
-            if (publish_status != Status::OK()) {
-                LOG(WARNING) << "failed to publish version. rowset_id=" << rowset->rowset_id()
-                             << ", tablet_id=" << tablet_info.tablet_id
-                             << ", txn_id=" << transaction_id;
-                _error_tablet_ids->push_back(tablet_info.tablet_id);
-                res = publish_status;
+            Version max_version = tablet->max_version();
+            // in uniq key model with aux index, we should see all
+            // previous version when update delete bitmap, so add a check
+            // here and wait pre version publish or lock timeout
+            if (tablet->keys_type() == KeysType::UNIQUE_KEYS &&
+                tablet->enable_unique_key_merge_on_write() &&
+                version.first != max_version.second + 1) {
+                LOG(INFO) << "uniq key with aux index version not continuous, current max version="

Review Comment:
   uniq key with merge-on-write



##########
be/src/olap/tablet.h:
##########
@@ -438,6 +438,9 @@ inline void Tablet::set_cumulative_layer_point(int64_t new_point) {
 }
 
 inline bool Tablet::enable_unique_key_merge_on_write() const {
+    if (_tablet_meta == nullptr) {

Review Comment:
   It should not happen? `_tablet_meta` is assigned in ctor.



-- 
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-unsubscribe@doris.apache.org

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


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