Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,12 @@ public static void modifyTblReplicaCount(Database database, String tblName) {
InternalCatalog.INTERNAL_CATALOG_NAME,
StatisticConstants.DB_NAME,
tbl.getName());
// 1. modify table's default replica num
// 1. modify table's default replica allocation
Map<String, String> props = new HashMap<>();
props.put("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
"" + StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM);
ReplicaAllocation replicaAllocation = new ReplicaAllocation(
(short) StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM);
props.put("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
replicaAllocation.toCreateStmt());
Env.getCurrentEnv().modifyTableDefaultReplicaAllocation(database, tbl, props);

// 2. modify each partition's replica num
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@
*/
public class TableProperty implements GsonPostProcessable {
private static final Logger LOG = LogManager.getLogger(TableProperty.class);
private static final String DEFAULT_REPLICATION_NUM =
"default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM;
private static final String DEFAULT_REPLICATION_ALLOCATION =
"default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION;

@SerializedName(value = "properties")
private Map<String, String> properties;
Expand Down Expand Up @@ -678,10 +682,23 @@ public TableProperty buildInvertedIndexFileStorageFormat() {
}

public void modifyTableProperties(Map<String, String> modifyProperties) {
// Compatibility note: ModifyTablePropertyOperationLog persists only properties to set, not keys removed
// here. Keep its payload unchanged for this legacy repair. During a rolling FE upgrade, alter these
// properties on a table that already contains both legacy keys only after all FEs have been upgraded;
// otherwise old and new FEs may apply different effective replica settings.
removeConflictingDefaultReplicaProperty(modifyProperties);
Comment thread
deardeng marked this conversation as resolved.
properties.putAll(modifyProperties);
removeDuplicateReplicaNumProperty();
}

private void removeConflictingDefaultReplicaProperty(Map<String, String> modifyProperties) {
if (modifyProperties.containsKey(DEFAULT_REPLICATION_ALLOCATION)) {
properties.remove(DEFAULT_REPLICATION_NUM);
} else if (modifyProperties.containsKey(DEFAULT_REPLICATION_NUM)) {
properties.remove(DEFAULT_REPLICATION_ALLOCATION);
}
}

public void modifyDataSortInfoProperties(DataSortInfo dataSortInfo) {
properties.put(DataSortInfo.DATA_SORT_TYPE, String.valueOf(dataSortInfo.getSortType()));
properties.put(DataSortInfo.DATA_SORT_COL_NUM, String.valueOf(dataSortInfo.getColNum()));
Expand All @@ -690,8 +707,8 @@ public void modifyDataSortInfoProperties(DataSortInfo dataSortInfo) {
public void setReplicaAlloc(ReplicaAllocation replicaAlloc) {
this.replicaAlloc = replicaAlloc;
// set it to "properties" so that this info can be persisted
properties.put("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
replicaAlloc.toCreateStmt());
properties.remove(DEFAULT_REPLICATION_NUM);
properties.put(DEFAULT_REPLICATION_ALLOCATION, replicaAlloc.toCreateStmt());
}

public ReplicaAllocation getReplicaAllocation() {
Expand Down Expand Up @@ -968,11 +985,8 @@ public void gsonPostProcess() throws IOException {
buildColumnSeqMapping();
}

// For some historical reason,
// both "dynamic_partition.replication_num" and "dynamic_partition.replication_allocation"
// may be exist in "properties". we need remove the "dynamic_partition.replication_num", or it will always replace
// the "dynamic_partition.replication_allocation",
// result in unable to set "dynamic_partition.replication_allocation".
// Historical dynamic partition metadata may contain both replica properties. Keep the allocation form by
// removing replication_num because the analyzer checks it first.
private void removeDuplicateReplicaNumProperty() {
if (properties.containsKey(DynamicPartitionProperty.REPLICATION_NUM)
&& properties.containsKey(DynamicPartitionProperty.REPLICATION_ALLOCATION)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.plugin.audit.AuditLoader;
import org.apache.doris.statistics.StatisticConstants;
import org.apache.doris.utframe.TestWithFeService;
Expand Down Expand Up @@ -67,6 +68,10 @@ private void checkReplicationNum(Database db, String tblName) throws AnalysisExc
PartitionInfo partitionInfo = olapTable.getPartitionInfo();
Assertions.assertEquals((short) 3, olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum(),
tblName);
Assertions.assertFalse(olapTable.getTableProperty().getProperties()
.containsKey("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM), tblName);
Assertions.assertTrue(olapTable.getTableProperty().getProperties()
.containsKey("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION), tblName);
for (Partition partition : olapTable.getPartitions()) {
Assertions.assertEquals((short) 3,
partitionInfo.getReplicaAllocation(partition.getId()).getTotalReplicaNum());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,23 @@

package org.apache.doris.catalog;

import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.resource.Tag;

import com.google.common.collect.Maps;
import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.util.Map;

public class TablePropertyTest {
private static final String DEFAULT_REPLICATION_NUM =
"default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM;
private static final String DEFAULT_REPLICATION_ALLOCATION =
"default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION;
private static final String REPLICATION_ALLOCATION =
"tag.location.group_0: 1, tag.location.group_1: 1, tag.location.group_2: 1";

// A non-whitelisted dynamic_partition.* key is ignored (skipped via continue), so it is not
// collected at all and the table is neither built as dynamic nor flagged as incomplete.
Expand Down Expand Up @@ -105,4 +115,80 @@ public void testCompleteDynamicPartitionIsBuilt() {
Assert.assertEquals(3, tableProperty.getDynamicPartitionProperty().getEnd());
Assert.assertEquals(1, tableProperty.getDynamicPartitionProperty().getBuckets());
}

@Test
public void testModifyDefaultReplicaAllocationRemovesLegacyReplicationNum() {
Map<String, String> properties = Maps.newHashMap();
properties.put(DEFAULT_REPLICATION_NUM, "3");
TableProperty tableProperty = new TableProperty(properties);
tableProperty.buildReplicaAllocation();

Map<String, String> modifiedProperties = Maps.newHashMap();
modifiedProperties.put(DEFAULT_REPLICATION_ALLOCATION, REPLICATION_ALLOCATION);
tableProperty.modifyTableProperties(modifiedProperties);
tableProperty.buildReplicaAllocation();

assertReplicaAllocationWins(tableProperty);
}

@Test
public void testDeserializeConflictingDefaultReplicaPropertiesPreservesNumericPrecedence() throws IOException {
Map<String, String> properties = Maps.newHashMap();
properties.put(DEFAULT_REPLICATION_NUM, "3");
properties.put(DEFAULT_REPLICATION_ALLOCATION, REPLICATION_ALLOCATION);
TableProperty tableProperty = new TableProperty(properties);

tableProperty.gsonPostProcess();

Assert.assertTrue(tableProperty.getProperties().containsKey(DEFAULT_REPLICATION_NUM));
Assert.assertTrue(tableProperty.getProperties().containsKey(DEFAULT_REPLICATION_ALLOCATION));
Assert.assertEquals(Short.valueOf((short) 3),
tableProperty.getReplicaAllocation().getReplicaNumByTag(Tag.DEFAULT_BACKEND_TAG));
}

@Test
public void testResetPropertiesForRestoreRemovesLegacyReplicationNum() {
Map<String, String> properties = Maps.newHashMap();
properties.put(DEFAULT_REPLICATION_NUM, "3");
TableProperty tableProperty = new TableProperty(properties);
tableProperty.buildReplicaAllocation();

ReplicaAllocation restoredReplicaAllocation = new ReplicaAllocation((short) 2);
tableProperty.resetPropertiesForRestore(false, false, restoredReplicaAllocation);

Assert.assertFalse(tableProperty.getProperties().containsKey(DEFAULT_REPLICATION_NUM));
Assert.assertEquals(restoredReplicaAllocation.toCreateStmt(),
tableProperty.getProperties().get(DEFAULT_REPLICATION_ALLOCATION));
Assert.assertEquals((short) 2, tableProperty.getReplicaAllocation().getTotalReplicaNum());
}

@Test
public void testModifyDefaultReplicationNumRemovesExistingAllocation() {
Map<String, String> properties = Maps.newHashMap();
properties.put(DEFAULT_REPLICATION_ALLOCATION, REPLICATION_ALLOCATION);
TableProperty tableProperty = new TableProperty(properties);
tableProperty.buildReplicaAllocation();

Map<String, String> modifiedProperties = Maps.newHashMap();
modifiedProperties.put(DEFAULT_REPLICATION_NUM, "2");
tableProperty.modifyTableProperties(modifiedProperties);
tableProperty.buildReplicaAllocation();

Assert.assertFalse(tableProperty.getProperties().containsKey(DEFAULT_REPLICATION_ALLOCATION));
Assert.assertEquals(Short.valueOf((short) 2),
tableProperty.getReplicaAllocation().getReplicaNumByTag(Tag.DEFAULT_BACKEND_TAG));
}

private void assertReplicaAllocationWins(TableProperty tableProperty) {
Assert.assertFalse(tableProperty.getProperties().containsKey(DEFAULT_REPLICATION_NUM));
Assert.assertEquals(Short.valueOf((short) 1),
tableProperty.getReplicaAllocation()
.getReplicaNumByTag(Tag.createNotCheck(Tag.TYPE_LOCATION, "group_0")));
Assert.assertEquals(Short.valueOf((short) 1),
tableProperty.getReplicaAllocation()
.getReplicaNumByTag(Tag.createNotCheck(Tag.TYPE_LOCATION, "group_1")));
Assert.assertEquals(Short.valueOf((short) 1),
tableProperty.getReplicaAllocation()
.getReplicaNumByTag(Tag.createNotCheck(Tag.TYPE_LOCATION, "group_2")));
}
}
Loading