This is an automated email from the ASF dual-hosted git repository.

gerlowskija 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 f3a798e424b SOLR-18502: INSTALLSHARD skips write.lock download (#5026)
f3a798e424b is described below

commit f3a798e424bb933ba988c8d316d0e8f35c8b6e0f
Author: Jason Gerlowski <[email protected]>
AuthorDate: Mon Oct 12 06:38:52 2026 -0400

    SOLR-18502: INSTALLSHARD skips write.lock download (#5026)
---
 ...LR-18502-ignore-write-locks-in-installshard.yml |  8 ++++
 .../java/org/apache/solr/handler/RestoreCore.java  |  6 +++
 .../api/collections/AbstractInstallShardTest.java  | 46 +++++++++++++++++++++-
 .../apache/solr/core/TrackingBackupRepository.java | 13 ++++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git 
a/changelog/unreleased/SOLR-18502-ignore-write-locks-in-installshard.yml 
b/changelog/unreleased/SOLR-18502-ignore-write-locks-in-installshard.yml
new file mode 100644
index 00000000000..fa929f97131
--- /dev/null
+++ b/changelog/unreleased/SOLR-18502-ignore-write-locks-in-installshard.yml
@@ -0,0 +1,8 @@
+title: >
+  INSTALLSHARD now ignores 'write.lock' file if present in offline index data.
+type: changed
+authors:
+  - name: Jason Gerlowski
+links:
+  - name: SOLR-18502
+    url: https://issues.apache.org/jira/browse/SOLR-18502
diff --git a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java 
b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
index 2d5e1f2fa92..fecfc6d58de 100644
--- a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
+++ b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
@@ -34,6 +34,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import org.apache.lucene.codecs.CodecUtil;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
@@ -145,6 +146,11 @@ public class RestoreCore implements Callable<Boolean> {
       for (String filename : repository.listAllFiles()) {
         checkInterrupted();
 
+        // Lock files only make sense for the process that created them, and 
carry no checksum
+        if (IndexWriter.WRITE_LOCK_NAME.equals(filename)) {
+          continue;
+        }
+
         // Capture variables for lambda
         final String filenameFinal = filename;
         final boolean fileExistsLocally = indexDirFiles.contains(filename);
diff --git 
a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
 
b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
index 86d12f4a7bd..17e20cbeed8 100644
--- 
a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
+++ 
b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
@@ -35,6 +35,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.store.Directory;
 import org.apache.solr.client.solrj.RemoteSolrException;
 import org.apache.solr.client.solrj.SolrClient;
@@ -50,6 +51,7 @@ import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.core.DirectoryFactory;
 import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.TrackingBackupRepository;
 import org.apache.solr.core.backup.repository.BackupRepository;
 import org.apache.solr.handler.admin.api.InstallShardData;
 import org.junit.After;
@@ -187,6 +189,41 @@ public abstract class AbstractInstallShardTest extends 
SolrCloudTestCase {
     assertCollectionHasNumDocs(collectionName, singleShardNumDocs);
   }
 
+  @Test
+  public void testInstallSkipsWriteLockFile() throws Exception {
+    final String collectionName = createAndAwaitEmptyCollection(1, 
replicasPerShard);
+    deleteAfterTest(collectionName);
+    enableReadOnly(collectionName);
+
+    // Offline index data _shouldn't_ contain a "write.lock", but it's common 
for users
+    // to copy a live index dir, so the file is often included regardless.  
Make sure
+    // write.lock is present to simulate this case so that we can ensure 
INSTALLSHARD
+    // skips it gracefully.
+    final CoreContainer cc = cluster.getJettySolrRunner(0).getCoreContainer();
+    final URI lockUri;
+    try (final BackupRepository repo = 
cc.newBackupRepository(BACKUP_REPO_NAME)) {
+      lockUri = repo.resolve(singleShard1Uri, "write.lock");
+      assertTrue("Test data should include a write.lock", 
repo.exists(lockUri));
+    }
+
+    TrackingBackupRepository.clear();
+    CollectionAdminRequest.installDataToShard(
+            collectionName, "shard1", singleShard1Uri.toString(), 
BACKUP_REPO_NAME)
+        .process(cluster.getSolrClient());
+    waitForState(
+        "The core-installs should complete and all replicas become healthy",
+        collectionName,
+        30,
+        TimeUnit.SECONDS,
+        SolrCloudTestCase.activeClusterShape(1, replicasPerShard));
+
+    assertCollectionHasNumDocs(collectionName, singleShardNumDocs);
+
+    final List<URI> restored = TrackingBackupRepository.restoredFiles();
+    assertFalse("Expected other index files to be installed", 
restored.isEmpty());
+    assertFalse("write.lock should not be copied during install", 
restored.contains(lockUri));
+  }
+
   @Test
   public void testInstallReportsErrorsAppropriately() throws Exception {
     final String collectionName = createAndAwaitEmptyCollection(1, 
replicasPerShard);
@@ -355,7 +392,14 @@ public abstract class AbstractInstallShardTest extends 
SolrCloudTestCase {
                   core.getSolrConfig().indexConfig.lockType);
       try {
         for (final String dirContent : dir.listAll()) {
-          if (dirContent.contains("write.lock")) continue;
+          if (dirContent.equals(IndexWriter.WRITE_LOCK_NAME)) {
+            // Backup upload requires a checksum footer that lock files lack, 
so create it directly.
+            // Real-world "offline" indices copied from a live index dir often 
contain one.
+            backupRepository
+                .createOutput(backupRepository.resolve(destinationUri, 
dirContent))
+                .close();
+            continue;
+          }
           backupRepository.copyFileFrom(dir, dirContent, destinationUri);
         }
       } finally {
diff --git 
a/solr/test-framework/src/java/org/apache/solr/core/TrackingBackupRepository.java
 
b/solr/test-framework/src/java/org/apache/solr/core/TrackingBackupRepository.java
index f81d59d1d4a..4148bc667ab 100644
--- 
a/solr/test-framework/src/java/org/apache/solr/core/TrackingBackupRepository.java
+++ 
b/solr/test-framework/src/java/org/apache/solr/core/TrackingBackupRepository.java
@@ -28,6 +28,7 @@ import 
org.apache.solr.core.backup.repository.DelegatingBackupRepository;
 
 public class TrackingBackupRepository extends DelegatingBackupRepository {
   private static final List<URI> COPIED_FILES = 
Collections.synchronizedList(new ArrayList<>());
+  private static final List<URI> RESTORED_FILES = 
Collections.synchronizedList(new ArrayList<>());
   private static final List<URI> DIRECTORIES_CREATED =
       Collections.synchronizedList(new ArrayList<>());
   private static final List<URI> OUTPUTS_CREATED = 
Collections.synchronizedList(new ArrayList<>());
@@ -52,6 +53,12 @@ public class TrackingBackupRepository extends 
DelegatingBackupRepository {
     super.copyIndexFileFrom(sourceDir, sourceFileName, destDir, destFileName);
   }
 
+  @Override
+  public void copyFileTo(URI sourceRepo, String fileName, Directory dest) 
throws IOException {
+    RESTORED_FILES.add(resolve(sourceRepo, fileName));
+    super.copyFileTo(sourceRepo, fileName, dest);
+  }
+
   /**
    * @return list of files were copied by using {@link 
#copyFileFrom(Directory, String, URI)}
    */
@@ -59,6 +66,11 @@ public class TrackingBackupRepository extends 
DelegatingBackupRepository {
     return new ArrayList<>(COPIED_FILES);
   }
 
+  /** Repository files that were copied into a Directory, i.e. 
restored/installed */
+  public static List<URI> restoredFiles() {
+    return new ArrayList<>(RESTORED_FILES);
+  }
+
   public static List<URI> directoriesCreated() {
     return new ArrayList<>(DIRECTORIES_CREATED);
   }
@@ -70,6 +82,7 @@ public class TrackingBackupRepository extends 
DelegatingBackupRepository {
   /** Clear all tracking data */
   public static void clear() {
     COPIED_FILES.clear();
+    RESTORED_FILES.clear();
     DIRECTORIES_CREATED.clear();
     OUTPUTS_CREATED.clear();
   }

Reply via email to