This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 5ea5dd73e13 [fix](replica) Resolve conflicting default replica
properties (#65836) (#66189)
5ea5dd73e13 is described below
commit 5ea5dd73e130ed0cf8fd2f2bb6e16c769f284ab9
Author: deardeng <[email protected]>
AuthorDate: Fri Jul 31 16:04:33 2026 +0800
[fix](replica) Resolve conflicting default replica properties (#65836)
(#66189)
pick from https://github.com/apache/doris/pull/65836
Problem Summary: ALTER TABLE updates to default.replication_allocation
could leave the legacy default.replication_num property in table
metadata. Because replica analysis checks replication_num first, SHOW
CREATE TABLE and later consumers could observe the wrong default
allocation. Remove the mutually exclusive property when applying an
update, prefer replication_allocation when cleaning historical metadata,
and cover allocation updates, metadata cleanup, and reverse numeric
updates.
(cherry picked from commit 8a1bf788e290dc5832c4bd432a34d0e8b4c42906)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
.../doris/catalog/InternalSchemaInitializer.java | 8 +-
.../org/apache/doris/catalog/TableProperty.java | 28 +++--
.../doris/alter/InternalSchemaAlterTest.java | 5 +
.../apache/doris/catalog/TablePropertyTest.java | 113 +++++++++++++++++++++
4 files changed, 144 insertions(+), 10 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java
index 34dec30fe8a..fa87be6451a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java
@@ -256,10 +256,12 @@ public class InternalSchemaInitializer extends Thread {
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
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
index 830a668a3a5..dfa0db8b6d5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
@@ -54,6 +54,10 @@ import java.util.Map;
*/
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;
@@ -635,10 +639,23 @@ public class TableProperty implements GsonPostProcessable
{
}
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);
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()));
@@ -647,8 +664,8 @@ public class TableProperty implements GsonPostProcessable {
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() {
@@ -842,11 +859,8 @@ public class TableProperty implements GsonPostProcessable {
buildTDEAlgorithm();
}
- // 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)) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/alter/InternalSchemaAlterTest.java
b/fe/fe-core/src/test/java/org/apache/doris/alter/InternalSchemaAlterTest.java
index 122014f0e8b..3ad88c49c40 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/alter/InternalSchemaAlterTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/alter/InternalSchemaAlterTest.java
@@ -28,6 +28,7 @@ import org.apache.doris.catalog.PartitionInfo;
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;
@@ -67,6 +68,10 @@ public class InternalSchemaAlterTest extends
TestWithFeService {
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());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/TablePropertyTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/TablePropertyTest.java
new file mode 100644
index 00000000000..ca13c727691
--- /dev/null
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/TablePropertyTest.java
@@ -0,0 +1,113 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+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";
+
+ @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")));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
