You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/04/08 02:45:02 UTC

[GitHub] [incubator-doris] acelyc111 opened a new pull request #3276: [refactor] A small refactor on class DataDir

acelyc111 opened a new pull request #3276: [refactor] A small refactor on class DataDir
URL: https://github.com/apache/incubator-doris/pull/3276
 
 
   main refactor points are:
   - Use a single get_absolute_tablet_path function instead of 3
     independent functions
   - Remove meaningless return value of register_tablet and deregister_tablet
   - Some typo and format

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

[GitHub] [incubator-doris] imay commented on a change in pull request #3276: [refactor] A small refactor on class DataDir

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3276: [refactor] A small refactor on class DataDir
URL: https://github.com/apache/incubator-doris/pull/3276#discussion_r405363840
 
 

 ##########
 File path: be/src/olap/data_dir.cpp
 ##########
 @@ -395,69 +395,47 @@ OLAPStatus DataDir::get_shard(uint64_t* shard) {
     return OLAP_SUCCESS;
 }
 
-OLAPStatus DataDir::register_tablet(Tablet* tablet) {
-    std::lock_guard<std::mutex> l(_mutex);
-
+void DataDir::register_tablet(Tablet* tablet) {
     TabletInfo tablet_info(tablet->tablet_id(), tablet->schema_hash(), tablet->tablet_uid());
-    _tablet_set.insert(tablet_info);
-    return OLAP_SUCCESS;
-}
 
-OLAPStatus DataDir::deregister_tablet(Tablet* tablet) {
     std::lock_guard<std::mutex> l(_mutex);
+    _tablet_set.emplace(tablet_info);
 
 Review comment:
   ```suggestion
       _tablet_set.emplace(std::move(tablet_info));
   ```

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

[GitHub] [incubator-doris] imay commented on a change in pull request #3276: [refactor] A small refactor on class DataDir

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3276: [refactor] A small refactor on class DataDir
URL: https://github.com/apache/incubator-doris/pull/3276#discussion_r405370937
 
 

 ##########
 File path: be/src/olap/data_dir.cpp
 ##########
 @@ -395,69 +395,47 @@ OLAPStatus DataDir::get_shard(uint64_t* shard) {
     return OLAP_SUCCESS;
 }
 
-OLAPStatus DataDir::register_tablet(Tablet* tablet) {
-    std::lock_guard<std::mutex> l(_mutex);
-
+void DataDir::register_tablet(Tablet* tablet) {
     TabletInfo tablet_info(tablet->tablet_id(), tablet->schema_hash(), tablet->tablet_uid());
-    _tablet_set.insert(tablet_info);
-    return OLAP_SUCCESS;
-}
 
-OLAPStatus DataDir::deregister_tablet(Tablet* tablet) {
     std::lock_guard<std::mutex> l(_mutex);
+    _tablet_set.emplace(tablet_info);
+}
 
+void DataDir::deregister_tablet(Tablet* tablet) {
     TabletInfo tablet_info(tablet->tablet_id(), tablet->schema_hash(), tablet->tablet_uid());
+
+    std::lock_guard<std::mutex> l(_mutex);
     _tablet_set.erase(tablet_info);
-    return OLAP_SUCCESS;
 }
 
 void DataDir::clear_tablets(std::vector<TabletInfo>* tablet_infos) {
-    for (auto& tablet : _tablet_set) {
-        tablet_infos->push_back(tablet);
-    }
+    std::lock_guard<std::mutex> l(_mutex);
+
+    tablet_infos->insert(tablet_infos->end(), _tablet_set.begin(), _tablet_set.end());
     _tablet_set.clear();
 }
 
 std::string DataDir::get_absolute_shard_path(const std::string& shard_string) {
     return _path + DATA_PREFIX + "/" + shard_string;
 }
 
-std::string DataDir::get_absolute_tablet_path(TabletMeta* tablet_meta, bool with_schema_hash) {
+template <typename T>
+std::string DataDir::get_absolute_tablet_path(const T& tablet_meta, bool with_schema_hash) {
 
 Review comment:
   I think it will be better to have a function with argument shard_id, tablet_id and schema_hash. Then there won't be the following functions which do the same thing.
   And the string `operator +` performance is bad, prefer strings::Substitute.

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

[GitHub] [incubator-doris] imay merged pull request #3276: [refactor] A small refactor on class DataDir

Posted by GitBox <gi...@apache.org>.
imay merged pull request #3276: [refactor] A small refactor on class DataDir
URL: https://github.com/apache/incubator-doris/pull/3276
 
 
   

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

[GitHub] [incubator-doris] imay commented on issue #3276: [refactor] A small refactor on class DataDir

Posted by GitBox <gi...@apache.org>.
imay commented on issue #3276: [refactor] A small refactor on class DataDir
URL: https://github.com/apache/incubator-doris/pull/3276#issuecomment-611624078
 
 
   The UT is successful.
   ![image](https://user-images.githubusercontent.com/1249159/78918226-a93fdb80-7ac2-11ea-9105-e80a9aedcc3a.png)
   I will merge this PR

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