This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch branch_10x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_10x by this push:
new d9f86bd17e6 SOLR-17953: Ignore hidden files when locating an
incremental backup (10x backport) (#5197)
d9f86bd17e6 is described below
commit d9f86bd17e6f3547f7a22e8fc6d33540715f38f1
Author: sarkaramrit1993 <[email protected]>
AuthorDate: Sun Oct 25 07:40:23 2026 -0700
SOLR-17953: Ignore hidden files when locating an incremental backup (10x
backport) (#5197)
---
.../unreleased/SOLR-17953-backup-hidden-files.yml | 7 ++
.../solr/cloud/api/collections/BackupCmd.java | 3 +-
.../cloud/api/collections/DeleteBackupCmd.java | 1 +
.../solr/cloud/api/collections/RestoreCmd.java | 10 +-
.../apache/solr/core/backup/BackupFilePaths.java | 24 +++-
.../org/apache/solr/core/backup/ShardBackupId.java | 2 +-
.../solr/core/backup/BackupFilePathsTest.java | 44 +++++++
.../collections/AbstractIncrementalBackupTest.java | 135 +++++++++++++++++++++
8 files changed, 212 insertions(+), 14 deletions(-)
diff --git a/changelog/unreleased/SOLR-17953-backup-hidden-files.yml
b/changelog/unreleased/SOLR-17953-backup-hidden-files.yml
new file mode 100644
index 00000000000..a82a6398c1e
--- /dev/null
+++ b/changelog/unreleased/SOLR-17953-backup-hidden-files.yml
@@ -0,0 +1,7 @@
+title: Hidden files such as .DS_Store in an incremental backup no longer break
RESTORE, LISTBACKUP, DELETEBACKUP or BACKUP, which also stops a second
collection from being backed up under the same name.
+type: fixed
+authors:
+ - name: Amrit Sarkar
+links:
+ - name: SOLR-17953
+ url: https://issues.apache.org/jira/browse/SOLR-17953
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 d7e8e21e309..35c45d73acb 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
@@ -225,7 +225,8 @@ public class BackupCmd implements
CollApiCmds.CollectionApiCommand {
if (!repository.exists(backupNamePath)) {
repository.createDirectory(backupNamePath);
} else if (incremental) {
- final String[] directoryContents = repository.listAll(backupNamePath);
+ final String[] directoryContents =
+
BackupFilePaths.findCollectionDirs(repository.listAll(backupNamePath));
if (directoryContents.length == 1) {
String directoryContentsName = directoryContents[0];
// Strip the trailing '/' if it exists
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 6c0ec870bf1..de7a9ad7864 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
@@ -168,6 +168,7 @@ public class DeleteBackupCmd implements
CollApiCmds.CollectionApiCommand {
List<ShardBackupId> shardBackupIds =
Arrays.stream(repository.listAllOrEmpty(shardBackupMetadataDir))
+ .filter(sbi -> sbi.endsWith(ShardBackupId.FILENAME_SUFFIX))
.map(sbi -> ShardBackupId.fromShardMetadataFilename(sbi))
.collect(Collectors.toList());
for (ShardBackupId shardBackupId : shardBackupIds) {
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
index dabaf64420e..9380f72d3fc 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
@@ -33,7 +33,6 @@ import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.net.URI;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
@@ -74,6 +73,7 @@ import org.apache.solr.common.util.StrUtils;
import org.apache.solr.common.util.Utils;
import org.apache.solr.core.ConfigSetService;
import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.backup.BackupFilePaths;
import org.apache.solr.core.backup.BackupManager;
import org.apache.solr.core.backup.BackupProperties;
import org.apache.solr.core.backup.ShardBackupId;
@@ -189,14 +189,8 @@ public class RestoreCmd implements
CollApiCmds.CollectionApiCommand {
this.location =
repository.createDirectoryURI(message.getStr(CoreAdminParams.BACKUP_LOCATION));
- final URI backupNameUri = repository.resolveDirectory(location,
backupName);
- final String[] entries = repository.listAll(backupNameUri);
- final boolean incremental =
- !Arrays.stream(entries)
- .anyMatch(entry ->
entry.equals(BackupManager.TRADITIONAL_BACKUP_PROPS_FILE));
- // incremental backups have an extra path component representing the
backed up collection
this.backupPath =
- (incremental) ? repository.resolveDirectory(backupNameUri,
entries[0]) : backupNameUri;
+ BackupFilePaths.buildExistingBackupLocationURI(repository, location,
backupName);
this.zkStateReader = ccc.getZkStateReader();
this.backupManager =
backupId == -1
diff --git
a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
index 3e1727714ef..8157bd136ba 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
@@ -157,6 +157,8 @@ public class BackupFilePaths {
* @param repository the backup repository, used to list files and resolve
URI's.
* @param location a URI representing the repository location holding each
backup name
* @param backupName the specific backup name to create a URI for
+ * @throws IllegalStateException if an incremental backup does not hold
exactly one collection
+ * directory, after ignoring entries as described in {@link
#findCollectionDirs(String[])}
*/
public static URI buildExistingBackupLocationURI(
BackupRepository repository, URI location, String backupName) throws
IOException {
@@ -168,20 +170,34 @@ public class BackupFilePaths {
if (incremental) {
// Incremental backups have an additional URI path component
representing the collection that
// was backed up. This collection directory is the path assumed by other
backup code.
- if (entries.length != 1) {
+ final String[] collectionDirs = findCollectionDirs(entries);
+ if (collectionDirs.length != 1) {
throw new IllegalStateException(
"Incremental backup URI ["
+ backupNameUri
+ "] expected to hold a single directory. Number found: "
- + entries.length);
+ + collectionDirs.length);
}
- final String collectionName = entries[0];
- return repository.resolveDirectory(backupNameUri, entries[0]);
+ return repository.resolveDirectory(backupNameUri, collectionDirs[0]);
} else {
return backupNameUri;
}
}
+ /**
+ * Returns the entries of a backup name directory that may be collection
directories, ignoring
+ * dot-prefixed entries such as .DS_Store. A lone dot-prefixed entry is
kept, since collection
+ * names may start with a dot.
+ *
+ * @param entries the listing of a backup name directory
+ */
+ public static String[] findCollectionDirs(String[] entries) {
+ if (entries.length <= 1) {
+ return entries;
+ }
+ return Arrays.stream(entries).filter(entry ->
!entry.startsWith(".")).toArray(String[]::new);
+ }
+
private static String getBackupPropsName(int id) {
return String.format(Locale.ROOT, "backup_%d.properties", id);
}
diff --git a/solr/core/src/java/org/apache/solr/core/backup/ShardBackupId.java
b/solr/core/src/java/org/apache/solr/core/backup/ShardBackupId.java
index a2744d04926..cf10075db17 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/ShardBackupId.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/ShardBackupId.java
@@ -31,7 +31,7 @@ import java.util.Arrays;
* @see ShardBackupMetadata
*/
public class ShardBackupId {
- private static final String FILENAME_SUFFIX = ".json";
+ public static final String FILENAME_SUFFIX = ".json";
private final String shardName;
private final BackupId containingBackupId;
diff --git
a/solr/core/src/test/org/apache/solr/core/backup/BackupFilePathsTest.java
b/solr/core/src/test/org/apache/solr/core/backup/BackupFilePathsTest.java
index 8b382ac7a4e..395b42640c4 100644
--- a/solr/core/src/test/org/apache/solr/core/backup/BackupFilePathsTest.java
+++ b/solr/core/src/test/org/apache/solr/core/backup/BackupFilePathsTest.java
@@ -16,12 +16,19 @@
*/
package org.apache.solr.core.backup;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.List;
import java.util.Optional;
+import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.solr.SolrTestCase;
+import org.apache.solr.core.backup.repository.LocalFileSystemRepository;
import org.junit.Test;
/** Unit tests for {@link BackupFilePaths} */
[email protected]("ExtrasFS")
public class BackupFilePathsTest extends SolrTestCase {
@Test
@@ -83,4 +90,41 @@ public class BackupFilePathsTest extends SolrTestCase {
assertTrue(filenameOption.isPresent());
assertEquals(new BackupId(2), filenameOption.get());
}
+
+ @Test
+ public void testExistingBackupLocationIgnoresHiddenEntries() throws
Exception {
+ Path backupDir = backupDirWith(".DS_Store", "coll1/");
+ assertEquals(backupDir.resolve("coll1").toUri(),
existingBackupLocation(backupDir));
+ }
+
+ @Test
+ public void testExistingBackupLocationAllowsLoneHiddenCollection() throws
Exception {
+ Path backupDir = backupDirWith(".coll1/");
+ assertEquals(backupDir.resolve(".coll1").toUri(),
existingBackupLocation(backupDir));
+ }
+
+ @Test
+ public void testExistingBackupLocationRejectsMultipleCollections() throws
Exception {
+ Path backupDir = backupDirWith("coll1/", "coll2/");
+ expectThrows(IllegalStateException.class, () ->
existingBackupLocation(backupDir));
+ }
+
+ private Path backupDirWith(String... entries) throws IOException {
+ Path backupDir = createTempDir().resolve("mybackup");
+ for (String entry : entries) {
+ if (entry.endsWith("/")) {
+ Files.createDirectories(backupDir.resolve(entry));
+ } else {
+ Files.createDirectories(backupDir);
+ Files.writeString(backupDir.resolve(entry), "x");
+ }
+ }
+ return backupDir;
+ }
+
+ private URI existingBackupLocation(Path backupDir) throws IOException {
+ LocalFileSystemRepository repository = new LocalFileSystemRepository();
+ return BackupFilePaths.buildExistingBackupLocationURI(
+ repository, backupDir.getParent().toUri(),
backupDir.getFileName().toString());
+ }
}
diff --git
a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
index fb643e551a4..0896f101d44 100644
---
a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
+++
b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
@@ -57,6 +57,7 @@ import
org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.response.RequestStatusState;
import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.Slice;
@@ -235,6 +236,140 @@ public abstract class AbstractIncrementalBackupTest
extends SolrCloudTestCase {
assertEquals(firstBatchNumDocs,
getNumDocsInCollection(backupCollectionName));
}
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testHiddenFileInBackupDir() throws Exception {
+ setTestSuffix("testHiddenFileInBackupDir");
+ final String backupCollectionName = getCollectionName();
+ final String restoreCollectionName = backupCollectionName + "_restore";
+
+ CloudSolrClient solrClient = cluster.getSolrClient();
+
+ CollectionAdminRequest.createCollection(backupCollectionName, "conf1",
NUM_SHARDS, 1)
+ .process(solrClient);
+ int numDocs = indexDocs(backupCollectionName, true);
+ String backupName = BACKUPNAME_PREFIX + testSuffix;
+ try (BackupRepository repository =
+
cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME))
{
+ String backupLocation =
repository.getBackupLocation(getBackupLocation());
+ RequestStatusState state =
+ CollectionAdminRequest.backupCollection(backupCollectionName,
backupName)
+ .setLocation(backupLocation)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .processAndWait(solrClient, 100);
+ assertEquals(RequestStatusState.COMPLETED, state);
+
+ URI backupNameUri =
+
repository.resolveDirectory(repository.createDirectoryURI(backupLocation),
backupName);
+ try (OutputStream out =
+ repository.createOutput(repository.resolve(backupNameUri,
".DS_Store"))) {
+ out.write(1);
+ }
+
+ state =
+ CollectionAdminRequest.restoreCollection(restoreCollectionName,
backupName)
+ .setLocation(backupLocation)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .processAndWait(solrClient, 500);
+ assertEquals(RequestStatusState.COMPLETED, state);
+
+ AbstractFullDistribZkTestBase.waitForRecoveriesToFinish(
+ restoreCollectionName, ZkStateReader.from(solrClient), false, false,
3);
+ assertEquals(
+ numDocs,
+ solrClient.query(restoreCollectionName, new
SolrQuery("*:*")).getResults().getNumFound());
+
+ CollectionAdminResponse response =
+ CollectionAdminRequest.listBackup(backupName)
+ .setBackupLocation(backupLocation)
+ .setBackupRepository(BACKUP_REPO_NAME)
+ .process(solrClient);
+ assertEquals(1, ((List<Map<String, Object>>)
response.getResponse().get("backups")).size());
+
+ expectThrows(
+ SolrException.class,
+ () ->
+ CollectionAdminRequest.backupCollection(restoreCollectionName,
backupName)
+ .setLocation(backupLocation)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .process(solrClient));
+
+ response =
+ CollectionAdminRequest.deleteBackupById(backupName, 0)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .setLocation(backupLocation)
+ .process(solrClient);
+ assertEquals(0, response.getResponse()._get("deleted[0]/backupId"));
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testHiddenFileInShardBackupMetadataDir() throws Exception {
+ setTestSuffix("testHiddenFileInShardBackupMetadataDir");
+ final String backupCollectionName = getCollectionName();
+
+ CloudSolrClient solrClient = cluster.getSolrClient();
+
+ CollectionAdminRequest.createCollection(backupCollectionName, "conf1",
NUM_SHARDS, 1)
+ .process(solrClient);
+ indexDocs(backupCollectionName, true);
+ String backupName = BACKUPNAME_PREFIX + testSuffix;
+ try (BackupRepository repository =
+
cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME))
{
+ String backupLocation =
repository.getBackupLocation(getBackupLocation());
+ for (int i = 0; i < 3; i++) {
+ RequestStatusState state =
+ CollectionAdminRequest.backupCollection(backupCollectionName,
backupName)
+ .setLocation(backupLocation)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .processAndWait(solrClient, 100);
+ assertEquals(RequestStatusState.COMPLETED, state);
+ }
+
+ URI backupUri =
+ repository.resolveDirectory(
+ repository.createDirectoryURI(backupLocation), backupName,
backupCollectionName);
+ URI shardBackupMetadataDir =
+ new BackupFilePaths(repository,
backupUri).getShardBackupMetadataDir();
+ try (OutputStream out =
+ repository.createOutput(repository.resolve(shardBackupMetadataDir,
".DS_Store"))) {
+ out.write(1);
+ }
+
+ CollectionAdminResponse response =
+ CollectionAdminRequest.deleteBackupById(backupName, 0)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .setLocation(backupLocation)
+ .process(solrClient);
+ assertEquals(0, response.getResponse()._get("deleted[0]/backupId"));
+
+ response =
+ CollectionAdminRequest.deleteBackupByRecency(backupName, 1)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .setLocation(backupLocation)
+ .process(solrClient);
+ assertEquals(1, response.getResponse()._get("deleted[0]/backupId"));
+
+ RequestStatusState state =
+ CollectionAdminRequest.backupCollection(backupCollectionName,
backupName)
+ .setLocation(backupLocation)
+ .setMaxNumberBackupPoints(1)
+ .setRepositoryName(BACKUP_REPO_NAME)
+ .processAndWait(solrClient, 100);
+ assertEquals(RequestStatusState.COMPLETED, state);
+
+ response =
+ CollectionAdminRequest.listBackup(backupName)
+ .setBackupLocation(backupLocation)
+ .setBackupRepository(BACKUP_REPO_NAME)
+ .process(solrClient);
+ List<Map<String, Object>> backups =
+ (List<Map<String, Object>>) response.getResponse().get("backups");
+ assertEquals(List.of(3), backups.stream().map(b ->
b.get("backupId")).toList());
+ }
+ }
+
@SuppressWarnings("unchecked")
@Test
@Nightly
