SOLR-17949: Add Azure Blob Storage backup repository module by prateeksinghalgit · Pull Request #3750 · apache/solr

prateeksinghalgit · 2025-10-09T20:09:33Z

Implements Azure Blob Storage backup repository as discussed in SOLR-17949.

Description

This PR adds a new backup repository implementation for Azure Blob Storage, enabling Solr collections to be backed up to and restored from Microsoft Azure.

Key Features:

Full backup/restore functionality to Azure Blob Storage
Support for 4 authentication methods (Connection String, Account Key, SAS Token, Azure Identity)
Incremental backup support with versioning
Data integrity verification (checksum validation)
Compatible with Azurite emulator for local testing
Comprehensive documentation and 76 passing unit tests

Solution

The implementation follows Solr's BackupRepository interface pattern, similar to existing S3 and GCS repository modules:

AzureBlobBackupRepository: Main class implementing Solr's BackupRepository interface
AzureBlobStorageClient: Wrapper for Azure SDK, providing file operations
AzureBlobIndexInput: Custom Lucene IndexInput for reading from Azure blobs
AzureBlobOutputStream: Custom output stream for writing to Azure blobs
Authentication: Supports 4 methods via flexible configuration in solr.xml

All streaming operations are compatible with Solr's ResumableInputStream for fault-tolerant transfers.

Implementation stats:

8 implementation files (1,606 LOC)
8 test files (2,180 LOC)
All dependencies Apache 2.0 licensed

Tests

Unit Tests: 76/76 passing (100%)

./gradlew :solr:modules:blob-repository:test
# Result: BUILD SUCCESSFUL - 76 test(s)

Test Coverage:

Basic read/write operations
Large file handling (1GB+)
Binary data integrity
Concurrent operations
Stream lifecycle (close/resume behavior)
Incremental backups
All 4 authentication methods
Integration with Azurite (local emulator)
Integration with real Azure Blob Storage

Testing Instructions:
Can be tested locally with Azurite emulator (no Azure account needed) or with real Azure Blob Storage. See solr/modules/blob-repository/README.md for detailed setup instructions.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew :solr:modules:blob-repository:check (module-specific check passed).
I have added tests for my changes.
I have added documentation for the Reference Guide

pratsgit · 2025-11-20T05:43:55Z

Quick update: following up on the dev@ thread.

To clarify scope — although the diff is large, the actual implementation is centered in 8 main files and 8 test files; the rest are license header updates.

Happy to split the PR into smaller logical chunks (core module / tests / docs) if that helps with review.

Thanks everyone!

janhoy · 2025-11-20T07:30:08Z

Don’t have time to review but asked copilot for an opinion 😉

janhoy

Only immediate comment is naming. «Blob-repository» is too generic. Should contain word «azure»?

Copilot

Pull Request Overview

This PR implements Azure Blob Storage backup repository support for Apache Solr, enabling collections to be backed up to and restored from Microsoft Azure. The implementation follows established patterns from existing S3 and GCS modules, providing 4 authentication methods (Connection String, Account Key, SAS Token, Azure Identity), incremental backup support, and comprehensive documentation with 76 passing unit tests.

Reviewed Changes

Copilot reviewed 70 out of 70 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
backup-restore.adoc	Adds comprehensive documentation for BlobBackupRepository with authentication methods and configuration
BlobBackupRepository.java	Main repository implementation following Solr's BackupRepository interface
BlobStorageClient.java	Azure SDK wrapper providing blob storage operations
BlobOutputStream.java	Custom output stream for block-based blob uploads
BlobIndexInput.java	Lucene IndexInput implementation with page caching
Test files	8 test classes with 76 passing tests covering all functionality
build.gradle	Module build configuration with Azure SDK dependencies
License files	License and notice files for Azure and Netty dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This commit adds support for backing up and restoring Solr collections to Azure Blob Storage with multiple authentication options. Features: - Full backup/restore functionality to Azure Blob Storage - Support for 4 authentication methods: * Connection String (for development) * Account Name + Key (for simple production) * SAS Token (recommended for production) * Azure Identity (Managed Identity, Service Principal, Azure CLI) - Incremental backup support with versioning - Data integrity verification (checksum validation) - Compatible with Azurite emulator for local testing - Comprehensive documentation and 76 passing unit tests Implementation: - 8 implementation files (1,606 LOC) - 8 test files (2,180 LOC) - All dependencies Apache 2.0 licensed - Follows Solr's backup repository patterns

- Renamed module from blob-repository to azure-blob-repository - Renamed all classes from Blob* to AzureBlob* for clarity - Updated package from org.apache.solr.blob to org.apache.solr.azureblob - Added Azure SDK dependencies (azure-storage-blob, azure-identity) - Updated Solr Reference Guide with Azure Blob Storage documentation - Added .gitignore entries for Azurite test infrastructure All authentication methods tested successfully with real Azure Blob Storage: - Connection String authentication - Account Name + Key authentication - SAS Token authentication - Service Principal (Azure Identity) authentication Testing completed with 100% success rate on backup/restore operations.

prateeksinghalgit · 2025-11-22T20:43:04Z

Only immediate comment is naming. «Blob-repository» is too generic. Should contain word «azure»?

made the change to azure-blob-repository

- Switch from Netty to OkHttp for better Security Manager compatibility - Use static shared HttpClient for better resource management - Fix licenses: msal4j and Azure SDK are MIT licensed - Add changelog entry - Add JFR permissions for Reactor

janhoy · 2025-11-25T08:10:12Z

I'll leave the rest of the review to others more proficient in Azure Blob than me (have never used it). I'd love to test it with a read AzBlob but I'll leave that to someone who already have an active account.

I have a concern that the PR feels largely AI generated(?), lacking the care to details that we require for contributions. Have me excused @prateeksinghalgit if this is not correct, it was just a hunch I got while reviewing. I'd rather have a short well thought out README with useful advice for users than three pages of detailed step by step instructions for testing and developing the feature.

For other reviewers to pick up where I left, consider in partifular the HTTP client choice, correct licensing and avoiding hard coded ports in tests. I have not done a complete review, not read the ref-guide part at all, just commented on things me and Copilot saw immediately.

pratsgit · 2025-12-01T07:06:56Z

Hi all,

I’ve pushed a set of updates based on the feedback so far:

Switched the integration tests to use Testcontainers with Azurite (no hard-coded ports / external prereqs) and disabled the Security Manager for this module’s tests, following the pattern used in the extraction module.
Addressed HTTP client concerns by using azure-core-http-okhttp and keeping those deps in test scope only, plus explicit test deps.
Cleaned up and significantly shortened the README and docs
Did additional cleanup to align naming, structure, and configuration patterns with those of the existing repository plugins.

Please let me know if anything else should be adjusted that would help move the review forward.

Happy to make further changes.

Thanks again for the thoughtful review and guidance so far!

— Prateek

prateeksinghalgit · 2025-12-02T07:22:19Z

Hi All,

I’ve fixed the recent CI failures:

Added proper thread leak filters for Testcontainers / JNA so tests are cleanly skipped when Docker isn’t available in CI.
Fixed two error-prone findings (IntLongMath and MissingOverride) that only surfaced in CI because error-prone is disabled by default in local builds unless -Pvalidation.errorprone=true is used.

Please let me know if anything else should be adjusted to help move the review forward. I’m happy to iterate further as needed.

- Use Testcontainers (Azurite) for integration tests to avoid hardcoded ports and external dependencies - Disable Security Manager for Azure Blob tests to support Testcontainers (similar to extraction module) - Fix OkHttp compilation error by adding explicit testImplementation - Update documentation: BlobBackupRepository -> AzureBlobBackupRepository

github-actions · 2026-03-16T00:19:58Z

This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!

prateeksinghalgit · 2026-04-21T18:45:37Z

Thanks @janhoy for adding more reviewers. @HoustonPutman and @psalagnac ,let me know if you have any questions related to the pr.

psalagnac · 2026-04-27T12:30:32Z

Thanks for adding me as a reviewer.
I've been pretty busy recently and I haven't got the chance to do the review yet. I'll try to do a complete review before end of next week.

Made-with: Cursor

…eam merge Made-with: Cursor

psalagnac

Only reviewed the production code (skiped tests and other stuff).

Some general comments on the review:

can you replace all unnecessary qualified class names by imports? (replacing java.lang.String by String) This makes the code painful to read
there is a lot of shared logic between Azure backups and S3 backups. Lot of code is duplicated. I wonder if abstract classes should be introduced to factor out some logic
we always upload files with a block list. At least for small files, can we keep things simples with a simple pull? It think we may skip the commit call.
overall, I think it's pretty inefficient that AzureBlobIndexInput always reads files by small pages. Mostly for backup/restore, we will only fetch full files. You should not do a request by page, or use much bigger pages (without keeping them in memory).

Please note I did a code review, but I have no way to test it with Azure.

psalagnac · 2026-05-07T15:45:04Z

+
+    String destPath = getBlobPath(dest);
+
+    String blobPath = destPath.endsWith("/") ? destPath + destFileName : destPath;


I think something is wrong here. We fully drop destFileName when destPath does not end with a /

yes, updated it.

psalagnac · 2026-05-07T15:55:09Z

+      String accountKey,
+      String sasToken,
+      String tenantId,
+      String clientId,


clientId and clientSecret are unused. These parameters could be removed from this class.

using them now to build, ClientSecretCredentialBuilder. Helps with users using service principal authentication method to do solr backup and restore to azure blob storage.

psalagnac · 2026-05-07T16:18:11Z

+
+    try {
+      BlobClient blobClient = containerClient.getBlobClient(blobPath);
+      final long contentLength = blobClient.getProperties().getBlobSize();


This is under optimized. We do two queries to open the stream. My understanding is invoking blobClient.getProperties() issues a HEAD query.

The result of blobClient.openInputStream() also contains the length. This may be retrieved with blobInputStream.getProperties().getBlobSize()

psalagnac · 2026-05-07T16:30:58Z

+
+      try {
+        BlobClient blobClient = containerClient.getBlobClient(sanitizedDirPath);
+        blobClient.upload(new ByteArrayInputStream(new byte[0]), 0, true);


This is under optimized. We do two requests to create an empty object. Metadata should be set in the initial request.

Something like this should work:

BlobParallelUploadOptions options = new BlobParallelUploadOptions(new ByteArrayInputStream(new byte[0])) .setMetadata(metadata); blobClient.uploadWithResponse(options, null, Context.NONE);

psalagnac · 2026-05-07T16:46:05Z

+
+  private Collection<String> deleteBlobs(Collection<String> paths) throws AzureBlobException {
+    try {
+      return deleteBlobs(paths, DELETE_BATCH_SIZE);


DELETE_BATCH_SIZE has no impact (unused parameter). Does it come from S3 code?

psalagnac · 2026-05-07T17:45:20Z

+    }
+
+    @Override
+    public void clear() {


Useless override

psalagnac · 2026-05-07T17:50:03Z

+    }
+
+    AzureBlobIndexInput slice =
+        new AzureBlobIndexInput(


This seems broken to me. We pass the description as path. If we create a slice from a slice, there is a high change we end if fetching a bad object.

psalagnac · 2026-05-07T17:52:00Z

+        new AzureBlobIndexInput(
+            getFullSliceDescription(sliceDescription), client, length, pageSize, MAX_CACHED_PAGES);
+
+    slice.position = 0L;


Position is already set to 0 at construction

psalagnac · 2026-05-07T17:53:44Z

+  }
+
+  // Internal state for slices: base offset to add to all range requests
+  private long clientViewBaseOffset = 0L;


This should be immutable and passed to the constructor.
Also, I'm rather it is moved with other class members.

psalagnac · 2026-05-07T17:57:49Z

+  public void seek(long pos) throws IOException {
+    ensureOpen();
+    if (pos < 0 || pos > length) {
+      throw new IOException("Seek position out of bounds: " + pos);


Should be EOFException.

Adopt Azure SDK BOM 1.3.6 and Azurite 3.35.0 to enable the single-call pullStream pattern. Rewrite AzureBlobIndexInput on BufferedIndexInput with closed flag, overflow-safe slice, and proactive stream close. Fix DELETEBACKUP double-slash path. Cache BlobProperties in isDirectory. Document SecurityManager + Azure Identity limitation in the README. Add tests for clone independence, backward seek, slice, random access. Co-authored-by: Cursor <cursoragent@cursor.com>

prateeksinghalgit · 2026-05-12T07:56:29Z

Only reviewed the production code (skiped tests and other stuff).

Some general comments on the review:

can you replace all unnecessary qualified class names by imports? (replacing java.lang.String by String) This makes the code painful to read

there is a lot of shared logic between Azure backups and S3 backups. Lot of code is duplicated. I wonder if abstract classes should be introduced to factor out some logic

we always upload files with a block list. At least for small files, can we keep things simples with a simple pull? It think we may skip the commit call.

overall, I think it's pretty inefficient that AzureBlobIndexInput always reads files by small pages. Mostly for backup/restore, we will only fetch full files. You should not do a request by page, or use much bigger pages (without keeping them in memory).

Please note I did a code review, but I have no way to test it with Azure.

@psalagnac , thank you for taking the time to review.

made the change by replacing all unnecessary qualified class names by imports
s3-repository is in active production use; a refactor would force shared abstractions and re-testing on a stable module to land a brand-new one. That's significant scope creep with no end-user benefit. Also the duplication is mostly in boilerplate (resolveDirectory shape, sanitization helpers); both repositories have different SDK contracts underneath.
Let me know what you think.
Given the comments on AzureBlobIndexInput, I have rewritten AzureBlobIndexInput to extend Lucene's standard BufferedIndexInput instead of using IndexInput directly. Now only override readInternal(ByteBuffer) and seekInternal(long), and everything else (buffer management, slice, clone, EOF handling) comes from the parent class.

github-actions Bot added documentation Improvements or additions to documentation dependencies Dependency upgrades tool:build tests labels Oct 9, 2025

janhoy requested a review from Copilot November 20, 2025 07:29

Copilot started reviewing on behalf of janhoy November 20, 2025 07:30 View session

Copilot finished reviewing on behalf of janhoy November 20, 2025 07:30

janhoy reviewed Nov 20, 2025

View reviewed changes

Copilot AI reviewed Nov 20, 2025

View reviewed changes

Comment thread ...ules/azure-blob-repository/src/java/org/apache/solr/azureblob/AzureBlobBackupRepository.java Outdated

Comment thread solr/licenses/azure-NOTICE.txt Outdated

Comment thread solr/licenses/msal4j-NOTICE.txt Outdated

Comment thread solr/licenses/reactor-NOTICE.txt Outdated

janhoy reviewed Nov 20, 2025

View reviewed changes

Comment thread gradle/libs.versions.toml Outdated

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch 3 times, most recently from 32efbbd to 5462186 Compare November 21, 2025 23:10

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from 5462186 to 47f456a Compare November 22, 2025 02:35

janhoy reviewed Nov 22, 2025

View reviewed changes

Comment thread solr/licenses/msal4j-NOTICE.txt Outdated

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from 24daaaa to e6d9a64 Compare November 24, 2025 09:22

github-actions Bot added the jetty-server label Nov 24, 2025

janhoy reviewed Nov 24, 2025

View reviewed changes

Comment thread solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc Outdated

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from e6d9a64 to f7adb05 Compare November 24, 2025 23:43

janhoy reviewed Nov 25, 2025

View reviewed changes

Comment thread ...es/azure-blob-repository/src/test/org/apache/solr/azureblob/AbstractAzureBlobClientTest.java Outdated

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch 3 times, most recently from 8e812b1 to 3545833 Compare November 26, 2025 07:25

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from 3545833 to e18dd13 Compare November 26, 2025 07:59

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from e18dd13 to b28e265 Compare December 2, 2025 07:12

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from b28e265 to 428ca18 Compare December 3, 2025 19:37

prateeksinghalgit requested a review from janhoy December 16, 2025 23:52

Merge upstream/main and resolve conflicts in gradle/libs.versions.toml

d015bbe

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from 3cef18e to d015bbe Compare January 14, 2026 20:44

github-actions Bot added the stale PR not updated in 60 days label Mar 16, 2026

janhoy requested review from HoustonPutman and psalagnac and removed request for janhoy March 19, 2026 21:47

github-actions Bot removed the stale PR not updated in 60 days label Mar 20, 2026

prateeksinghalgit added 2 commits April 30, 2026 13:48

Merge upstream/main and resolve conflicts in gradle/libs.versions.toml

9d6242c

Made-with: Cursor

SOLR-17949: AGENTS.md compliance + fix SocketProxy import after upstr…

70ed112

…eam merge Made-with: Cursor

psalagnac reviewed May 7, 2026

View reviewed changes

prateeksinghalgit force-pushed the SOLR-17949-azure-blob-repository branch from 1923c1e to f944f72 Compare May 12, 2026 07:53


		String destPath = getBlobPath(dest);

		String blobPath = destPath.endsWith("/") ? destPath + destFileName : destPath;

Conversation

prateeksinghalgit commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Checklist

Uh oh!

pratsgit commented Nov 20, 2025

Uh oh!

janhoy commented Nov 20, 2025

Uh oh!

janhoy left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prateeksinghalgit commented Nov 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janhoy commented Nov 25, 2025

Uh oh!

pratsgit commented Dec 1, 2025

Uh oh!

prateeksinghalgit commented Dec 2, 2025

Uh oh!

github-actions Bot commented Mar 16, 2026

Uh oh!

prateeksinghalgit commented Apr 21, 2026

Uh oh!

psalagnac commented Apr 27, 2026

Uh oh!

psalagnac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prateeksinghalgit commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

prateeksinghalgit commented Oct 9, 2025 •

edited

Loading

prateeksinghalgit commented May 12, 2026 •

edited

Loading