This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 84f0da56682 SOLR-18303: Stop backup cleanup from hiding the original
failure (#4776)
84f0da56682 is described below
commit 84f0da56682bd2dc545731448c456125f1828b70
Author: Prithvi S <[email protected]>
AuthorDate: Tue Sep 15 21:52:36 2026 +0530
SOLR-18303: Stop backup cleanup from hiding the original failure (#4776)
Signed-off-by: prithvi <[email protected]>
---
...8303-incremental-backup-cleanup-masks-error.yml | 8 ++
.../solr/cloud/api/collections/BackupCmd.java | 12 ++-
.../cloud/api/collections/DeleteBackupCmd.java | 14 ++--
.../cloud/api/collections/DeleteBackupCmdTest.java | 92 ++++++++++++++++++++++
4 files changed, 118 insertions(+), 8 deletions(-)
diff --git
a/changelog/unreleased/SOLR-18303-incremental-backup-cleanup-masks-error.yml
b/changelog/unreleased/SOLR-18303-incremental-backup-cleanup-masks-error.yml
new file mode 100644
index 00000000000..73132e57536
--- /dev/null
+++ b/changelog/unreleased/SOLR-18303-incremental-backup-cleanup-masks-error.yml
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Incremental backup failures now report the original error instead of an
unrelated missing zk_backup directory exception
+type: fixed
+authors:
+ - name: Prithvi S
+links:
+ - name: SOLR-18303
+ url: https://issues.apache.org/jira/browse/SOLR-18303
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
index 45d136061af..2393ab55763 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
@@ -119,8 +119,16 @@ public class BackupCmd implements
CollApiCmds.CollectionApiCommand {
} catch (SolrException e) {
log.error(
"Error happened during incremental backup for collection:
{}", collectionName, e);
- CollectionHandlingUtils.cleanBackup(
- repository, backupUri, backupMgr.getBackupId(), ccc);
+ try {
+ CollectionHandlingUtils.cleanBackup(
+ repository, backupUri, backupMgr.getBackupId(), ccc);
+ } catch (Exception cleanupException) {
+ log.warn(
+ "Unable to clean up incomplete backup for collection: {}",
+ collectionName,
+ cleanupException);
+ e.addSuppressed(cleanupException);
+ }
throw e;
}
break;
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteBackupCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteBackupCmd.java
index 6b81d28cf0c..6c0ec870bf1 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteBackupCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteBackupCmd.java
@@ -25,6 +25,7 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.net.URI;
+import java.nio.file.NoSuchFileException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -206,13 +207,14 @@ public class DeleteBackupCmd implements
CollApiCmds.CollectionApiCommand {
.map(ShardBackupId::getBackupMetadataFilename)
.collect(Collectors.toList()));
repository.delete(incBackupFiles.getIndexDir(), unusedFiles);
- try {
- for (BackupId backupId : backupIdsDeletes) {
- repository.deleteDirectory(
- repository.resolveDirectory(backupUri,
BackupFilePaths.getZkStateDir(backupId)));
+ for (BackupId backupId : backupIdsDeletes) {
+ URI zkStateDir =
+ repository.resolveDirectory(backupUri,
BackupFilePaths.getZkStateDir(backupId));
+ try {
+ repository.deleteDirectory(zkStateDir);
+ } catch (FileNotFoundException | NoSuchFileException e) {
+ // zk_backup_* is created after shard copy, so a failed incremental
backup may not have it
}
- } catch (FileNotFoundException e) {
- // ignore this
}
// add details to result before deleting backupPropFiles
diff --git
a/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteBackupCmdTest.java
b/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteBackupCmdTest.java
new file mode 100644
index 00000000000..5d01b5ce3af
--- /dev/null
+++
b/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteBackupCmdTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.solr.cloud.api.collections;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.backup.BackupFilePaths;
+import org.apache.solr.core.backup.BackupId;
+import org.apache.solr.core.backup.repository.BackupRepository;
+import org.apache.solr.core.backup.repository.LocalFileSystemRepository;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Unit tests for {@link DeleteBackupCmd}. */
+public class DeleteBackupCmdTest extends SolrTestCase {
+
+ private BackupRepository repository;
+ private URI backupUri;
+
+ @Before
+ public void setUpRepo() throws Exception {
+ repository = new LocalFileSystemRepository();
+ backupUri =
+ repository.createDirectoryURI(
+ createTempDir("backup_" +
UUID.randomUUID()).toAbsolutePath().toString());
+ new BackupFilePaths(repository,
backupUri).createIncrementalBackupFolders();
+ }
+
+ @Test
+ public void testDeleteBackupIdsIgnoresMissingZkStateDir() throws Exception {
+ NamedList<Object> results = new NamedList<>();
+ new DeleteBackupCmd(null)
+ .deleteBackupIds(backupUri, repository, Set.of(BackupId.zero()),
results);
+
+ assertNotNull(results.get("deleted"));
+ assertFalse(repository.exists(zkStateDir(BackupId.zero())));
+ }
+
+ @Test
+ public void testDeleteBackupIdsRemovesExistingZkStateDir() throws Exception {
+ URI zkStateDir = zkStateDir(BackupId.zero());
+ repository.createDirectory(zkStateDir);
+ assertTrue(repository.exists(zkStateDir));
+
+ new DeleteBackupCmd(null)
+ .deleteBackupIds(backupUri, repository, Set.of(BackupId.zero()), new
NamedList<>());
+
+ assertFalse(repository.exists(zkStateDir));
+ }
+
+ @Test
+ public void testDeleteBackupIdsPropagatesUnexpectedDeleteErrors() {
+ BackupRepository failingRepository =
+ new LocalFileSystemRepository() {
+ @Override
+ public void deleteDirectory(URI path) throws IOException {
+ throw new IOException("simulated repository failure");
+ }
+ };
+
+ IOException thrown =
+ expectThrows(
+ IOException.class,
+ () ->
+ new DeleteBackupCmd(null)
+ .deleteBackupIds(
+ backupUri, failingRepository, Set.of(BackupId.zero()),
new NamedList<>()));
+ assertEquals("simulated repository failure", thrown.getMessage());
+ }
+
+ private URI zkStateDir(BackupId backupId) {
+ return repository.resolveDirectory(backupUri,
BackupFilePaths.getZkStateDir(backupId));
+ }
+}
