[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3294: Use read lock when iterate tablet_map in TabletManager::start_trash_sweep

[GitBox <gi...@apache.org>]

chaoyli commented on a change in pull request #3294: Use read lock when iterate tablet_map in TabletManager::start_trash_sweep
URL: https://github.com/apache/incubator-doris/pull/3294#discussion_r406658150
 
 

 ##########
 File path: be/src/olap/tablet_manager.cpp
 ##########
 @@ -960,28 +960,36 @@ OLAPStatus TabletManager::start_trash_sweep() {
     {
         std::vector<int64_t> tablets_to_clean;
         for (int32 i = 0; i < _tablet_map_lock_shard_size; i++) {
-            WriteLock rlock(&_tablet_map_lock_array[i]);
             tablet_map_t& tablet_map = _tablet_map_array[i];
-            for (auto& item : tablet_map) {
-                // try to clean empty item
-                if (item.second.table_arr.empty()) {
-                    // try to get schema change lock if could get schema change lock, then nobody
-                    // own the lock could remove the item
-                    // it will core if schema change thread may hold the lock and this thread will deconstruct lock
-                    if (item.second.schema_change_lock.trylock() == OLAP_SUCCESS) {
-                        item.second.schema_change_lock.unlock();
+            {
+                ReadLock r_lock(&_tablet_map_lock_array[i]);
+                for (auto& item : tablet_map) {
+                    // try to clean empty item
+                    if (item.second.table_arr.empty()) {
                         tablets_to_clean.push_back(item.first);
                     }
-                }
-                for (TabletSharedPtr tablet : item.second.table_arr) {
-                    tablet->delete_expired_inc_rowsets();
+                    for (TabletSharedPtr tablet : item.second.table_arr) {
 
 Review comment:
   You can put tablet->delete_expired_inc_rowsets() between the ReadLock and WriteLock.
   This is bottleneck which consumes much resource.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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