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/06/02 10:04:08 UTC
[GitHub] [incubator-doris] acelyc111 opened a new pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
acelyc111 opened a new pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750
TabletMeta's _preferred_rowset_type is not initialized after object constructing and
may be a random value, and this field is not updated when create ALPHA_ROWSET tablet,
and it will not be serialized into pb in this case. So if cloning an ALPHA_ROWSET
tablet from another BE, this new created local tablet's _preferred_rowset_type field
may be random as BETA_ROWSET and can not be overwrote after cloned, then new input
rows will be wrote as BETA_ROWSET format which is not we expect.
This patch fix this bug by giving _preferred_rowset_type a default value and updating
this field when create any type of tablet, and add an unit test and related overwrite
equal operator functions.
----------------------------------------------------------------
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] imay merged pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
imay merged pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750
----------------------------------------------------------------
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] morningman commented on a change in pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750#discussion_r434942618
##########
File path: be/src/olap/tablet_schema.cpp
##########
@@ -337,14 +337,12 @@ void TabletColumn::to_schema_pb(ColumnPB* column) {
}
}
-TabletSchema::TabletSchema()
- : _num_columns(0),
- _num_key_columns(0),
- _num_null_columns(0),
- _num_short_key_columns(0) { }
-
void TabletSchema::init_from_pb(const TabletSchemaPB& schema) {
_keys_type = schema.keys_type();
+ _num_columns = 0;
+ _num_key_columns = 0;
+ _num_null_columns = 0;
Review comment:
ok
----------------------------------------------------------------
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] morningman commented on a change in pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750#discussion_r433856036
##########
File path: be/src/olap/tablet_schema.cpp
##########
@@ -337,14 +337,12 @@ void TabletColumn::to_schema_pb(ColumnPB* column) {
}
}
-TabletSchema::TabletSchema()
- : _num_columns(0),
- _num_key_columns(0),
- _num_null_columns(0),
- _num_short_key_columns(0) { }
-
void TabletSchema::init_from_pb(const TabletSchemaPB& schema) {
_keys_type = schema.keys_type();
+ _num_columns = 0;
+ _num_key_columns = 0;
+ _num_null_columns = 0;
Review comment:
I am not sure, but did you miss the `_num_short_key_columns` here?
----------------------------------------------------------------
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] imay commented on a change in pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750#discussion_r435340647
##########
File path: be/src/olap/tablet_schema.h
##########
@@ -107,22 +113,30 @@ class TabletSchema {
inline void set_is_in_memory (bool is_in_memory) {
_is_in_memory = is_in_memory;
}
+
private:
- KeysType _keys_type;
+ friend bool operator==(const TabletSchema& a, const TabletSchema& b);
+ friend bool operator!=(const TabletSchema& a, const TabletSchema& b);
+
+private:
+ KeysType _keys_type = DUP_KEYS;
std::vector<TabletColumn> _cols;
- size_t _num_columns;
- size_t _num_key_columns;
- size_t _num_null_columns;
- size_t _num_short_key_columns;
- size_t _num_rows_per_row_block;
- CompressKind _compress_kind;
- size_t _next_column_unique_id;
+ size_t _num_columns = 0;
+ size_t _num_key_columns = 0;
+ size_t _num_null_columns = 0;
+ size_t _num_short_key_columns = 0;
+ size_t _num_rows_per_row_block = 0;
+ CompressKind _compress_kind = COMPRESS_NONE;
+ size_t _next_column_unique_id = 0;
bool _has_bf_fpp = false;
double _bf_fpp = 0;
bool _is_in_memory = false;
};
+bool operator==(const TabletSchema& a, const TabletSchema& b);
Review comment:
Is this function used?
----------------------------------------------------------------
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] acelyc111 commented on a change in pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750#discussion_r433912838
##########
File path: be/src/olap/tablet_schema.cpp
##########
@@ -337,14 +337,12 @@ void TabletColumn::to_schema_pb(ColumnPB* column) {
}
}
-TabletSchema::TabletSchema()
- : _num_columns(0),
- _num_key_columns(0),
- _num_null_columns(0),
- _num_short_key_columns(0) { }
-
void TabletSchema::init_from_pb(const TabletSchemaPB& schema) {
_keys_type = schema.keys_type();
+ _num_columns = 0;
+ _num_key_columns = 0;
+ _num_null_columns = 0;
Review comment:
@morningman `_num_short_key_columns` will be assigned in line 358 directly, not like the above variables which will be increased in the following loop, so I didn't reset it here.
----------------------------------------------------------------
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] acelyc111 commented on a change in pull request #3750: [Bug] Fix a bug that tablet may be modified to BETA_ROWSET after cloned
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #3750:
URL: https://github.com/apache/incubator-doris/pull/3750#discussion_r435397028
##########
File path: be/src/olap/tablet_schema.h
##########
@@ -107,22 +113,30 @@ class TabletSchema {
inline void set_is_in_memory (bool is_in_memory) {
_is_in_memory = is_in_memory;
}
+
private:
- KeysType _keys_type;
+ friend bool operator==(const TabletSchema& a, const TabletSchema& b);
+ friend bool operator!=(const TabletSchema& a, const TabletSchema& b);
+
+private:
+ KeysType _keys_type = DUP_KEYS;
std::vector<TabletColumn> _cols;
- size_t _num_columns;
- size_t _num_key_columns;
- size_t _num_null_columns;
- size_t _num_short_key_columns;
- size_t _num_rows_per_row_block;
- CompressKind _compress_kind;
- size_t _next_column_unique_id;
+ size_t _num_columns = 0;
+ size_t _num_key_columns = 0;
+ size_t _num_null_columns = 0;
+ size_t _num_short_key_columns = 0;
+ size_t _num_rows_per_row_block = 0;
+ CompressKind _compress_kind = COMPRESS_NONE;
+ size_t _next_column_unique_id = 0;
bool _has_bf_fpp = false;
double _bf_fpp = 0;
bool _is_in_memory = false;
};
+bool operator==(const TabletSchema& a, const TabletSchema& b);
Review comment:
TabletMeta has a TabletSchema member, overload TabletSchema's == operator is aim to overload TabletMeta's == operator.
Now TabletMeta's == operator is used for convenient unit test.
----------------------------------------------------------------
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