From c7e4dc85970079ffccf206c0734c6cdac5a073fc Mon Sep 17 00:00:00 2001 From: Prithvi S Date: Wed, 12 Aug 2026 19:55:50 +0530 Subject: [PATCH] GCSBackupRepository.copyIndexFileTo must not swallow failures Signed-off-by: prithvi --- .../unreleased/gcs-copyIndexFileTo-fix.yml | 5 + .../apache/solr/gcs/GCSBackupRepository.java | 36 ++-- .../solr/gcs/GCSBackupRepositoryTest.java | 169 ++++++++++++++++++ 3 files changed, 193 insertions(+), 17 deletions(-) create mode 100644 changelog/unreleased/gcs-copyIndexFileTo-fix.yml diff --git a/changelog/unreleased/gcs-copyIndexFileTo-fix.yml b/changelog/unreleased/gcs-copyIndexFileTo-fix.yml new file mode 100644 index 000000000000..c99b24825892 --- /dev/null +++ b/changelog/unreleased/gcs-copyIndexFileTo-fix.yml @@ -0,0 +1,5 @@ +title: > + Fixed GCS backup restores silently swallowing failures and stopping early on zero-byte reads (SOLR-18250). +type: fixed +authors: + - name: Prithvi S diff --git a/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java b/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java index 3d176a5c3d29..ae767aa2e432 100644 --- a/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java +++ b/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java @@ -361,24 +361,26 @@ public void copyIndexFileFrom( public void copyIndexFileTo( URI sourceRepo, String sourceFileName, Directory dest, String destFileName) throws IOException { - try { - String blobName = sourceRepo.toString(); - blobName = appendTrailingSeparatorIfNecessary(blobName); - blobName += sourceFileName; - final BlobId blobId = BlobId.of(bucketName, blobName); - try (final ReadChannel readChannel = storage.reader(blobId); - IndexOutput output = - dest.createOutput(destFileName, DirectoryFactory.IOCONTEXT_NO_CACHE)) { - ByteBuffer buffer = ByteBuffer.allocate(readBufferSizeBytes); - while (readChannel.read(buffer) > 0) { - buffer.flip(); - byte[] arr = buffer.array(); - output.writeBytes(arr, buffer.position(), buffer.limit() - buffer.position()); - buffer.clear(); - } + String blobName = sourceRepo.toString(); + blobName = appendTrailingSeparatorIfNecessary(blobName); + blobName += sourceFileName; + final BlobId blobId = BlobId.of(bucketName, blobName); + try (final ReadChannel readChannel = storage.reader(blobId); + IndexOutput output = dest.createOutput(destFileName, DirectoryFactory.IOCONTEXT_NO_CACHE)) { + ByteBuffer buffer = ByteBuffer.allocate(readBufferSizeBytes); + while (readChannel.read(buffer) != -1) { + buffer.flip(); + byte[] arr = buffer.array(); + output.writeBytes(arr, buffer.position(), buffer.limit() - buffer.position()); + buffer.clear(); } - } catch (Exception e) { - log.info("Here's an exception e", e); + } catch (IOException e) { + log.error("Failed to copy index file from GCS: {}/{}", bucketName, blobName, e); + throw e; + } catch (RuntimeException e) { + log.error("Failed to copy index file from GCS: {}/{}", bucketName, blobName, e); + throw new IOException( + "Failed to copy index file from GCS: " + bucketName + "/" + blobName, e); } } diff --git a/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java b/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java index 10d2acb2bd69..247939f8dc25 100644 --- a/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java +++ b/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java @@ -21,10 +21,26 @@ import static org.apache.solr.gcs.GCSConfigParser.GCS_BUCKET_ENV_VAR_NAME; import static org.apache.solr.gcs.GCSConfigParser.GCS_CREDENTIAL_ENV_VAR_NAME; +import com.google.cloud.ReadChannel; +import com.google.cloud.storage.BlobId; +import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.StorageException; +import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper; +import java.io.IOException; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.net.URI; import java.net.URISyntaxException; +import java.nio.ByteBuffer; import java.util.HashMap; import java.util.Map; +import org.apache.lucene.store.ByteBuffersDirectory; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexInput; import org.apache.solr.cloud.api.collections.AbstractBackupRepositoryTest; import org.apache.solr.common.util.NamedList; import org.apache.solr.core.backup.repository.BackupRepository; @@ -77,4 +93,157 @@ public void testInitStoreDoesNotFailWithMissingCredentials() { gcsBackupRepository.init(new NamedList<>(config)); } + + @Test + public void testCopyIndexFileToPropagatesReadFailures() throws Exception { + Storage failingStorage = createFailingStorage(); + GCSBackupRepository repo = createRepositoryWithStorage(failingStorage); + + try (Directory dest = new ByteBuffersDirectory()) { + URI sourceDir = repo.resolve(getBaseUri(), "backup"); + IOException thrown = + expectThrows( + IOException.class, + () -> repo.copyIndexFileTo(sourceDir, "any.dat", dest, "dest.dat")); + assertTrue(thrown.getMessage().contains("Failed to copy index file from GCS")); + assertNotNull(thrown.getCause()); + assertTrue(thrown.getCause() instanceof StorageException); + assertEquals("simulated GCS read failure", thrown.getCause().getMessage()); + } + } + + @Test + public void testCopyIndexFileToHandlesZeroByteReads() throws Exception { + Storage realStorage = LocalStorageHelper.customOptions(false).getService(); + byte[] data = new byte[100]; + random().nextBytes(data); + // "solrBackupsBucket" matches GCSConfigParser.DEFAULT_GCS_BUCKET_VALUE + String bucketName = "solrBackupsBucket"; + + GCSBackupRepository repo = createRepositoryWithStorage(realStorage); + URI sourceDir = repo.resolve(getBaseUri(), "backup"); + BlobId blobId = BlobId.of(bucketName, sourceDir + "/source.dat"); + realStorage.create(BlobInfo.newBuilder(blobId).build(), data); + + Storage zeroReturningStorage = createZeroReturningStorage(realStorage); + GCSBackupRepository proxyRepo = createRepositoryWithStorage(zeroReturningStorage); + + try (Directory dest = new ByteBuffersDirectory()) { + proxyRepo.copyIndexFileTo(sourceDir, "source.dat", dest, "dest.dat"); + try (IndexInput in = dest.openInput("dest.dat", IOContext.DEFAULT)) { + assertEquals(data.length, in.length()); + byte[] read = new byte[data.length]; + in.readBytes(read, 0, data.length); + assertArrayEquals(data, read); + } + } + } + + @Test + public void testCopyIndexFileToCopiesFile() throws Exception { + Storage realStorage = LocalStorageHelper.customOptions(false).getService(); + byte[] data = new byte[100]; + random().nextBytes(data); + // "solrBackupsBucket" matches GCSConfigParser.DEFAULT_GCS_BUCKET_VALUE + String bucketName = "solrBackupsBucket"; + + GCSBackupRepository repo = createRepositoryWithStorage(realStorage); + URI sourceDir = repo.resolve(getBaseUri(), "backup"); + BlobId blobId = BlobId.of(bucketName, sourceDir + "/source.dat"); + realStorage.create(BlobInfo.newBuilder(blobId).build(), data); + + try (Directory dest = new ByteBuffersDirectory()) { + repo.copyIndexFileTo(sourceDir, "source.dat", dest, "dest.dat"); + try (IndexInput in = dest.openInput("dest.dat", IOContext.DEFAULT)) { + assertEquals(data.length, in.length()); + byte[] read = new byte[data.length]; + in.readBytes(read, 0, data.length); + assertArrayEquals(data, read); + } + } + } + + private static Storage createFailingStorage() { + Storage delegate = LocalStorageHelper.customOptions(false).getService(); + return (Storage) + Proxy.newProxyInstance( + Storage.class.getClassLoader(), + new Class[] {Storage.class}, + (proxy, method, args) -> { + if ("reader".equals(method.getName())) { + throw new StorageException(0, "simulated GCS read failure"); + } + return invokeAndUnwrap(method, delegate, args); + }); + } + + private static Storage createZeroReturningStorage(Storage delegate) { + return (Storage) + Proxy.newProxyInstance( + Storage.class.getClassLoader(), + new Class[] {Storage.class}, + (proxy, method, args) -> { + if ("reader".equals(method.getName()) + && args != null + && args.length == 1 + && args[0] instanceof BlobId) { + ReadChannel realChannel = (ReadChannel) invokeAndUnwrap(method, delegate, args); + return createZeroFirstReadChannel(realChannel); + } + return invokeAndUnwrap(method, delegate, args); + }); + } + + private static ReadChannel createZeroFirstReadChannel(ReadChannel delegate) { + return (ReadChannel) + Proxy.newProxyInstance( + ReadChannel.class.getClassLoader(), + new Class[] {ReadChannel.class}, + new InvocationHandler() { + private boolean returnedZero = false; + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if ("read".equals(method.getName()) + && args != null + && args.length == 1 + && args[0] instanceof ByteBuffer) { + if (!returnedZero) { + returnedZero = true; + return 0; + } + } + return invokeAndUnwrap(method, delegate, args); + } + }); + } + + private static Object invokeAndUnwrap(Method method, Object target, Object[] args) + throws Throwable { + try { + return method.invoke(target, args); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + } + + private GCSBackupRepository createRepositoryWithStorage(Storage storage) { + TestGCSBackupRepository repo = new TestGCSBackupRepository(storage); + repo.init(getBaseBackupRepositoryConfiguration()); + return repo; + } + + private static class TestGCSBackupRepository extends GCSBackupRepository { + private final Storage testStorage; + + TestGCSBackupRepository(Storage testStorage) { + this.testStorage = testStorage; + } + + @Override + protected Storage initStorage() { + this.storage = testStorage; + return testStorage; + } + } }