From caa4ddfed4599602ffca5c3aafc17c8c2ecc565c Mon Sep 17 00:00:00 2001 From: Jerry Hu Date: Tue, 20 Oct 2026 18:54:18 +0800 Subject: [PATCH] [fix](fe) Check automatic partition limit before creation ### What problem does this PR solve? Issue Number: None Related PR: #69128 Problem Summary: Automatic partition creation checked max_auto_partition_num only after adding all requested partitions, so a rejected load could leave newly created partitions behind. Count missing partition names under the table read lock before the batch, and recheck capacity under the table write lock just before publishing each automatic partition. Existing-name requests at the limit remain valid; explicit partition DDL keeps its previous behavior. Under concurrent exhaustion, earlier within-limit partitions in a batch may remain. Cherry-picked from #69128 onto branch-4.2. Resolve conflicts by retaining branch-4.2's AddPartitionClause API and JUnit 4 test conventions. The master-only adjacent row-binlog test is not included. ### Release note Automatic partition requests exceeding max_auto_partition_num are rejected before creating partitions in the normal batch path. ### Check List (For Author) - Test: FE build and Checkstyle; focused FE unit tests for range overflow, existing LIST partitions, and stale-precheck publication; git diff --check. - Behavior changed: Yes. Over-limit automatic partition requests no longer leave newly created partitions in the normal batch path. - Does this need documentation: No. (cherry picked from commit 33c460d7365f7bba0111b01d2ee3adc82097669f) --- .../java/org/apache/doris/catalog/Env.java | 12 ++- .../doris/datasource/InternalCatalog.java | 22 ++++ .../doris/service/FrontendServiceImpl.java | 33 +++++- .../service/FrontendServiceImplTest.java | 102 ++++++++++++++++++ 4 files changed, 163 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java index e73f6c1c0ad953..75f92510b71206 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java @@ -3647,8 +3647,18 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa boolean writeEditLog, List> batchPartitions) throws DdlException { + addPartition(db, tableName, addPartitionClause, isCreateTable, generatedPartitionId, + writeEditLog, batchPartitions, false); + } + + public void addPartition(Database db, String tableName, AddPartitionClause addPartitionClause, + boolean isCreateTable, long generatedPartitionId, + boolean writeEditLog, + List> batchPartitions, + boolean enforceAutoPartitionLimit) + throws DdlException { getInternalCatalog().addPartition(db, tableName, addPartitionClause, - isCreateTable, generatedPartitionId, writeEditLog, batchPartitions); + isCreateTable, generatedPartitionId, writeEditLog, batchPartitions, enforceAutoPartitionLimit); } public void addMultiPartitions(Database db, String tableName, AlterMultiPartitionClause multiPartitionClause) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 4ebfb9f95f576f..adda42ad5c1498 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -1468,6 +1468,16 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa boolean writeEditLog, List> batchPartitions) throws DdlException { + addPartition(db, tableName, addPartitionClause, isCreateTable, generatedPartitionId, + writeEditLog, batchPartitions, false); + } + + public void addPartition(Database db, String tableName, AddPartitionClause addPartitionClause, + boolean isCreateTable, long generatedPartitionId, + boolean writeEditLog, + List> batchPartitions, + boolean enforceAutoPartitionLimit) + throws DdlException { // in cloud mode, isCreateTable == true, create dynamic partition use, so partitionId must have been generated. // isCreateTable == false, other case, partitionId generate in below, must be set 0 if (!FeConstants.runningUnitTest && Config.isCloudMode() @@ -1747,6 +1757,18 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa } } + // Check at publication under the table write lock. A request may have passed the + // batch precheck before another request published its own auto partitions. + if (enforceAutoPartitionLimit) { + int partitionNum = olapTable.getPartitionNum(); + int autoPartitionLimit = Config.max_auto_partition_num; + if (partitionNum >= autoPartitionLimit) { + throw new DdlException(String.format( + "partition numbers %d exceeded limit of variable max_auto_partition_num %d", + (long) partitionNum + 1, autoPartitionLimit)); + } + } + // check if meta changed // rollup index may be added or dropped during add partition operation. // schema may be changed during add partition operation. diff --git a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java index 14f38a680e4338..dc9c0c3c0467f7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java +++ b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java @@ -4104,10 +4104,35 @@ public TCreatePartitionResult createPartition(TCreatePartitionRequest request) t return result; } + olapTable.readLock(); + try { + int partitionNum = olapTable.getPartitionNum(); + int newPartitionNum = 0; + for (String partitionName : addPartitionClauseMap.keySet()) { + if (!olapTable.checkPartitionNameExist(partitionName)) { + newPartitionNum++; + } + } + long expectedPartitionNum = (long) partitionNum + newPartitionNum; + int autoPartitionLimit = Config.max_auto_partition_num; + if (expectedPartitionNum > autoPartitionLimit) { + String errorMessage = String.format( + "partition numbers %d exceeded limit of variable max_auto_partition_num %d", + expectedPartitionNum, autoPartitionLimit); + LOG.warn(errorMessage); + errorStatus.setErrorMsgs(Lists.newArrayList(errorMessage)); + result.setStatus(errorStatus); + LOG.warn("send create partition error status: {}", result); + return result; + } + } finally { + olapTable.readUnlock(); + } + for (AddPartitionClause addPartitionClause : addPartitionClauseMap.values()) { try { - // here maybe check and limit created partitions num - Env.getCurrentEnv().addPartition(db, olapTable.getName(), addPartitionClause, false, 0, true, null); + Env.getCurrentEnv().addPartition(db, olapTable.getName(), addPartitionClause, false, 0, true, + null, true); } catch (DdlException e) { LOG.warn(e); errorStatus.setErrorMsgs( @@ -4133,9 +4158,7 @@ public TCreatePartitionResult createPartition(TCreatePartitionRequest request) t olapTable.readLock(); try { - // check partition's number limit. because partitions in addPartitionClauseMap may be duplicated with - // existing partitions, which would lead to false positive. so we should check the partition number AFTER - // adding new partitions using its ACTUAL NUMBER, rather than the sum of existing and requested partitions. + // Retain the actual-count check for concurrent changes and configuration updates during creation. int partitionNum = olapTable.getPartitionNum(); int autoPartitionLimit = Config.max_auto_partition_num; if (partitionNum > autoPartitionLimit) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java b/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java index e45bed95a47b57..6a4071c08c6d6c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java @@ -412,6 +412,108 @@ public void testCreatePartitionRange() throws Exception { Assert.assertNotNull(p20230807); } + @Test + public void testCreatePartitionRangeLimitDoesNotLeavePartitions() throws Exception { + createTable("CREATE TABLE test.partition_range_limit (event_day DATETIME NOT NULL, id INT) " + + "DUPLICATE KEY(event_day, id) " + + "AUTO PARTITION BY RANGE (date_trunc(event_day, 'day')) () " + + "DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES('replication_num' = '1')"); + Database db = Env.getCurrentInternalCatalog().getDbOrAnalysisException("test"); + OlapTable table = (OlapTable) db.getTableOrAnalysisException("partition_range_limit"); + int oldLimit = Config.max_auto_partition_num; + try { + Config.max_auto_partition_num = 1; + TCreatePartitionResult result = new FrontendServiceImpl(exeEnv).createPartition( + createPartitionRequest(db, table, "2023-08-07 00:00:00", "2023-08-08 00:00:00")); + Assert.assertEquals(TStatusCode.RUNTIME_ERROR, result.getStatus().getStatusCode()); + Assert.assertTrue(result.getStatus().getErrorMsgs().get(0).contains("max_auto_partition_num")); + Assert.assertEquals(0, table.getPartitionNum()); + } finally { + Config.max_auto_partition_num = oldLimit; + } + } + + @Test + public void testCreatePartitionListLimitCountsOnlyNewPartitions() throws Exception { + createTable("CREATE TABLE test.partition_list_limit (city_code VARCHAR(100) NOT NULL, id INT) " + + "DUPLICATE KEY(city_code, id) " + + "AUTO PARTITION BY LIST (city_code) () " + + "DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES('replication_num' = '1')"); + Database db = Env.getCurrentInternalCatalog().getDbOrAnalysisException("test"); + OlapTable table = (OlapTable) db.getTableOrAnalysisException("partition_list_limit"); + int oldLimit = Config.max_auto_partition_num; + try { + Config.max_auto_partition_num = 1; + FrontendServiceImpl impl = new FrontendServiceImpl(exeEnv); + TCreatePartitionResult first = impl.createPartition(createPartitionRequest(db, table, "BEIJING")); + Assert.assertEquals(TStatusCode.OK, first.getStatus().getStatusCode()); + Assert.assertEquals(1, table.getPartitionNum()); + + TCreatePartitionResult existing = impl.createPartition(createPartitionRequest(db, table, "BEIJING")); + Assert.assertEquals(TStatusCode.OK, existing.getStatus().getStatusCode()); + Assert.assertEquals(1, table.getPartitionNum()); + + TCreatePartitionResult overflow = impl.createPartition( + createPartitionRequest(db, table, "BEIJING", "SHANGHAI")); + Assert.assertEquals(TStatusCode.RUNTIME_ERROR, overflow.getStatus().getStatusCode()); + Assert.assertTrue(overflow.getStatus().getErrorMsgs().get(0).contains("max_auto_partition_num")); + Assert.assertEquals(1, table.getPartitionNum()); + } finally { + Config.max_auto_partition_num = oldLimit; + } + } + + @Test + public void testCreatePartitionLimitIsRecheckedAtPublication() throws Exception { + createTable("CREATE TABLE test.partition_list_concurrent_limit (city_code VARCHAR(100) NOT NULL, id INT) " + + "DUPLICATE KEY(city_code, id) " + + "AUTO PARTITION BY LIST (city_code) () " + + "DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES('replication_num' = '1')"); + Database db = Env.getCurrentInternalCatalog().getDbOrAnalysisException("test"); + OlapTable table = (OlapTable) db.getTableOrAnalysisException("partition_list_concurrent_limit"); + int oldLimit = Config.max_auto_partition_num; + try { + Config.max_auto_partition_num = 1; + FrontendServiceImpl impl = new FrontendServiceImpl(exeEnv); + TCreatePartitionResult first = impl.createPartition(createPartitionRequest(db, table, "BEIJING")); + Assert.assertEquals(TStatusCode.OK, first.getStatus().getStatusCode()); + + // Model a concurrent publication after the request's read-lock precheck. + OlapTable spyTable = Mockito.spy(table); + AtomicBoolean stalePrecheck = new AtomicBoolean(true); + db.unregisterTable(table.getName()); + db.registerTable(spyTable); + try { + Mockito.doAnswer(invocation -> stalePrecheck.compareAndSet(true, false) + ? 0 : invocation.callRealMethod()).when(spyTable).getPartitionNum(); + TCreatePartitionResult result = impl.createPartition(createPartitionRequest(db, spyTable, "SHANGHAI")); + Assert.assertEquals(TStatusCode.RUNTIME_ERROR, result.getStatus().getStatusCode()); + Assert.assertTrue(result.getStatus().getErrorMsgs().get(0).contains("max_auto_partition_num")); + Assert.assertFalse(stalePrecheck.get()); + Assert.assertEquals(1, spyTable.getPartitionNum()); + } finally { + db.unregisterTable(spyTable.getName()); + db.registerTable(table); + } + } finally { + Config.max_auto_partition_num = oldLimit; + } + } + + private static TCreatePartitionRequest createPartitionRequest(Database db, OlapTable table, String... values) { + List> partitionValues = new ArrayList<>(); + for (String value : values) { + TNullableStringLiteral literal = new TNullableStringLiteral(); + literal.setValue(value); + partitionValues.add(Collections.singletonList(literal)); + } + TCreatePartitionRequest request = new TCreatePartitionRequest(); + request.setDbId(db.getId()); + request.setTableId(table.getId()); + request.setPartitionValues(partitionValues); + return request; + } + @Test public void testCreatePartitionReturnsRetryErrorWhenResultPartitionIsMissing() throws Exception { String createOlapTblStmt = "CREATE TABLE test.partition_dropped_before_result_snapshot(\n"