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/01/11 10:16:58 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #2737:
Support replication_num setting for table level
caiconghui opened a new pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737
Support replication_num setting for table level, so There is no need for user to set replication_num for every alter table add partition statement.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365641566
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -148,6 +150,8 @@ public OlapTable() {
this.indexes = null;
this.tableProperty = null;
+
+ this.replicationNum = null;
Review comment:
ok. I will fix 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365582307
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -148,6 +150,8 @@ public OlapTable() {
this.indexes = null;
this.tableProperty = null;
+
+ this.replicationNum = null;
Review comment:
The current analyzer will remove all the table property after analyzed, include replication_num, so is it a better way to change this design?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r366287489
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -1203,4 +1203,19 @@ public boolean convertRandomDistributionToHashDistribution() {
}
return hasChanged;
}
+
+ public void setReplicationNum(Short replicationNum) {
+ if (tableProperty == null) {
+ tableProperty = new TableProperty(new HashMap<String, String>());
+ }
+ tableProperty.getProperties().put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM, replicationNum.toString());
+ }
+
+ public Short getReplicationNum() {
Review comment:
I think that may confused user. Because the user doesn't set the replication_num in table level,
however, FeConstants.default_replication_num can be changed by admin, and when user don't modify config for table, he may find the replication_num config in table level changed, return null indicate that we don't set any replication_num config in table level, and then we use FeConstants.default_replication_num
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365562823
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -148,6 +150,8 @@ public OlapTable() {
this.indexes = null;
this.tableProperty = null;
+
+ this.replicationNum = null;
Review comment:
I think you can add this `this.replicationNum` to the `tableProperty` so that we do not need to update the FeMetaVersion.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #2737: Support
replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r366163562
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -1203,4 +1203,19 @@ public boolean convertRandomDistributionToHashDistribution() {
}
return hasChanged;
}
+
+ public void setReplicationNum(Short replicationNum) {
+ if (tableProperty == null) {
+ tableProperty = new TableProperty(new HashMap<String, String>());
+ }
+ tableProperty.getProperties().put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM, replicationNum.toString());
+ }
+
+ public Short getReplicationNum() {
Review comment:
how about just return `FeConstants.default_replication_num` is there is no default replica num?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r367432244
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -5117,15 +5118,28 @@ public void replayModifyTableDynamicPartition(ModifyDynamicPartitionInfo info) {
OlapTable olapTable = (OlapTable) db.getTable(tableId);
TableProperty tableProperty = olapTable.getTableProperty();
if (tableProperty == null) {
- olapTable.setTableProperty(new TableProperty(properties).buildDynamicProperty());
+ olapTable.setTableProperty(new TableProperty(properties).buildProperty(opCode));
} else {
tableProperty.modifyTableProperties(properties);
+ tableProperty.buildProperty(opCode);
}
} finally {
db.writeUnlock();
}
}
+ public void modifyTableReplicationNum(Database db, OlapTable table, Map<String, String> properties) throws DdlException {
+ TableProperty tableProperty = table.getTableProperty();
+ if (tableProperty == null) {
+ tableProperty = new TableProperty(properties);
+ } else {
+ tableProperty.modifyTableProperties(properties);
+ }
+ tableProperty.buildReplicationNum();
+ ModifyTablePropertyOperationLog info = new ModifyTablePropertyOperationLog(db.getId(), table.getId(), table.getTableProperty().getProperties());
Review comment:
You do not need to log all properties in TableProperty, only `properties` is enough
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r366163599
##########
File path: fe/src/main/java/org/apache/doris/catalog/OlapTable.java
##########
@@ -1203,4 +1203,19 @@ public boolean convertRandomDistributionToHashDistribution() {
}
return hasChanged;
}
+
+ public void setReplicationNum(Short replicationNum) {
Review comment:
See comment of `TableProperty` about how to modify and use this class.
You should add a new field `defaultReplicaNum` in `TableProperty`, and this field is built from `properties`.
Also, a method `buildDefaultReplicaNum()` should be implemented.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #2737:
Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365519791
##########
File path: fe/src/main/java/org/apache/doris/analysis/SingleRangePartitionDesc.java
##########
@@ -87,15 +87,13 @@ public short getReplicationNum() {
return this.properties;
}
- public void analyze(int partColNum, Map<String, String> otherProperties) throws AnalysisException {
+ public void analyze(int partColNum, Map<String, String> otherProperties, Short replicationNum) throws AnalysisException {
Review comment:
I don't think it is a good way to analyze this class with a input replicationNum. It is up to the caller to decide how to use this object.
If we do this through input arguments, there will be too many arguments.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365637502
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3908,6 +3907,12 @@ public static void getDdlStmt(Table table, List<String> createTableStmt, List<St
sb.append(olapTable.getTableProperty().getDynamicPartitionProperty().toString());
}
+ // 7. replicationNum
Review comment:
OK, I agree with u.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365562890
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -242,6 +242,8 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
+import static org.apache.doris.common.util.PropertyAnalyzer.PROPERTIES_REPLICATION_NUM;
Review comment:
Do not use static import
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on issue #2737: Support
replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#issuecomment-573304009
#2738
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365582329
##########
File path: fe/src/main/java/org/apache/doris/analysis/SingleRangePartitionDesc.java
##########
@@ -87,15 +87,13 @@ public short getReplicationNum() {
return this.properties;
}
- public void analyze(int partColNum, Map<String, String> otherProperties) throws AnalysisException {
+ public void analyze(int partColNum, Map<String, String> otherProperties, Short replicationNum) throws AnalysisException {
Review comment:
ok, I will fix 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365581841
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3908,6 +3907,12 @@ public static void getDdlStmt(Table table, List<String> createTableStmt, List<St
sb.append(olapTable.getTableProperty().getDynamicPartitionProperty().toString());
}
+ // 7. replicationNum
Review comment:
I think just like the DISTRIBUTED BY HASH(id) BUCKETS 5, which also does not reflect the real hash info of each partition of a table. so in the create table it just show the baisc config for the table and user can still set different distribution info for the special partition
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365581841
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3908,6 +3907,12 @@ public static void getDdlStmt(Table table, List<String> createTableStmt, List<St
sb.append(olapTable.getTableProperty().getDynamicPartitionProperty().toString());
}
+ // 7. replicationNum
Review comment:
I think just like the DISTRIBUTED BY HASH(id) BUCKETS 5, which also does not reflect the real hash info of each partition of a table. so in the create table it just show the baisc config for the table and user can still set different distribution info for the special partition
the real table status like partition real information should be got by show partitions statement, and create table statement show only show the basic config for the table and the initial status
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2737: Support replication_num setting for table level
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2737: Support replication_num setting for table level
URL: https://github.com/apache/incubator-doris/pull/2737#discussion_r365562870
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3908,6 +3907,12 @@ public static void getDdlStmt(Table table, List<String> createTableStmt, List<St
sb.append(olapTable.getTableProperty().getDynamicPartitionProperty().toString());
}
+ // 7. replicationNum
Review comment:
Do not add replication num in `create table stmt`, cause it does not reflect the real replication num of each partition of a table.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org