SOLR-17949: Verify checksums on Azure Blob backup and document flush() no-op

Verify Lucene checksums when copying index files to the Azure Blob
backup repository (mirroring the S3 repository), rewriting the footer
after validation and rejecting too-small or corrupt files. Clarify that
AzureBlobOutputStream.flush() is intentionally a no-op so frequent
flushes cannot exhaust Azure's committed-block limit, and document the
pathExists/isDirectory asymmetry. Add covering tests and group test
helpers and lifecycle methods consistently.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: prateeksinghalgit

solr/modules/azure-blob-repository/build.gradle

@@ -64,7 +64,6 @@ dependencies {
 
   testImplementation libs.azure.core.http.okhttp
   testImplementation libs.squareup.okhttp3.okhttp
-  testImplementation libs.projectreactor.core
 
   // Testcontainers for Azurite integration testing
   testImplementation libs.testcontainers

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobBackupRepository.java

@@ -29,6 +29,9 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
+import org.apache.lucene.codecs.CodecUtil;
+import org.apache.lucene.index.CorruptIndexException;
+import org.apache.lucene.store.ChecksumIndexInput;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
@@ -146,11 +149,7 @@ public void createDirectory(URI path) throws IOException {
       log.debug("Create directory '{}'", blobPath);
     }
 
-    try {
-      client.createDirectory(blobPath);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to create directory " + blobPath, e);
-    }
+    client.createDirectory(blobPath);
   }
 
   @Override
@@ -163,11 +162,7 @@ public void deleteDirectory(URI path) throws IOException {
       log.debug("Delete directory '{}'", blobPath);
     }
 
-    try {
-      client.deleteDirectory(blobPath);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to delete directory " + blobPath, e);
-    }
+    client.deleteDirectory(blobPath);
   }
 
   @Override
@@ -177,13 +172,9 @@ public void delete(URI path, Collection<String> files) throws IOException {
 
     String basePath = getBlobPath(path);
 
-    try {
-      if (!client.isDirectory(basePath)) {
-        int lastSlash = basePath.lastIndexOf('/');
-        basePath = lastSlash >= 0 ? basePath.substring(0, lastSlash) : "";
-      }
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to check path type for " + basePath, e);
+    if (!client.isDirectory(basePath)) {
+      int lastSlash = basePath.lastIndexOf('/');
+      basePath = lastSlash >= 0 ? basePath.substring(0, lastSlash) : "";
     }
 
     final String prefix;
@@ -202,11 +193,7 @@ public void delete(URI path, Collection<String> files) throws IOException {
       log.debug("Delete files '{}'", fullPaths);
     }
 
-    try {
-      client.delete(fullPaths);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to delete files " + fullPaths, e);
-    }
+    client.delete(fullPaths);
   }
 
   @Override
@@ -219,11 +206,7 @@ public boolean exists(URI path) throws IOException {
       log.debug("Check existence '{}'", blobPath);
     }
 
-    try {
-      return client.pathExists(blobPath);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to check existence of " + blobPath, e);
-    }
+    return client.pathExists(blobPath);
   }
 
   @Override
@@ -236,14 +219,10 @@ public PathType getPathType(URI path) throws IOException {
       log.debug("Get path type '{}'", blobPath);
     }
 
-    try {
-      if (client.isDirectory(blobPath)) {
-        return BackupRepository.PathType.DIRECTORY;
-      } else {
-        return BackupRepository.PathType.FILE;
-      }
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to get path type for " + blobPath, e);
+    if (client.isDirectory(blobPath)) {
+      return BackupRepository.PathType.DIRECTORY;
+    } else {
+      return BackupRepository.PathType.FILE;
     }
   }
 
@@ -257,11 +236,7 @@ public String[] listAll(URI path) throws IOException {
       log.debug("List all '{}'", blobPath);
     }
 
-    try {
-      return client.listDir(blobPath);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to list directory " + blobPath, e);
-    }
+    return client.listDir(blobPath);
   }
 
   @Override
@@ -276,11 +251,7 @@ public IndexInput openInput(URI dirPath, String fileName, IOContext ctx) throws
       log.debug("Open input '{}'", blobPath);
     }
 
-    try {
-      return new AzureBlobIndexInput(client, blobPath, client.length(blobPath));
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to open input stream for " + blobPath, e);
-    }
+    return new AzureBlobIndexInput(client, blobPath, client.length(blobPath));
   }
 
   @Override
@@ -293,11 +264,7 @@ public OutputStream createOutput(URI path) throws IOException {
       log.debug("Create output '{}'", blobPath);
     }
 
-    try {
-      return client.pushStream(blobPath);
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to create output stream for " + blobPath, e);
-    }
+    return client.pushStream(blobPath);
   }
 
   @Override
@@ -330,18 +297,29 @@ public void copyIndexFileFrom(
       // ignore; write will surface real issues
     }
 
-    try (IndexInput input = sourceDir.openInput(sourceFileName, IOContext.DEFAULT);
-        OutputStream output = client.pushStream(blobPath)) {
-      byte[] buffer = new byte[COPY_BUFFER_SIZE];
-      long remaining = input.length();
-      while (remaining > 0) {
-        int toRead = (int) Math.min(buffer.length, remaining);
-        input.readBytes(buffer, 0, toRead);
-        output.write(buffer, 0, toRead);
-        remaining -= toRead;
+    try (IndexInput input =
+        shouldVerifyChecksum
+            ? sourceDir.openChecksumInput(sourceFileName)
+            : sourceDir.openInput(sourceFileName, IOContext.READONCE)) {
+      if (input.length() <= CodecUtil.footerLength()) {
+        throw new CorruptIndexException("file is too small:" + input.length(), input);
+      }
+
+      try (OutputStream output = client.pushStream(blobPath)) {
+        byte[] buffer = new byte[COPY_BUFFER_SIZE];
+        long remaining =
+            shouldVerifyChecksum ? input.length() - CodecUtil.footerLength() : input.length();
+        while (remaining > 0) {
+          int toRead = (int) Math.min(buffer.length, remaining);
+          input.readBytes(buffer, 0, toRead);
+          output.write(buffer, 0, toRead);
+          remaining -= toRead;
+        }
+        if (shouldVerifyChecksum) {
+          long checksum = CodecUtil.checkFooter((ChecksumIndexInput) input);
+          writeFooter(checksum, output);
+        }
       }
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to copy file from " + sourceFileName + " to " + blobPath, e);
     }
   }
 
@@ -379,8 +357,6 @@ public void copyIndexFileTo(
       while ((len = inputStream.read(buffer)) != -1) {
         indexOutput.writeBytes(buffer, 0, len);
       }
-    } catch (AzureBlobException e) {
-      throw new IOException("Failed to copy file from " + blobPath + " to " + destFileName, e);
     }
 
     long timeElapsed = Duration.between(start, Instant.now()).toMillis();
@@ -403,4 +379,33 @@ private String getBlobPath(URI uri) {
     }
     return uri.getPath();
   }
+
+  private void writeFooter(long checksum, OutputStream outputStream) throws IOException {
+    IndexOutput out =
+        new IndexOutput("", "") {
+          @Override
+          public void writeByte(byte b) throws IOException {
+            outputStream.write(b);
+          }
+
+          @Override
+          public void writeBytes(byte[] b, int offset, int length) throws IOException {
+            outputStream.write(b, offset, length);
+          }
+
+          @Override
+          public void close() {}
+
+          @Override
+          public long getFilePointer() {
+            return 0;
+          }
+
+          @Override
+          public long getChecksum() {
+            return checksum;
+          }
+        };
+    CodecUtil.writeFooter(out);
+  }
 }

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobException.java

@@ -16,11 +16,17 @@
  */
 package org.apache.solr.azureblob;
 
+import java.io.IOException;
+
 /**
  * Generic exception for Blob Storage related failures. Could originate from the {@link
  * AzureBlobBackupRepository} or from its underlying {@link AzureBlobStorageClient}.
  */
-public class AzureBlobException extends Exception {
+public class AzureBlobException extends IOException {
+  public AzureBlobException(Throwable cause) {
+    super(cause);
+  }
+
   public AzureBlobException(String message) {
     super(message);
   }

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobIndexInput.java

@@ -143,12 +143,7 @@ private void ensureStreamAt(long targetAbsolutePos) throws IOException {
           "read past EOF: pos=" + targetAbsolutePos + " vs end=" + (absoluteOffset + length));
     }
 
-    try {
-      inputStream = client.pullRangeStream(path, targetAbsolutePos, remaining);
-    } catch (AzureBlobException e) {
-      throw new IOException(
-          "Failed to open range stream for " + path + " at offset " + targetAbsolutePos, e);
-    }
+    inputStream = client.pullRangeStream(path, targetAbsolutePos, remaining);
     streamAbsolutePos = targetAbsolutePos;
   }
 

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobNotFoundException.java

@@ -18,6 +18,10 @@
 
 /** Exception thrown when a blob is not found in Azure Blob Storage. */
 public class AzureBlobNotFoundException extends AzureBlobException {
+  public AzureBlobNotFoundException(Throwable cause) {
+    super(cause);
+  }
+
   public AzureBlobNotFoundException(String message) {
     super(message);
   }

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobOutputStream.java

@@ -36,7 +36,7 @@
  * OutputStream implementation for Azure Blob Storage using block blobs. Supports chunked uploads
  * for large files.
  */
-public class AzureBlobOutputStream extends OutputStream {
+class AzureBlobOutputStream extends OutputStream {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   static final int BLOCK_SIZE = 4 * 1024 * 1024;
@@ -47,7 +47,7 @@ public class AzureBlobOutputStream extends OutputStream {
   private final ByteBuffer buffer;
   private BlockUpload blockUpload;
 
-  public AzureBlobOutputStream(BlobClient blobClient, String blobPath) {
+  AzureBlobOutputStream(BlobClient blobClient, String blobPath) {
     this.blobClient = blobClient;
     this.blobPath = blobPath;
     this.closed = false;
@@ -138,9 +138,9 @@ public void flush() throws IOException {
       throw new IOException("Stream closed");
     }
 
-    if (buffer.position() > 0) {
-      uploadBlock();
-    }
+    // Intentionally a no-op. Full blocks are staged as the buffer fills in write(), and the
+    // partial tail is staged in close(). Staging on every flush() would create tiny blocks and a
+    // frequently-flushing caller could exhaust Azure's 50,000-committed-block limit on small files.
   }
 
   @Override

solr/modules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobStorageClient.java

@@ -105,13 +105,6 @@ public class AzureBlobStorageClient {
   AzureBlobStorageClient(BlobServiceClient blobServiceClient, String containerName) {
     this.containerClient = blobServiceClient.getBlobContainerClient(containerName);
     this.batchClient = new BlobBatchClientBuilder(blobServiceClient).buildClient();
-    try {
-      containerClient.create();
-    } catch (BlobStorageException e) {
-      if (e.getStatusCode() != HTTP_CONFLICT) {
-        throw e;
-      }
-    }
   }
 
   private static BlobServiceClient createInternalClient(
@@ -235,6 +228,13 @@ String[] listDir(String path) throws AzureBlobException {
     }
   }
 
+  /**
+   * Checks existence by resolving the exact blob (a HEAD request). This module always writes {@code
+   * hdi_isfolder} marker blobs for directories, so it is self-consistent. Note the asymmetry with
+   * {@link #isDirectory(String)}: a marker-less "virtual" directory created by an external tool
+   * (e.g. azcopy) returns {@code false} here even though {@code isDirectory} reports it as a
+   * directory via prefix listing.
+   */
   boolean pathExists(String path) throws AzureBlobException {
     final String blobPath = sanitizedPath(path);
 
@@ -434,7 +434,21 @@ OutputStream pushStream(String path) throws AzureBlobException {
     }
   }
 
-  void close() {}
+  void close() {
+    // No-op: the underlying OkHttp client is SPI-loaded and shared process-wide, so there is
+    // nothing per-instance to release here.
+  }
+
+  @VisibleForTesting
+  void createContainerForTests() {
+    try {
+      containerClient.create();
+    } catch (BlobStorageException e) {
+      if (e.getStatusCode() != HTTP_CONFLICT) {
+        throw e;
+      }
+    }
+  }
 
   @VisibleForTesting
   void deleteContainerForTests() {