Skip to content

[fix](cloud) Skip empty partition meta RPCs in cloud restore #67139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
felixwluo wants to merge 4 commits into
base: master
Choose a base branch
from
+35 −2
Open

[fix](cloud) Skip empty partition meta RPCs in cloud restore #67139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e1ce8d
[opt](scan) Use lazy-init for segment iterators and avoid caching all…
mrhhsg May 29, 2024
87d783e
[fix](cloud) Skip empty partition meta RPCs in cloud restore
felixwluo Aug 25, 2026
14bd4c8
Merge branch 'master' into fix-cloud-restore
felixwluo Aug 26, 2026
15b5f44
Merge branch 'master' into fix-cloud-restore
felixwluo Aug 26, 2026
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
10 changes: 9 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/cloud/backup/CloudRestoreJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,13 @@ public void doCreateReplicas() {
}
// set storage vault for new restoring table
if (((CloudEnv) Env.getCurrentEnv()).getEnableStorageVault()) {
if (Strings.isNullOrEmpty(storageVaultId)) {
storageVaultId = Env.getCurrentEnv().getStorageVaultMgr().getVaultIdByName(storageVaultName);

@github-actions github-actions Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not treat the asynchronous vault cache as authoritative

For an empty restore there is no create-tablets response to validate the vault, so this lookup is the only source of the ID. After restart/master promotion, storageVaultId is transient and the vault map starts empty; BackupHandler can resume jobs before CloudInstanceStatusChecker populates it, turning a valid restore into a permanent cancellation. The map can also be stale: the checker leaves old entries when Meta Service reports zero vaults, so deleting the last vault can let this code persist a deleted nonempty ID. Please resolve/revalidate the vault from an authoritative source at empty-table creation time, treating transient unavailability as retry/defer and definitive absence as failure before registration; merely serializing the earlier cached ID would not handle deletion.

}
for (Table table : restoredTbls) {
if (table.getType() == TableIf.TableType.OLAP) {
OlapTable olapTable = (OlapTable) table;
if (olapTable.getStorageVaultId().isEmpty() && storageVaultId != null) {
if (olapTable.getStorageVaultId().isEmpty()) {
olapTable.setStorageVaultId(storageVaultId);
}
}
Expand Down Expand Up @@ -494,6 +497,11 @@ private void handleMetaObject(MetaSeriviceOperation operation) throws DdlExcepti

private void handleOlapTableMeta(MetaSeriviceOperation operation, OlapTable olapTable,
Collection<Partition> partitions) throws DdlException {
if (partitions.isEmpty()) {

@github-actions github-actions Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the index lifecycle for an empty restored table

This avoids the invalid empty-partition RPCs, but it also skips the index-only lifecycle used by normal empty Cloud table creation. That path still calls prepare/commit materialized-index; commit-index creates the versioned index mappings and initializes the table version. A later ADD PARTITION only commits partition keys, so it does not repair the missing index mappings. The orphan recycler then sees no index-inverted key for the table and can delete the newly added partition and table-version metadata. Please use a replay-safe index PREPARE/COMMIT lifecycle for zero partitions, including cleanup of prepared/committed transient IDs on cancellation or PENDING replay, instead of suppressing all Meta Service work.

LOG.info("cloud restore job skip {} partitions, dbId: {}, tableName: {}, vault name: {}",
operation, dbId, olapTable.getName(), storageVaultName);
return;

@github-actions github-actions Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not register an unpartitioned table without its implicit partition

The new test exercises a SinglePartitionInfo table, removes its only partition, and this return lets the restore register that zero-partition table successfully. It is then permanently unusable: inserts fail with ERR_EMPTY_PARTITION_IN_TABLE, while ADD PARTITION is rejected for unpartitioned tables. The index-only lifecycle needed for valid empty RANGE/LIST tables does not repair this case. Please either reject exclusion of an unpartitioned table's sole implicit partition, or recreate an empty implicit partition/tablets so the restored table remains writable, and cover that outcome end to end.

}
List<Long> partitionIds = new ArrayList<>();
switch (operation) {
case PREPARE: {
Expand Down
27 changes: 26 additions & 1 deletion fe/fe-core/src/test/java/org/apache/doris/cloud/backup/CloudRestoreJobTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public void setUp() throws Exception {
Assert.assertTrue(cloudSystemInfoService instanceof CloudSystemInfoService);

Mockito.when(storageVaultMgr.getVaultNameById(Mockito.anyString())).thenReturn("test_vault");
Mockito.when(storageVaultMgr.getVaultIdByName("test_vault")).thenReturn("test_vault_id");

mockMetaServiceProxyInstance = Mockito.mock(MetaServiceProxy.class);
mockedMetaServiceProxy = Mockito.mockStatic(MetaServiceProxy.class);
Expand Down Expand Up @@ -256,5 +257,29 @@ public void testCreateReplicas() throws UserException {
Assert.assertTrue(job.getStatus().ok());
}

}
@Test
public void testSkipCloudMetaServiceForEmptyRestorePartitions() throws Exception {
Map<String, String> properties = Maps.newHashMap();
properties.put("storage_vault_id", "");
expectedRestoreTbl.setTableProperty(new TableProperty(properties));
Partition partition = expectedRestoreTbl.getPartition(CatalogTestUtil.testPartition1);
expectedRestoreTbl.getPartitionInfo().setIsInMemory(partition.getId(), false);
expectedRestoreTbl.dropPartitionAndReserveTablet(partition.getName());
Deencapsulation.setField(job, "restoredTbls", Lists.newArrayList(expectedRestoreTbl));

job.doCreateReplicas();
Assert.assertTrue(job.getStatus().ok());
Assert.assertEquals("test_vault_id", expectedRestoreTbl.getStorageVaultId());

job.waitingAllReplicasCreated();
Assert.assertTrue(job.getStatus().ok());

job.cleanMetaObjects(false);
Assert.assertTrue(job.getStatus().ok());

Mockito.verify(mockMetaServiceProxyInstance, Mockito.never()).preparePartition(Mockito.any());
Mockito.verify(mockMetaServiceProxyInstance, Mockito.never()).commitPartition(Mockito.any());
Mockito.verify(mockMetaServiceProxyInstance, Mockito.never()).dropPartition(Mockito.any());
}

}
Loading