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-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to