zhannngchen commented on code in PR #11953:
URL: https://github.com/apache/doris/pull/11953#discussion_r951105361
##########
be/src/olap/tablet.h:
##########
@@ -392,6 +393,9 @@ class Tablet : public BaseTablet {
// TODO(lingbin): There is a _meta_lock TabletMeta too, there should be a
comment to
// explain how these two locks work together.
mutable std::shared_mutex _meta_lock;
+
+ // use to protect compaction and publish_txn update delete_bitmap
+ mutable std::shared_mutex _delete_bitmap_lock;
Review Comment:
rename to _rowset_update_mutex is better? And it should be a `mutex` rather
than a `shared_mutex`.
Also we need to add more comments on it:
> In unique key table with MoW, we should guarantee that only one writer can
update rowset and delete bitmap at the same time. We use a separate lock rather
than `_meta_lock`, to avoid blocking read queries during publish_txn, which
might take hundreds of milliseconds
##########
be/src/olap/memtable.cpp:
##########
@@ -400,20 +400,21 @@ bool MemTable::need_to_agg() {
Status MemTable::_generate_delete_bitmap() {
// generate delete bitmap, build a tmp rowset and load recent segment
- if (_tablet->enable_unique_key_merge_on_write()) {
- auto rowset = _rowset_writer->build_tmp();
- auto beta_rowset = reinterpret_cast<BetaRowset*>(rowset.get());
- std::vector<segment_v2::SegmentSharedPtr> segments;
- segment_v2::SegmentSharedPtr segment;
- if (beta_rowset->num_segments() == 0) {
- return Status::OK();
- }
- RETURN_IF_ERROR(beta_rowset->load_segment(beta_rowset->num_segments()
- 1, &segment));
- segments.push_back(segment);
- std::lock_guard<std::shared_mutex>
meta_wrlock(_tablet->get_header_lock());
- RETURN_IF_ERROR(_tablet->calc_delete_bitmap(beta_rowset->rowset_id(),
segments,
- &_rowset_ids,
_delete_bitmap));
+ if (!_tablet->enable_unique_key_merge_on_write()) {
+ return Status::OK();
+ }
+ auto rowset = _rowset_writer->build_tmp();
+ auto beta_rowset = reinterpret_cast<BetaRowset*>(rowset.get());
+ std::vector<segment_v2::SegmentSharedPtr> segments;
+ segment_v2::SegmentSharedPtr segment;
+ if (beta_rowset->num_segments() == 0) {
+ return Status::OK();
}
+ RETURN_IF_ERROR(beta_rowset->load_segment(beta_rowset->num_segments() - 1,
&segment));
+ segments.push_back(segment);
+ std::shared_lock meta_rlock(_tablet->get_header_lock());
Review Comment:
Can we move this read lock into `calc_delete_bitmap`?
--
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
