yiguolei commented on code in PR #21969:
URL: https://github.com/apache/doris/pull/21969#discussion_r1270844337


##########
be/src/olap/tablet_manager.cpp:
##########
@@ -244,7 +244,18 @@ Status TabletManager::create_tablet(const 
TCreateTabletReq& request, std::vector
     int64_t tablet_id = request.tablet_id;
     LOG(INFO) << "begin to create tablet. tablet_id=" << tablet_id;
 
-    std::lock_guard<std::shared_mutex> 
wrlock(_get_tablets_shard_lock(tablet_id));
+    // when we create rollup tablet A(assume on shard-1) from tablet B(assume 
on shard-2)
+    // we need use write lock on shard-1 and then use read lock on shard-2
+    // if there have create rollup tablet C(assume on shard-2) from tablet 
D(assume on shard-1) at the same time, we will meet deadlock
+    std::unique_lock two_tablet_lock(_two_tablet_mtx, std::defer_lock);
+    bool is_schema_change = request.__isset.base_tablet_id && 
request.base_tablet_id > 0;
+    bool need_two_lock = is_schema_change && ((_tablets_shards_mask & 
request.base_tablet_id) !=
+                                              (_tablets_shards_mask & 
tablet_id));
+    if (need_two_lock) {
+        two_tablet_lock.lock();

Review Comment:
   这样会导致并发度很低,tablet map lock 分shard 就没啥意义了。 相当于建rollup 变成顺序的了



##########
be/src/olap/tablet_manager.cpp:
##########
@@ -244,7 +244,18 @@ Status TabletManager::create_tablet(const 
TCreateTabletReq& request, std::vector
     int64_t tablet_id = request.tablet_id;
     LOG(INFO) << "begin to create tablet. tablet_id=" << tablet_id;
 
-    std::lock_guard<std::shared_mutex> 
wrlock(_get_tablets_shard_lock(tablet_id));
+    // when we create rollup tablet A(assume on shard-1) from tablet B(assume 
on shard-2)
+    // we need use write lock on shard-1 and then use read lock on shard-2
+    // if there have create rollup tablet C(assume on shard-2) from tablet 
D(assume on shard-1) at the same time, we will meet deadlock
+    std::unique_lock two_tablet_lock(_two_tablet_mtx, std::defer_lock);
+    bool is_schema_change = request.__isset.base_tablet_id && 
request.base_tablet_id > 0;
+    bool need_two_lock = is_schema_change && ((_tablets_shards_mask & 
request.base_tablet_id) !=
+                                              (_tablets_shards_mask & 
tablet_id));
+    if (need_two_lock) {
+        two_tablet_lock.lock();

Review Comment:
   这样会导致并发度很低,tablet map lock 分shard 就没啥意义了。 相当于建rollup 变成顺序的了



-- 
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