[GitHub] [incubator-doris] morningman commented on a diff in pull request #9248: [Refactor] Forbidden rowset v1

[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