Check deserialize result when save meta

be/src/olap/tablet.cpp

@@ -190,7 +190,7 @@ OLAPStatus Tablet::revise_tablet_meta(
     do {
         // load new local tablet_meta to operate on
         TabletMetaSharedPtr new_tablet_meta(new (nothrow) TabletMeta());
-        TabletMetaManager::get_header(_data_dir, tablet_id(), schema_hash(), new_tablet_meta);
+        RETURN_NOT_OK(TabletMetaManager::get_header(_data_dir, tablet_id(), schema_hash(), new_tablet_meta));
 
         // delete versions from new local tablet_meta
         for (const Version& version : versions_to_delete) {
@@ -222,7 +222,7 @@ OLAPStatus Tablet::revise_tablet_meta(
        VLOG(3) << "load rowsets successfully when clone. tablet=" << full_name()
                 << ", added rowset size=" << rowsets_to_clone.size();
         // save and reload tablet_meta
-        res = TabletMetaManager::save(_data_dir, tablet_id(), schema_hash(), new_tablet_meta);
+        res = new_tablet_meta->save_meta(_data_dir);
         if (res != OLAP_SUCCESS) {
             LOG(WARNING) << "failed to save new local tablet_meta when clone. res:" << res;
             break;

be/src/olap/tablet.h

@@ -238,6 +238,7 @@ class Tablet : public std::enable_shared_from_this<Tablet> {
     Mutex _ingest_lock;
     Mutex _base_lock;
     Mutex _cumulative_lock;
+    RWMutex _migration_lock;
     std::unordered_map<Version, RowsetSharedPtr, HashOfVersion> _rs_version_map;
     std::unordered_map<Version, RowsetSharedPtr, HashOfVersion> _inc_rs_version_map;
 
@@ -379,6 +380,18 @@ inline size_t Tablet::row_size() const {
     return _schema.row_size();
 }
 
+inline OLAPStatus Tablet::try_migration_rdlock() {
+    return _migration_lock.tryrdlock();
+}
+
+inline OLAPStatus Tablet::try_migration_wrlock() {
+    return _migration_lock.trywrlock();
+}
+
+inline void Tablet::release_migration_lock() {
+    _migration_lock.unlock();
+}
+
 }
 
 #endif // DORIS_BE_SRC_OLAP_TABLET_H

be/src/olap/tablet_manager.cpp

@@ -770,6 +770,20 @@ OLAPStatus TabletManager::load_tablet_from_meta(DataDir* data_dir, TTabletId tab
         return OLAP_ERR_HEADER_PB_PARSE_FAILED;
     }
 
+    // check if tablet meta is valid
+    if (tablet_meta->tablet_id() != tablet_id || tablet_meta->schema_hash() != schema_hash) {
+        LOG(WARNING) << "tablet meta load from meta is invalid"
+                   << " input tablet id=" << tablet_id
+                   << " input tablet schema_hash=" << schema_hash
+                   << " meta tablet=" << tablet_meta->full_name();
+        return OLAP_ERR_HEADER_PB_PARSE_FAILED;
+    }
+    if (tablet_meta->tablet_uid().hi == 0 && tablet_meta->tablet_uid().lo == 0) {
+        LOG(WARNING) << "not load this tablet because uid == 0"
+                  << " tablet=" << tablet_meta->full_name();
+        return OLAP_ERR_HEADER_PB_PARSE_FAILED;
+    }
+
     // init must be called
     TabletSharedPtr tablet = Tablet::create_tablet_from_meta(tablet_meta, data_dir);
     if (tablet == nullptr) {
@@ -1279,6 +1293,8 @@ OLAPStatus TabletManager::_drop_tablet_directly_unlocked(
             TabletSharedPtr tablet = *it;
             it = _tablet_map[tablet_id].table_arr.erase(it);
             if (!keep_files) {
+                // drop tablet will update tablet meta, should lock
+                WriteLock wrlock(tablet->get_header_lock_ptr()); 
                 LOG(INFO) << "set tablet to shutdown state and remove it from memory"
                           << " tablet_id=" << tablet_id
                           << " schema_hash=" << schema_hash

be/src/olap/tablet_meta.cpp

@@ -25,6 +25,7 @@
 #include "olap/rowset/alpha_rowset_meta.h"
 #include "olap/tablet_meta_manager.h"
 #include "util/uid_util.h"
+#include "util/url_coding.h"
 
 namespace doris {
 
@@ -266,19 +267,33 @@ OLAPStatus TabletMeta::_save_meta(DataDir* data_dir) {
     for (auto& rs_meta : _rs_metas) {
         if (rs_meta->rowset_id() >= _next_rowset_id) {
             LOG(FATAL) << "meta contains invalid rowsetid " 
+                       << " tablet=" << full_name() 
                        << " rowset_id=" <<  rs_meta->rowset_id()
                        << " next_rowset_id=" << _next_rowset_id;
         }
     }
     for (auto& rs_meta : _inc_rs_metas) {
         if (rs_meta->rowset_id() >= _next_rowset_id) {
             LOG(FATAL) << "meta contains invalid rowsetid " 
+                       << " tablet=" << full_name() 
                        << " rowset_id=" <<  rs_meta->rowset_id()
                        << " next_rowset_id=" << _next_rowset_id;
         }
     }
+    // check if _end_rowset_id > 10000
+    if (_end_rowset_id < 10000) {
+        LOG(FATAL) << "end_rowset_id is invalid" 
+                   << " tablet=" << full_name() 
+                   << " end_rowset_id=" << _end_rowset_id; 
+    }
+    // check if tablet uid is valid
+    if (_tablet_uid.hi == 0 && _tablet_uid.lo == 0) {
+        LOG(FATAL) << "tablet_uid is invalid" 
+                   << " tablet=" << full_name() 
+                   << " _tablet_uid=" << _tablet_uid.to_string(); 
+    }
     string meta_binary;
-    serialize(&meta_binary);
+    RETURN_NOT_OK(serialize(&meta_binary));
     OLAPStatus status = TabletMetaManager::save(data_dir, tablet_id(), schema_hash(), meta_binary);
     if (status != OLAP_SUCCESS) {
        LOG(FATAL) << "fail to save tablet_meta. status=" << status
@@ -291,7 +306,16 @@ OLAPStatus TabletMeta::_save_meta(DataDir* data_dir) {
 OLAPStatus TabletMeta::serialize(string* meta_binary) {
     TabletMetaPB tablet_meta_pb;
     RETURN_NOT_OK(to_meta_pb(&tablet_meta_pb));
-    tablet_meta_pb.SerializeToString(meta_binary);
+    bool serialize_success = tablet_meta_pb.SerializeToString(meta_binary);
+    if (!serialize_success) {
+        LOG(FATAL) << "failed to serialize meta " << full_name();
+    }
+    // deserialize the meta to check the result is correct
+    TabletMetaPB de_tablet_meta_pb;
+    bool parsed = de_tablet_meta_pb.ParseFromString(*meta_binary);
+    if (!parsed) {
+        LOG(FATAL) << "deserialize from previous serialize result failed " << full_name();
+    }
     return OLAP_SUCCESS;
 };
 
@@ -725,4 +749,12 @@ OLAPStatus TabletMeta::set_alter_state(AlterTabletState alter_state) {
     }
 }
 
+std::string TabletMeta::full_name() const {
+    std::stringstream ss;
+    ss << _tablet_id 
+       << "." << _schema_hash 
+       << "." << _tablet_uid.to_string();
+    return ss.str();
+}
+
 }  // namespace doris

be/src/olap/tablet_meta.h

@@ -187,6 +187,8 @@ class TabletMeta {
 
     RowsetId get_cur_rowset_id();
 
+    std::string full_name() const;
+
 private:
     OLAPStatus _save_meta(DataDir* data_dir);
 
@@ -210,7 +212,7 @@ class TabletMeta {
     vector<RowsetMetaSharedPtr> _inc_rs_metas;
     DelPredicateArray _del_pred_array;
     AlterTabletTaskSharedPtr _alter_task;
-    bool _in_restore_mode;
+    bool _in_restore_mode = false;
 
     RWMutex _meta_lock;
 };

be/src/olap/tablet_meta_manager.cpp

@@ -99,6 +99,13 @@ OLAPStatus TabletMetaManager::save(DataDir* store,
     std::string key = key_stream.str();
     VLOG(3) << "save tablet meta to meta store: key = " << key;
     OlapMeta* meta = store->get_meta();
+
+    TabletMetaPB de_tablet_meta_pb;
+    bool parsed = de_tablet_meta_pb.ParseFromString(meta_binary);
+    if (!parsed) {
+        LOG(FATAL) << "deserialize from previous serialize result failed";
+    }
+
     LOG(INFO) << "save tablet meta " 
               << " tablet_id=" << tablet_id
               << " schema_hash=" << schema_hash

be/src/olap/utils.h

@@ -245,14 +245,26 @@ class RWMutex {
 //
 class ReadLock {
 public:
-    explicit ReadLock(RWMutex* mutex)
-            : _mutex(mutex) {
-        this->_mutex->rdlock();
+    explicit ReadLock(RWMutex* mutex, bool try_lock = false)
+            : _mutex(mutex), own_lock(false) {
+        if (try_lock) {
+            own_lock = this->_mutex->tryrdlock() == OLAP_SUCCESS;
+        } else {
+            this->_mutex->rdlock();
+            own_lock = true;
+        }
     }
-    ~ReadLock() { this->_mutex->unlock(); }
+    ~ReadLock() { 
+        if (own_lock) {
+            this->_mutex->unlock(); 
+        }
+    }
+
+    bool has_own_lock() { return own_lock; }
 
 private:
     RWMutex* _mutex;
+    bool own_lock;
     DISALLOW_COPY_AND_ASSIGN(ReadLock);
 };
 
@@ -263,14 +275,26 @@ class ReadLock {
 //
 class WriteLock {
 public:
-    explicit WriteLock(RWMutex* mutex)
-            : _mutex(mutex) {
-        this->_mutex->wrlock();
+    explicit WriteLock(RWMutex* mutex, bool try_lock = false)
+            : _mutex(mutex), own_lock(false) {
+        if (try_lock) {
+            own_lock = this->_mutex->trywrlock() == OLAP_SUCCESS;
+        } else {
+            this->_mutex->wrlock();
+            own_lock = true;
+        }
     }
-    ~WriteLock() { this->_mutex->unlock(); }
+    ~WriteLock() { 
+        if (own_lock) {
+            this->_mutex->unlock();
+        } 
+    }
+
+    bool has_own_lock() { return own_lock; }
 
 private:
     RWMutex* _mutex;
+    bool own_lock;
     DISALLOW_COPY_AND_ASSIGN(WriteLock);
 };