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 2022/04/27 02:55:53 UTC
[GitHub] [incubator-doris] yiguolei opened a new pull request, #9248: Forbidden rowset v1
yiguolei opened a new pull request, #9248:
URL: https://github.com/apache/incubator-doris/pull/9248
# Proposed changes
This is part of #8984.
FE will never create rowset v1(or alpha rowset) any more. Fe will ignore the storage format parameters if it is v1. Create table, schema change and rollup job will use storage format v2.
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] github-actions[bot] commented on pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#issuecomment-1117057091
PR approved by at least one committer and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] yiguolei merged pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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 diff in pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#discussion_r864535072
##########
be/src/olap/storage_engine.cpp:
##########
@@ -916,8 +916,13 @@ void StorageEngine::_parse_default_rowset_type() {
boost::to_upper(default_rowset_type_config);
if (default_rowset_type_config == "BETA") {
_default_rowset_type = BETA_ROWSET;
- } else {
+ } else if (default_rowset_type_config == "ALPHA") {
_default_rowset_type = ALPHA_ROWSET;
+ LOG(WARNING) << "default_rowset_type in be.conf should be set to beta, alpha is not "
Review Comment:
I think we should use `CONF_Validator` to check this.
See `CONF_Validator` in be/src/common/config.h
##########
fe/fe-core/src/main/java/org/apache/doris/system/HeartbeatFlags.java:
##########
@@ -28,18 +28,21 @@
// Now the flag is represented by 64-bit long type, each bit can be used to control
// one behavior. The first bit is used for set default rowset type to beta flag.
public class HeartbeatFlags {
- private static final Logger LOG = LogManager.getLogger(HeartbeatFlags.class);
+ private static final Logger LOG = LogManager.getLogger(HeartbeatFlags.class);
- public static boolean isValidRowsetType(String rowsetType) {
- return "alpha".equalsIgnoreCase(rowsetType) || "beta".equalsIgnoreCase(rowsetType);
- }
+ public static boolean isValidRowsetType(String rowsetType) {
+ return "alpha".equalsIgnoreCase(rowsetType) || "beta".equalsIgnoreCase(rowsetType);
+ }
- public long getHeartbeatFlags() {
- long heartbeatFlags = 0;
- if ("beta".equalsIgnoreCase(GlobalVariable.defaultRowsetType)) {
- heartbeatFlags |= HeartbeatServiceConstants.IS_SET_DEFAULT_ROWSET_TO_BETA_BIT;
- }
+ public long getHeartbeatFlags() {
+ long heartbeatFlags = 0;
+ if ("beta".equalsIgnoreCase(GlobalVariable.defaultRowsetType)) {
+ heartbeatFlags |= HeartbeatServiceConstants.IS_SET_DEFAULT_ROWSET_TO_BETA_BIT;
+ } else {
+ throw new IllegalArgumentException("DEFAULT_ROWSET_TYPE in global "
Review Comment:
We should `ignore` "alpha" instead of throw exception. Otherwise, heartbeat may fail
##########
fe/fe-core/src/main/java/org/apache/doris/task/CreateReplicaTask.java:
##########
@@ -187,7 +189,14 @@ public void setBaseTablet(long baseTabletId, int baseSchemaHash) {
}
public void setStorageFormat(TStorageFormat storageFormat) {
- this.storageFormat = storageFormat;
+ if (storageFormat == TStorageFormat.V1) {
+ // Ignore v1 format, the storage format must not be v1
+ // V1 will be removed in the future
+ LOG.error("StorageFormat.V1 is not supported any more, will use V2 automatically");
Review Comment:
Throw exception instead of print error log.
No one will notice this log.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] github-actions[bot] commented on pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#issuecomment-1117057125
PR approved by anyone and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] yiguolei commented on a diff in pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#discussion_r864574834
##########
be/src/olap/storage_engine.cpp:
##########
@@ -916,8 +916,13 @@ void StorageEngine::_parse_default_rowset_type() {
boost::to_upper(default_rowset_type_config);
if (default_rowset_type_config == "BETA") {
_default_rowset_type = BETA_ROWSET;
- } else {
+ } else if (default_rowset_type_config == "ALPHA") {
_default_rowset_type = ALPHA_ROWSET;
+ LOG(WARNING) << "default_rowset_type in be.conf should be set to beta, alpha is not "
Review Comment:
Not use validator because it will throw exception during bootstrap and the process could not start. It's bad for upgrade process.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] yiguolei commented on a diff in pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#discussion_r859440233
##########
fe/fe-core/src/main/java/org/apache/doris/task/CreateReplicaTask.java:
##########
@@ -187,7 +189,12 @@ public void setBaseTablet(long baseTabletId, int baseSchemaHash) {
}
public void setStorageFormat(TStorageFormat storageFormat) {
- this.storageFormat = storageFormat;
+ if (storageFormat == TStorageFormat.V1) {
+ // Ignore v1 format, the storage format must not be v1
Review Comment:
done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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] yangzhg commented on a diff in pull request #9248: [Refactor] Forbidden rowset v1
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #9248:
URL: https://github.com/apache/incubator-doris/pull/9248#discussion_r859358077
##########
fe/fe-core/src/main/java/org/apache/doris/task/CreateReplicaTask.java:
##########
@@ -187,7 +189,12 @@ public void setBaseTablet(long baseTabletId, int baseSchemaHash) {
}
public void setStorageFormat(TStorageFormat storageFormat) {
- this.storageFormat = storageFormat;
+ if (storageFormat == TStorageFormat.V1) {
+ // Ignore v1 format, the storage format must not be v1
Review Comment:
print log or throw an exception
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
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