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

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

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

 ##########
 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:
   Can you give the example code? 
   I am not sure whether it is safe to iterate a map without read lock.

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