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/07/02 16:19:11 UTC

[GitHub] [incubator-doris] liutang123 opened a new pull request #4011: Remove rs when version conflict when insert to a tablet

liutang123 opened a new pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011


   For #3976 
   In this PR, when insert to a tablet, if the tablet has a different rs with the same version, remove it.


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



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


[GitHub] [incubator-doris] liutang123 commented on pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
liutang123 commented on pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#issuecomment-653102312


   @chaoyli 


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



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


[GitHub] [incubator-doris] chaoyli commented on pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#issuecomment-653307162


   > @chaoyli Now, should we prohibit insertion when `OLAP_ERR_PUSH_VERSION_ALREADY_EXIST` appears?
   
   Your function will add little cost to remove OLAP_ERR_PUSH_VERSION_ALREADY_EXIST.
   The cost will reflect on save the meta.


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



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


[GitHub] [incubator-doris] liutang123 commented on a change in pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
liutang123 commented on a change in pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#discussion_r449985800



##########
File path: be/src/olap/tablet.cpp
##########
@@ -826,6 +832,33 @@ void Tablet::_print_missed_versions(const std::vector<Version>& missed_versions)
     LOG(WARNING) << ss.str();
 }
 
+bool Tablet::_drop_rowset_if_version_conflict(const RowsetSharedPtr& rowset) {

Review comment:
       Why don't we need to do `do_tablet_meta_checkpoint` when adding a rs, but we need to do it when we delete a rs?
   




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



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


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#discussion_r449348960



##########
File path: be/src/olap/tablet.cpp
##########
@@ -826,6 +832,33 @@ void Tablet::_print_missed_versions(const std::vector<Version>& missed_versions)
     LOG(WARNING) << ss.str();
 }
 
+bool Tablet::_drop_rowset_if_version_conflict(const RowsetSharedPtr& rowset) {

Review comment:
       There has a problem in this place.
   modify_rowsets() only change the TabletMeta in memory. But not change the meta in disk.
   So you should perform do_tablet_meta_checkpoint to save the meta and remove the rowsetmeta in RowsetMetaManager.

##########
File path: be/src/olap/tablet.cpp
##########
@@ -826,6 +832,33 @@ void Tablet::_print_missed_versions(const std::vector<Version>& missed_versions)
     LOG(WARNING) << ss.str();
 }
 
+bool Tablet::_drop_rowset_if_version_conflict(const RowsetSharedPtr& rowset) {
+    if (rowset == nullptr) {
+        return false;
+    }
+    Version insert_version = {rowset->start_version(), rowset->end_version()};
+    Version max_version = _tablet_meta->max_version();
+    if (insert_version != max_version) {
+        return false;
+    }
+
+    RowsetSharedPtr exist_rs = get_rowset_by_version(insert_version);
+    // if a rs is created by doris 0.10 exists, remove it.
+    if (exist_rs != nullptr) {
+        vector<RowsetSharedPtr> to_add;
+        vector<RowsetSharedPtr> to_delete;
+        to_delete.push_back(exist_rs);
+        modify_rowsets(to_add, to_delete);
+        std::stringstream ss;
+        ss << "Remove rs " << exist_rs->rowset_id().to_string();

Review comment:
       LOG(WARNING) << "Remove rs " << exist_rs->rowset_id().to_string()
                                << " in " << full_name()
                                << " for version conflict with " << rowset->rowset_id().to_string()
   There is no necessity to use std::stringstream.




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



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


[GitHub] [incubator-doris] liutang123 edited a comment on pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
liutang123 edited a comment on pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#issuecomment-653102312


   @chaoyli Now, should we prohibit insertion when `OLAP_ERR_PUSH_VERSION_ALREADY_EXIST` appears?


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



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


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #4011: Remove rs when version conflict when insert to a tablet

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #4011:
URL: https://github.com/apache/incubator-doris/pull/4011#discussion_r450619615



##########
File path: be/src/olap/tablet.cpp
##########
@@ -826,6 +832,33 @@ void Tablet::_print_missed_versions(const std::vector<Version>& missed_versions)
     LOG(WARNING) << ss.str();
 }
 
+bool Tablet::_drop_rowset_if_version_conflict(const RowsetSharedPtr& rowset) {

Review comment:
       Because before add the rowset, it has already stills in RowsetMetaManager.
   It only make it visible in memory.
   After that, if you want to delete the rowset, you should delete from the RowsetMetaManager and add it to TabletMeta and save it.




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



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