persist: port Azure blob backend to the official Azure SDK by antiguru · Pull Request #39293 · MaterializeInc/materialize

antiguru · 2026-09-28T08:25:42Z

This replaces the abandoned azure_storage/azure_storage_blobs 0.21 crates with azure_storage_blob 1.1, azure_core 1.1, and azure_identity 1.0. It removes http-types and the advisory ignores for RUSTSEC-2026-0174 and RUSTSEC-2026-0275 (SEC-619), which were only needed for the old crates. This unblocks the Azure half of #39023.

The new azure_core implements its transport for reqwest 0.13 only. Persist uses it through a reqwest-0-13 workspace alias because the iceberg fork's public API still takes reqwest 0.12 types, which blocks a workspace-wide bump.

The following behavior carries over from the old SDK:

Custom refreshing workload identity credential. The token exchange now goes through ClientAssertionCredential.
Default credential chain. The new azure_identity removed DefaultAzureCredential, so persist rebuilds its order: environment (static federated token, client secret), managed identity with a 1s timeout, then Azure CLI.
Retry and timeout transport options.
Blob names trimmed of leading and trailing slashes, so existing data stays readable.
Single-request Put Blob uploads.
Concurrent 1 MiB ranged gets, now pinned to the first response's etag.

The new SDK has no shared key auth. The Azurite path signs an account SAS with the emulator's well-known key instead. This upgrades Azurite to 3.37.0 because the SDK sends API version 2026-04-06, which 3.33.0 rejects.

This adds azure_blob_multi_range_get, which covers gets that span several partitions, listing sizes, and delete.

🤖 Generated with Claude Code

Replace the unmaintained azure_storage and azure_storage_blobs 0.21 crates with azure_storage_blob 1.1, azure_core 1.1, and azure_identity 1.0. The new azure_core implements its HTTP transport for reqwest 0.13, so persist depends on reqwest 0.13 through a separate workspace alias while the rest of the tree stays on 0.12. The new azure_identity has no default credential chain, so persist builds the chain the old DefaultAzureCredential used: environment credentials, managed identity with a 1s timeout, and the Azure CLI. The new SDK also drops shared key authorization, so the Azurite path signs an account SAS with the emulator's well-known key. Azurite runs with --skipApiVersionCheck because the SDK sends a newer service version than the pinned Azurite release accepts. Blob names keep the old SDK's leading and trailing slash trimming, so existing data stays readable. Uploads stay single Put Blob requests. Co-Authored-By: Claude Opus 5.5 <[email protected]>

The Azure SDK sends service API version 2026-04-06, which Azurite 3.37.0 accepts, so the emulator no longer needs --skipApiVersionCheck. Co-Authored-By: Claude Opus 5.5 <[email protected]>

Co-Authored-By: Claude Opus 5.5 <[email protected]>

def- · 2026-09-28T13:14:22Z

QA LLM Review

1. MEDIUM -- Dropped Azure gets keep their ranged downloads running in detached SDK tasks

src/persist/src/azure.rs:760

For blobs larger than one 1 MiB partition, BlobClient::download runs the ranged requests in tokio::spawned tasks that are not aborted when the returned body stream is dropped. When HedgedBlob drops the losing leg of a race, those requests keep running until they finish or time out, outside both the hedge concurrency cap and the fetch memory semaphore, and a request hung on a dead connection is not cancelled at all.

Details

src/persist/src/hedge.rs:370 relies on "The losing future is dropped, which cancels the request in flight (there is no task boundary between here and the backend)", and src/persist-client/src/cache.rs:257 forbids a task boundary between HedgedBlob and the backend. The old get drove chunk bodies in a FuturesOrdered inside the caller's future, so it met that invariant. The new SDK breaks it inside the backend:

In azure_storage_blob 1.1 partitioned_transfer/download.rs, download spawns the first-partition reader immediately. As the body stream is polled, it spawns up to parallel more workers through get_async_runtime().spawn, where parallel is default_concurrency(), which clamps the core count to 8..96. Each worker allocates vec![0u8; range.len()] before it sends its request.
The tokio SpawnedTask (typespec_client_core async_runtime/tokio_runtime.rs) wraps a plain JoinHandle and has no abort on drop.

So every hedge race on a part over 1 MiB leaves the losing leg with up to parallel MiB of downloads whose results are discarded. The stalled request stays on its dead connection until persist_blob_read_timeout or the attempt timeout, and a stall before the response headers is then retried by the SDK retry policy for up to persist_blob_operation_timeout. Meanwhile the get has already returned, so the hedge slot (persist_blob_hedged_get_max_concurrent, documented as bounding raced in-flight bytes to about two parts) and the fetch semaphore permit are released while this work is still running. Hedging is on by default, so this path runs on Azure in production. Parts of 1 MiB or less are unaffected because their single response is read inline.

Possible fixes:

Register an AsyncRuntime with azure_core::async_runtime::set_async_runtime whose spawn returns an abort-on-drop handle, for example one wrapping mz_ore::task::spawn(..).abort_on_drop(). Dropping the stream would then abort its workers.
Or drive the ranges from persist's own future. Request range: Some(0..1 MiB), which spawns nothing because no ranges remain. Then fetch the remaining ranges with if_match set to the first etag, using a bounded FuturesOrdered as the old code did.

The Azure SDK's partitioned download reads every range after the first in spawned tasks that dropping the download does not abort. HedgedBlob cancels the losing leg of a race by dropping its get, so those requests kept running outside the hedge and fetch budgets. Each range is now its own single-partition download, which the SDK reads without spawning, and persist drives up to eight of them at once, pinned to the first range's etag. Co-Authored-By: Claude Opus 5.5 <[email protected]>

mtabebe

code looks good.

One minor question, and another more detailed one that is really work for the persist team to consider in testing infrastructure.

mtabebe · 2026-09-28T15:33:16Z

+const EMULATOR_ACCOUNT: &str = "devstoreaccount1";
+
+/// The publicly documented account key of the Azurite emulator's
+/// [EMULATOR_ACCOUNT].
+const EMULATOR_ACCOUNT_KEY: &str =
+    "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==";


Just wanted to say that this really confused me why these were in the code, but these are not secrets.

I do wonder if this will trip up any automation ...

Not a secret: it's Azurite's built-in, publicly documented key for devstoreaccount1. The repo's trufflehog lint (check-trufflehog.sh, which scans all tracked files) passes with it in place. Added a NOTE: to the doc comment saying so.

(Posted by Claude Code.)

Cool, I realized it was the publicly documented key. It just took me a minute to figure it out

mtabebe · 2026-09-28T15:41:46Z

+        let etag = first.properties.etag.clone();
+        let partition_size = u64::cast_from(GET_PARTITION_SIZE.get());
+        let offsets =
+            (partition_size..content_length.unwrap_or(0)).step_by(GET_PARTITION_SIZE.get());


What happens if we end up in the or(0) case from download_total_len? What are the offsets that we will read?

It feels like we will not download any ranges.

I am curious if we want to be explicit about the none case of how we handle download_total_len?

Good catch, fixed in f44c7f9. A missing length now fails the get instead of silently returning only the first partition. Looking at it also showed that a response ignoring the range would have been read twice, so the remaining ranges now start after the bytes actually received. Both cases have mock-transport tests that fail against the previous code.

(Posted by Claude Code.)

mtabebe · 2026-09-28T15:42:38Z

+    /// dropping a get must drop every request it has in flight.
+    #[mz_ore::test(tokio::test)]
+    #[cfg_attr(miri, ignore)] // too slow
+    async fn azure_blob_get_drop_cancels_requests() {


Thank you for the new tests

mtabebe · 2026-09-28T16:23:36Z

+    /// Returns a client for the blob at `path`.
+    fn blob_client(&self, path: &str) -> azure_storage_blob::BlobClient {
+        // Blobs are stored under their path without leading and trailing
+        // slashes, which `azure_storage_blobs` 0.21 stripped when building


Do you know if we have upgrade tests that would catch if we broke this?

My concern here is:

old version writes on the old SDK

new version reads on the new SDK

Then, there is a mismatch in prefix/label and now we can't read the data that was previously written.

FWIW: I had my agent try and write a reproduction and it passed so I am satisfied with the code here.

I don't think anything needs to change in this PR, so this is more we working through my thought process.

I think what we would want is an upgrade test with platform checks where we had different flags on the old/new version?

No CI job covers that today. The Azurite platform checks in release qualification only run the restart scenario, and the Nightly Azurite jobs don't upgrade. The platform-checks composition supports --features=azurite, so an upgrade scenario on Azurite would be the natural follow-up. Blob names keep the old SDK's slash trimming, so objects written by the old SDK map to the same URLs.

(Posted by Claude Code.)

I had some code on #36216 to cover the change / missing Azurite tests if and when anybody needs it

Thanks. #39306 added Nightly upgrade checks on Azurite (UpgradeEntireMz and UpgradeEntireMzFromLatestSelfManaged with --features=azurite). They start on the old SDK and upgrade to the new one, which covers the old-write/new-read case.

(Posted by Claude Code.)

A first range response without a length left the get unable to tell which ranges remain, and it returned only the first partition. Fail such gets instead. The remaining ranges now start after the bytes the first response delivered, so a response that ignores the requested range is not read twice. Also spell out that the emulator account key is not a secret. Co-Authored-By: Claude Opus 5.5 <[email protected]>

antiguru · 2026-09-28T18:36:55Z

Thanks for the review!

Azurite platform checks only run the restart scenario in release qualification, so no CI job catches an upgrade that can no longer read blobs an earlier release wrote to Azure blob storage. This adds two Nightly steps that run the whole-Mz upgrade scenarios with `--features=azurite`: `UpgradeEntireMz` from the last release, and `UpgradeEntireMzFromLatestSelfManaged`, which is the upgrade path Azure deployments take. `StartMz` already passes the Azurite blob store to every version it starts, so the scenarios need no changes. Follow-up to the review of #39293, which moves persist's Azure backend to the new Azure SDK. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <[email protected]>

jasonhernandez · 2026-09-28T23:32:21Z

Thank you @antiguru!

antiguru requested a review from mtabebe September 28, 2026 08:32

antiguru force-pushed the azure-sdk-port branch from 7362d5d to 2995050 Compare September 28, 2026 08:37

antiguru marked this pull request as ready for review September 28, 2026 08:40

antiguru requested a review from a team as a code owner September 28, 2026 08:40

antiguru force-pushed the azure-sdk-port branch from 2995050 to 0c720be Compare September 28, 2026 12:54

antiguru and others added 3 commits September 28, 2026 14:54

test: upgrade Azurite to 3.37.0

d0fb998

The Azure SDK sends service API version 2026-04-06, which Azurite 3.37.0 accepts, so the emulator no longer needs --skipApiVersionCheck. Co-Authored-By: Claude Opus 5.5 <[email protected]>

test: drop stale Azurite 3.35.0 data loss note

3f484b1

Co-Authored-By: Claude Opus 5.5 <[email protected]>

antiguru force-pushed the azure-sdk-port branch from 0c720be to 3f484b1 Compare September 28, 2026 13:04

mtabebe approved these changes Sep 28, 2026

View reviewed changes

antiguru enabled auto-merge (squash) September 28, 2026 18:36

antiguru mentioned this pull request Sep 28, 2026

ci: run upgrade platform checks on the Azure blob store #39306

Merged

antiguru merged commit f3e5694 into MaterializeInc:main Sep 28, 2026
90 checks passed

antiguru deleted the azure-sdk-port branch September 28, 2026 19:02

jasonhernandez mentioned this pull request Sep 28, 2026

persist: migrate to the GA Azure SDK crates #36216

Closed

This was referenced Sep 29, 2026

build(deps): bump azure_core from 0.21.0 to 0.32.0 #36019

Closed

build(deps): bump azure_identity from 0.21.0 to 0.32.0 #36094

Closed

Conversation

antiguru commented Sep 28, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

def- commented Sep 28, 2026

QA LLM Review

1. MEDIUM -- Dropped Azure gets keep their ranged downloads running in detached SDK tasks

Uh oh!

mtabebe 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!

antiguru commented Sep 28, 2026

Uh oh!

Uh oh!

jasonhernandez commented Sep 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

antiguru commented Sep 28, 2026 •

edited by github-actions Bot

Loading