From 91f8ad65783ce1755bca124ab9a77ea35dba259d Mon Sep 17 00:00:00 2001 From: Isabelle Date: Tue, 19 May 2026 19:17:06 -0700 Subject: [PATCH 01/10] stageblocksmall scenario adjustment --- .../scripts/fault-injector.sh | 4 ++-- .../scripts/stress-run.sh | 12 +++++++++++- .../templates/stress-test-job.yaml | 16 +++++++++++++--- .../azure/storage/stress/CrcInputStream.java | 18 ++++++++++++++++++ .../azure/storage/stress/TelemetryHelper.java | 2 +- 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/sdk/storage/azure-storage-blob-stress/scripts/fault-injector.sh b/sdk/storage/azure-storage-blob-stress/scripts/fault-injector.sh index ed834fc131d6..65c857d3d7ed 100644 --- a/sdk/storage/azure-storage-blob-stress/scripts/fault-injector.sh +++ b/sdk/storage/azure-storage-blob-stress/scripts/fault-injector.sh @@ -1,4 +1,4 @@ #!/bin/sh set -ex; -dotnet dev-certs https --export-path /mnt/outputs/dev-cert.pfx; -/root/.dotnet/tools/http-fault-injector; +dotnet dev-certs https --export-path /mnt/outputs/dev-cert.crt --format PEM --no-password; +/root/.dotnet/tools/http-fault-injector; \ No newline at end of file diff --git a/sdk/storage/azure-storage-blob-stress/scripts/stress-run.sh b/sdk/storage/azure-storage-blob-stress/scripts/stress-run.sh index 20a7669c46e9..cebbfb4d47f4 100644 --- a/sdk/storage/azure-storage-blob-stress/scripts/stress-run.sh +++ b/sdk/storage/azure-storage-blob-stress/scripts/stress-run.sh @@ -1,4 +1,14 @@ #!/bin/sh set -ex; set -exa; -keytool -import -alias test -file /mnt/outputs/dev-cert.pfx -keystore ${JAVA_HOME}/lib/security/cacerts -noprompt -keypass changeit -storepass changeit; +attempts=0; +while [ ! -s /mnt/outputs/dev-cert.crt ]; do + attempts=$((attempts + 1)); + if [ "$attempts" -gt 60 ]; then + echo "Timed out waiting for fault injector certificate" >&2; + exit 1; + fi; + sleep 1; +done; +keytool -delete -alias HttpFaultInject -keystore "${JAVA_HOME}/lib/security/cacerts" -storepass changeit >/dev/null 2>&1 || true; +keytool -importcert -trustcacerts -alias HttpFaultInject -file /mnt/outputs/dev-cert.crt -keystore "${JAVA_HOME}/lib/security/cacerts" -noprompt -storepass changeit; \ No newline at end of file diff --git a/sdk/storage/azure-storage-blob-stress/templates/stress-test-job.yaml b/sdk/storage/azure-storage-blob-stress/templates/stress-test-job.yaml index 2a22302d24ed..8cc1c2d71b5b 100644 --- a/sdk/storage/azure-storage-blob-stress/templates/stress-test-job.yaml +++ b/sdk/storage/azure-storage-blob-stress/templates/stress-test-job.yaml @@ -16,7 +16,7 @@ spec: args: - | set -ex; - dotnet dev-certs https --export-path /mnt/outputs/dev-cert.pfx; + dotnet dev-certs https --export-path /mnt/outputs/dev-cert.crt --format PEM --no-password; /root/.dotnet/tools/http-fault-injector; resources: limits: @@ -30,7 +30,17 @@ spec: - | set -xa; set -o pipefail; - keytool -import -alias test -file /mnt/outputs/dev-cert.pfx -keystore ${JAVA_HOME}/lib/security/cacerts -noprompt -keypass changeit -storepass changeit; + attempts=0; + while [ ! -s /mnt/outputs/dev-cert.crt ]; do + attempts=$((attempts + 1)); + if [ "$attempts" -gt 60 ]; then + echo "Timed out waiting for fault injector certificate" >&2; + exit 1; + fi; + sleep 1; + done; + keytool -delete -alias HttpFaultInject -keystore "${JAVA_HOME}/lib/security/cacerts" -storepass changeit >/dev/null 2>&1 || true; + keytool -importcert -trustcacerts -alias HttpFaultInject -file /mnt/outputs/dev-cert.crt -keystore "${JAVA_HOME}/lib/security/cacerts" -noprompt -storepass changeit || exit 1; mkdir -p "$DEBUG_SHARE"; . /mnt/outputs/.env; export AZURE_HTTP_CLIENT_IMPLEMENTATION=com.azure.core.http.netty.NettyAsyncHttpClientProvider; @@ -67,4 +77,4 @@ spec: cpu: "0.7" {{- include "stress-test-addons.container-env" . | nindent 6 }} -{{- end -}} +{{- end -}} \ No newline at end of file diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java index 7e48798b30b0..ed1cfc59f60c 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java @@ -53,6 +53,13 @@ public synchronized int read() throws IOException { head.put((byte) b); } length++; + // Emit as soon as the expected size has been delivered so that consumers (such as the + // Storage SDK upload path) that stop reading once they have the exact number of bytes + // they asked for do not leave the Sinks.One waiting on a never-arriving EOF read. + // Repeat emissions are no-ops because tryEmitValue returns FAIL_TERMINATED. + if (size > 0 && length >= size) { + emitContentInfo(); + } return b; } @@ -69,6 +76,11 @@ public synchronized int read(byte buf[], int off, int len) throws IOException { head.put(buf, off, Math.min(read, head.remaining())); } length += read; + // See note in read(): emit once the consumer has been handed all the bytes it requested + // so the sink is guaranteed to complete even if the consumer never reads past EOF. + if (size > 0 && length >= size) { + emitContentInfo(); + } return read; } @@ -134,6 +146,12 @@ public void close() { inputStream.close(); } catch (IOException e) { throw LOGGER.logExceptionAsError(new UncheckedIOException(e)); + } finally { + // Defensive: terminate the sink so any consumer still waiting on getContentInfo() + // does not hang in cases where the stream was closed before being fully read + // (for example after an upload failure that aborted the request body subscription). + // This is a no-op when the sink has already emitted. + emitContentInfo(); } } diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/TelemetryHelper.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/TelemetryHelper.java index 4bf6e523eae1..b633479100b1 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/TelemetryHelper.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/TelemetryHelper.java @@ -124,7 +124,7 @@ public String getDescription() { Cpu.registerObservers(otel); MemoryPools.registerObservers(otel); Threads.registerObservers(otel); - GarbageCollector.registerObservers(otel); + GarbageCollector.registerObservers(otel, true); OpenTelemetryAppender.install(otel); return otel; } From 13936bde4dad5f3c3646edbfc5e5da04379407df Mon Sep 17 00:00:00 2001 From: Isabelle Date: Tue, 19 May 2026 23:35:26 -0700 Subject: [PATCH 02/10] wip --- .../azure/storage/stress/CrcInputStream.java | 75 +++++++------------ 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java index ed1cfc59f60c..b0f913667ab4 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java @@ -10,7 +10,6 @@ import com.azure.perf.test.core.RepeatingInputStream; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import reactor.core.publisher.Sinks; import java.io.IOException; import java.io.InputStream; @@ -20,7 +19,6 @@ public class CrcInputStream extends InputStream { private final static ClientLogger LOGGER = new ClientLogger(CrcInputStream.class); - private final Sinks.One sink = Sinks.one(); private final InputStream inputStream; private final CRC32 crc = new CRC32(); private final ByteBuffer head = ByteBuffer.allocate(1024); @@ -44,7 +42,6 @@ public CrcInputStream(InputStream source) { public synchronized int read() throws IOException { int b = inputStream.read(); if (b < 0) { - emitContentInfo(); return b; } @@ -53,13 +50,6 @@ public synchronized int read() throws IOException { head.put((byte) b); } length++; - // Emit as soon as the expected size has been delivered so that consumers (such as the - // Storage SDK upload path) that stop reading once they have the exact number of bytes - // they asked for do not leave the Sinks.One waiting on a never-arriving EOF read. - // Repeat emissions are no-ops because tryEmitValue returns FAIL_TERMINATED. - if (size > 0 && length >= size) { - emitContentInfo(); - } return b; } @@ -67,7 +57,6 @@ public synchronized int read() throws IOException { public synchronized int read(byte buf[], int off, int len) throws IOException { int read = inputStream.read(buf, off, len); if (read < 0) { - emitContentInfo(); return read; } @@ -76,41 +65,9 @@ public synchronized int read(byte buf[], int off, int len) throws IOException { head.put(buf, off, Math.min(read, head.remaining())); } length += read; - // See note in read(): emit once the consumer has been handed all the bytes it requested - // so the sink is guaranteed to complete even if the consumer never reads past EOF. - if (size > 0 && length >= size) { - emitContentInfo(); - } return read; } - // Uses tryEmitValue instead of emitValue(FAIL_FAST) so that resubscriptions - // (SDK retries, verification passes) don't throw on the second EOF. - private void emitContentInfo() { - String baseErrorMessage = "Failed to emit content because "; - Sinks.EmitResult emitResult = sink.tryEmitValue(new ContentInfo(crc.getValue(), length, head)); - switch (emitResult) { - case OK: - case FAIL_TERMINATED: - // No action needed for successful or already-terminated emissions. - break; - case FAIL_CANCELLED: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + - " the sink was previously interrupted by its consumer: " + emitResult)); - case FAIL_OVERFLOW: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the buffer is full: " + emitResult)); - case FAIL_NON_SERIALIZED: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "two threads called emit at " + - "once: " + emitResult)); - case FAIL_ZERO_SUBSCRIBER: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the sink requires a " + - "subscriber:" + emitResult)); - default: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "unexpected emit result: " - + emitResult)); - } - } - @Override public synchronized void mark(int readLimit) { if (markSupported) { @@ -136,8 +93,32 @@ public boolean markSupported() { return markSupported; } + /** + * Returns a {@link Mono} that, on subscription, captures a snapshot of the stream's + * current CRC, byte count and head buffer. + * + *

The returned Mono is intentionally lazy: it does not wait for EOF or + * for any sink to be signaled. Callers are therefore responsible for subscribing only + * after the stream has been fully consumed (for example, after the SDK upload + * call has returned for synchronous flows, or via {@code .then(data.getContentInfo())} + * for reactive flows). Subscribing before the stream is done will produce a snapshot of + * whatever has been read so far.

+ * + *

This contract avoids the previous design's dependence on the SDK reading past EOF + * (which never happened on known-length uploads and could leave the legacy sink waiting + * indefinitely) and naturally tolerates SDK retries: the snapshot reflects the bytes + * that were actually delivered on the final, successful pass.

+ * + * @return a cold Mono that emits a {@link ContentInfo} snapshot on each subscription. + */ public Mono getContentInfo() { - return sink.asMono(); + return Mono.fromCallable(() -> { + synchronized (this) { + // duplicate() shares the underlying byte[] but gives the caller an independent + // position/limit so subsequent reads on this stream don't perturb the snapshot. + return new ContentInfo(crc.getValue(), length, head.duplicate()); + } + }); } @Override @@ -146,12 +127,6 @@ public void close() { inputStream.close(); } catch (IOException e) { throw LOGGER.logExceptionAsError(new UncheckedIOException(e)); - } finally { - // Defensive: terminate the sink so any consumer still waiting on getContentInfo() - // does not hang in cases where the stream was closed before being fully read - // (for example after an upload failure that aborted the request body subscription). - // This is a no-op when the sink has already emitted. - emitContentInfo(); } } From 8c95c4160173e482a243ff4a0237f2260959f2d2 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Wed, 20 May 2026 09:30:52 -0700 Subject: [PATCH 03/10] avoid depending on raw short baseName being available globally --- .../stress-test-resources.bicep | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/storage/azure-storage-blob-stress/stress-test-resources.bicep b/sdk/storage/azure-storage-blob-stress/stress-test-resources.bicep index 671aec65300b..25ebd947a8b7 100644 --- a/sdk/storage/azure-storage-blob-stress/stress-test-resources.bicep +++ b/sdk/storage/azure-storage-blob-stress/stress-test-resources.bicep @@ -1,10 +1,10 @@ param baseName string -param endpointSuffix string = 'core.windows.net' +param endpointSuffix string = environment().suffixes.storage param location string = resourceGroup().location -param storageApiVersion string = '2022-09-01' -var primaryAccountName = '${baseName}' -var pageBlobStorageAccountName = '${baseName}pageblob' +var uniqueSuffix = uniqueString(resourceGroup().id) +var primaryAccountName = '${take(baseName, 11)}${uniqueSuffix}' +var pageBlobStorageAccountName = '${take(baseName, 7)}${uniqueSuffix}page' resource primaryAccount 'Microsoft.Storage/storageAccounts@2022-09-01' = { name: primaryAccountName @@ -26,5 +26,5 @@ resource pageBlobStorageAccount 'Microsoft.Storage/storageAccounts@2022-09-01' = properties: {} } -output STORAGE_ENDPOINT_STRING string = '"https://${primaryAccountName}.blob.core.windows.net"' -output PAGE_BLOB_STORAGE_ENDPOINT_STRING string = '"https://${pageBlobStorageAccountName}.blob.core.windows.net"' +output STORAGE_ENDPOINT_STRING string = '"https://${primaryAccountName}.blob.${endpointSuffix}"' +output PAGE_BLOB_STORAGE_ENDPOINT_STRING string = '"https://${pageBlobStorageAccountName}.blob.${endpointSuffix}"' From 5f0a5ea2646a2dbce0273c82965c8620be0420a7 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Wed, 20 May 2026 10:38:55 -0700 Subject: [PATCH 04/10] dep pinning --- sdk/storage/azure-storage-blob-stress/pom.xml | 12 ++++++++++++ sdk/storage/azure-storage-stress/pom.xml | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/sdk/storage/azure-storage-blob-stress/pom.xml b/sdk/storage/azure-storage-blob-stress/pom.xml index a64ef5b1f1d3..92888ec20d0d 100644 --- a/sdk/storage/azure-storage-blob-stress/pom.xml +++ b/sdk/storage/azure-storage-blob-stress/pom.xml @@ -21,6 +21,18 @@ - + + + + io.opentelemetry + opentelemetry-bom + 1.58.0 + pom + import + + + + ch.qos.logback diff --git a/sdk/storage/azure-storage-stress/pom.xml b/sdk/storage/azure-storage-stress/pom.xml index cf1888ebf045..8170d61ed19f 100644 --- a/sdk/storage/azure-storage-stress/pom.xml +++ b/sdk/storage/azure-storage-stress/pom.xml @@ -21,6 +21,18 @@ - + + + + io.opentelemetry + opentelemetry-bom + 1.58.0 + pom + import + + + + ch.qos.logback From 4e956e0ab8abe2a565fb9edabefee494e4778ab3 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Tue, 26 May 2026 09:21:28 -0700 Subject: [PATCH 05/10] reversion --- .../azure/storage/stress/CrcInputStream.java | 57 +++++++++++-------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java index b0f913667ab4..7e48798b30b0 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java @@ -10,6 +10,7 @@ import com.azure.perf.test.core.RepeatingInputStream; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.publisher.Sinks; import java.io.IOException; import java.io.InputStream; @@ -19,6 +20,7 @@ public class CrcInputStream extends InputStream { private final static ClientLogger LOGGER = new ClientLogger(CrcInputStream.class); + private final Sinks.One sink = Sinks.one(); private final InputStream inputStream; private final CRC32 crc = new CRC32(); private final ByteBuffer head = ByteBuffer.allocate(1024); @@ -42,6 +44,7 @@ public CrcInputStream(InputStream source) { public synchronized int read() throws IOException { int b = inputStream.read(); if (b < 0) { + emitContentInfo(); return b; } @@ -57,6 +60,7 @@ public synchronized int read() throws IOException { public synchronized int read(byte buf[], int off, int len) throws IOException { int read = inputStream.read(buf, off, len); if (read < 0) { + emitContentInfo(); return read; } @@ -68,6 +72,33 @@ public synchronized int read(byte buf[], int off, int len) throws IOException { return read; } + // Uses tryEmitValue instead of emitValue(FAIL_FAST) so that resubscriptions + // (SDK retries, verification passes) don't throw on the second EOF. + private void emitContentInfo() { + String baseErrorMessage = "Failed to emit content because "; + Sinks.EmitResult emitResult = sink.tryEmitValue(new ContentInfo(crc.getValue(), length, head)); + switch (emitResult) { + case OK: + case FAIL_TERMINATED: + // No action needed for successful or already-terminated emissions. + break; + case FAIL_CANCELLED: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + + " the sink was previously interrupted by its consumer: " + emitResult)); + case FAIL_OVERFLOW: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the buffer is full: " + emitResult)); + case FAIL_NON_SERIALIZED: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "two threads called emit at " + + "once: " + emitResult)); + case FAIL_ZERO_SUBSCRIBER: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the sink requires a " + + "subscriber:" + emitResult)); + default: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "unexpected emit result: " + + emitResult)); + } + } + @Override public synchronized void mark(int readLimit) { if (markSupported) { @@ -93,32 +124,8 @@ public boolean markSupported() { return markSupported; } - /** - * Returns a {@link Mono} that, on subscription, captures a snapshot of the stream's - * current CRC, byte count and head buffer. - * - *

The returned Mono is intentionally lazy: it does not wait for EOF or - * for any sink to be signaled. Callers are therefore responsible for subscribing only - * after the stream has been fully consumed (for example, after the SDK upload - * call has returned for synchronous flows, or via {@code .then(data.getContentInfo())} - * for reactive flows). Subscribing before the stream is done will produce a snapshot of - * whatever has been read so far.

- * - *

This contract avoids the previous design's dependence on the SDK reading past EOF - * (which never happened on known-length uploads and could leave the legacy sink waiting - * indefinitely) and naturally tolerates SDK retries: the snapshot reflects the bytes - * that were actually delivered on the final, successful pass.

- * - * @return a cold Mono that emits a {@link ContentInfo} snapshot on each subscription. - */ public Mono getContentInfo() { - return Mono.fromCallable(() -> { - synchronized (this) { - // duplicate() shares the underlying byte[] but gives the caller an independent - // position/limit so subsequent reads on this stream don't perturb the snapshot. - return new ContentInfo(crc.getValue(), length, head.duplicate()); - } - }); + return sink.asMono(); } @Override From 93ca92df20986b74b583acfa40d2c983de8b4b6c Mon Sep 17 00:00:00 2001 From: Isabelle Date: Tue, 26 May 2026 09:24:28 -0700 Subject: [PATCH 06/10] adding back --- .../azure/storage/stress/CrcInputStream.java | 57 ++++++++----------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java index 7e48798b30b0..b0f913667ab4 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java @@ -10,7 +10,6 @@ import com.azure.perf.test.core.RepeatingInputStream; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import reactor.core.publisher.Sinks; import java.io.IOException; import java.io.InputStream; @@ -20,7 +19,6 @@ public class CrcInputStream extends InputStream { private final static ClientLogger LOGGER = new ClientLogger(CrcInputStream.class); - private final Sinks.One sink = Sinks.one(); private final InputStream inputStream; private final CRC32 crc = new CRC32(); private final ByteBuffer head = ByteBuffer.allocate(1024); @@ -44,7 +42,6 @@ public CrcInputStream(InputStream source) { public synchronized int read() throws IOException { int b = inputStream.read(); if (b < 0) { - emitContentInfo(); return b; } @@ -60,7 +57,6 @@ public synchronized int read() throws IOException { public synchronized int read(byte buf[], int off, int len) throws IOException { int read = inputStream.read(buf, off, len); if (read < 0) { - emitContentInfo(); return read; } @@ -72,33 +68,6 @@ public synchronized int read(byte buf[], int off, int len) throws IOException { return read; } - // Uses tryEmitValue instead of emitValue(FAIL_FAST) so that resubscriptions - // (SDK retries, verification passes) don't throw on the second EOF. - private void emitContentInfo() { - String baseErrorMessage = "Failed to emit content because "; - Sinks.EmitResult emitResult = sink.tryEmitValue(new ContentInfo(crc.getValue(), length, head)); - switch (emitResult) { - case OK: - case FAIL_TERMINATED: - // No action needed for successful or already-terminated emissions. - break; - case FAIL_CANCELLED: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + - " the sink was previously interrupted by its consumer: " + emitResult)); - case FAIL_OVERFLOW: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the buffer is full: " + emitResult)); - case FAIL_NON_SERIALIZED: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "two threads called emit at " + - "once: " + emitResult)); - case FAIL_ZERO_SUBSCRIBER: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the sink requires a " + - "subscriber:" + emitResult)); - default: - throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "unexpected emit result: " - + emitResult)); - } - } - @Override public synchronized void mark(int readLimit) { if (markSupported) { @@ -124,8 +93,32 @@ public boolean markSupported() { return markSupported; } + /** + * Returns a {@link Mono} that, on subscription, captures a snapshot of the stream's + * current CRC, byte count and head buffer. + * + *

The returned Mono is intentionally lazy: it does not wait for EOF or + * for any sink to be signaled. Callers are therefore responsible for subscribing only + * after the stream has been fully consumed (for example, after the SDK upload + * call has returned for synchronous flows, or via {@code .then(data.getContentInfo())} + * for reactive flows). Subscribing before the stream is done will produce a snapshot of + * whatever has been read so far.

+ * + *

This contract avoids the previous design's dependence on the SDK reading past EOF + * (which never happened on known-length uploads and could leave the legacy sink waiting + * indefinitely) and naturally tolerates SDK retries: the snapshot reflects the bytes + * that were actually delivered on the final, successful pass.

+ * + * @return a cold Mono that emits a {@link ContentInfo} snapshot on each subscription. + */ public Mono getContentInfo() { - return sink.asMono(); + return Mono.fromCallable(() -> { + synchronized (this) { + // duplicate() shares the underlying byte[] but gives the caller an independent + // position/limit so subsequent reads on this stream don't perturb the snapshot. + return new ContentInfo(crc.getValue(), length, head.duplicate()); + } + }); } @Override From 11d77c02b74fae4bcce0d2a3c9a60a40b13ff6d0 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Tue, 26 May 2026 17:59:37 -0700 Subject: [PATCH 07/10] wip --- .../perf/test/core/PerfStressProgram.java | 13 +++ .../storage/blob/stress/BlobScenarioBase.java | 81 +++++++++++++++++-- 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java b/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java index 66eae27f7f3e..5602b9b94f58 100644 --- a/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java +++ b/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java @@ -348,8 +348,21 @@ public static void runTests(PerfTestBase[] tests, boolean sync, boolean compl .block(); } } catch (Exception e) { + // Previously this catch swallowed the exception, printed a stack trace, and let the + // method fall through to "=== Results ===" / globalCleanupAsync, which caused the + // process to exit 0. That made real test failures (e.g. an SDK NullPointerException + // surfaced by a retry path during a stress run) invisible to CI and dashboards: the + // Kubernetes Job reported Succeeded and the workbook showed a healthy success ratio + // even though the test loop had aborted partway through its configured duration. + // + // Rethrow as an unchecked exception so the surrounding lifecycle still runs the + // `finally` block below (progressStatus.cancel()) and the outer `try { ... } finally` + // in run(...) still drives globalCleanupAsync, but the JVM ultimately exits non-zero + // and the failure is visible. See sdk/storage/BUG-payload-size-gate-npe.md for the + // specific case that motivated this change. System.err.println("Error occurred running tests: " + System.lineSeparator() + e); e.printStackTrace(System.err); + throw (e instanceof RuntimeException) ? (RuntimeException) e : new RuntimeException(e); } finally { progressStatus.cancel(); } diff --git a/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java b/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java index 6bf3dc004108..9ce4a8cbdb94 100644 --- a/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java +++ b/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java @@ -15,12 +15,10 @@ import com.azure.storage.blob.BlobServiceAsyncClient; import com.azure.storage.blob.BlobServiceClient; import com.azure.storage.blob.BlobServiceClientBuilder; -import com.azure.storage.stress.ContentMismatchException; import com.azure.storage.stress.TelemetryHelper; import com.azure.storage.stress.FaultInjectionProbabilities; import com.azure.storage.stress.FaultInjectingHttpPolicy; import com.azure.storage.stress.StorageStressOptions; -import reactor.core.Exceptions; import reactor.core.publisher.Mono; import java.time.Instant; @@ -156,20 +154,89 @@ public void run() { @SuppressWarnings("try") @Override public Mono runAsync() { + // We previously wrapped runInternalAsync with an unconditional `.retryWhen(Retry.max(3))`. + // That mask hid a real liveness bug in the SDK upload pipeline: when an HTTP fault + // (especially the request-side `pq*` indefinite variants from FaultInjectingHttpPolicy) + // causes one parallel rail's Mono to stop making progress, the unconditional retry burns + // 3 attempts of 60s Netty response timeouts on top of an already-stuck pipeline and the + // hang propagates outward. With three parallel rails, a single such stall freezes the + // whole `flatMap(runTestAsync, 1)` for the remainder of the configured --duration, which + // is exactly what the six "Failed" large-payload pods exhibited on 2026-05-26 (see + // sdk/storage/BUG-blob-upload-hang-on-fault.md). + // + // Replace the retry with an outer per-operation timeout. Any single iteration that + // doesn't complete within `OPERATION_TIMEOUT` fails fast as a TimeoutException, which: + // 1. lets the rail recycle and start the next iteration, + // 2. surfaces a real failure on the dashboard instead of a frozen progress counter, + // 3. produces evidence (the TimeoutException) for SDK bug triage, and + // 4. still tolerates the natural per-op latency under fault injection (~6.3% of HTTP + // calls are configured-indefinite; effective avg op latency is a few seconds for + // small payloads and tens of seconds for large multi-request operations, well under + // the 2-minute cap below). + // If a scenario legitimately needs longer than the default per op, override + // `getOperationTimeout()` in the scenario class rather than blanket-raising it here. + // + // Tuning note (2026-05-26): the previous default was 2 minutes, which was too coarse. + // Under fault injection the dominant wedge is the `pq*` (request-side, indefinite) + // variant. Netty's `responseTimeout` already fires after 60s, so any operation still + // unresponsive ~30s after that is a real liveness bug and we want to fail fast and + // recycle the rail. Empirically this 4-6x's small-scenario throughput (small-blob ops + // typically complete in <2s; even fault-injected ops are bounded by the 60s Netty + // response timeout) while still leaving plenty of headroom for multi-block large + // operations via per-scenario overrides. + // + // CRITICAL: the outer `.onErrorResume(e -> Mono.empty())` converts a failed iteration + // into a successful "this iteration is done" signal *after* `instrumentRunAsync` has + // already fired its `doOnError` side-effect (which calls `trackFailure` and increments + // the `failed_runs` metric). Without this, a TimeoutException -- or any other error -- + // propagates out of `runTestAsync()` into `flatMap(runTestAsync, 1)` in + // ApiPerfTestBase.runAllAsync and terminates the entire Flux, cancelling all parallel + // rails and aborting the test loop. The previous `.retryWhen(Retry.max(3))` happened to + // mask this because most ops succeeded within retries, so the propagation path was + // rarely exercised; with the retry gone, every long-tail op would otherwise kill the + // whole job after the first 2-minute timeout. The error has already been logged and + // counted as a failure by the time `onErrorResume` runs, so no information is lost. return telemetryHelper.instrumentRunAsync(ctx -> runInternalAsync(ctx) - .retryWhen(reactor.util.retry.Retry.max(3) - .filter(e -> !(Exceptions.unwrap(e) - instanceof ContentMismatchException))) + .timeout(getOperationTimeout()) .doOnError(e -> { // Log the error for debugging but let legitimate failures propagate LOGGER.atError() .addKeyValue("error", e.getMessage()) .addKeyValue("errorType", e.getClass().getSimpleName()) - .log("Test operation failed after retries"); - })); + .log("Test operation failed"); + })) + .onErrorResume(e -> Mono.empty()); } + /** + * Default per-operation timeout for stress scenarios. Small-payload scenarios complete + * well under 2s in steady state and are bounded by Netty's 60s `responseTimeout` under + * fault injection, so 30s catches genuine liveness wedges without throwing away healthy + * long-tail ops. Multi-block large-payload variants need more time (a single op can be + * many sequential block uploads), so we scale the default up based on `options.getSize()`: + * one extra minute per 16 MiB above 1 MiB, capped at 5 minutes. Scenarios that need an + * even larger envelope can override this method: + * + * 
{@code
+     * @Override
+     * protected Duration getOperationTimeout() { return Duration.ofMinutes(10); }
+     * }
+ */ + protected Duration getOperationTimeout() { + long sizeBytes = options.getSize(); + // Baseline 30s, +60s per 16 MiB beyond the first 1 MiB. Capped at 5 minutes so a + // genuinely-wedged rail still recovers promptly even for the largest configured payloads. + long extraSeconds = Math.max(0, (sizeBytes - SMALL_PAYLOAD_THRESHOLD_BYTES) / BYTES_PER_EXTRA_MINUTE) * 60; + long totalSeconds = Math.min(BASE_TIMEOUT_SECONDS + extraSeconds, MAX_TIMEOUT_SECONDS); + return Duration.ofSeconds(totalSeconds); + } + + private static final long SMALL_PAYLOAD_THRESHOLD_BYTES = (long) 1024 * 1024; // 1 MiB + private static final long BYTES_PER_EXTRA_MINUTE = 16L * 1024 * 1024; // 16 MiB + private static final long BASE_TIMEOUT_SECONDS = 30; + private static final long MAX_TIMEOUT_SECONDS = 5 * 60; + protected abstract void runInternal(Context context) throws Exception; protected abstract Mono runInternalAsync(Context context); From a51a5c0e8cdbf5694c83e1ffb78f1eed307f0d1d Mon Sep 17 00:00:00 2001 From: Isabelle Date: Thu, 28 May 2026 22:29:39 -0700 Subject: [PATCH 08/10] storageseekablebytechannel change --- .../StorageSeekableByteChannelBlobReadBehavior.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java index 840dd6dda6be..47ef361690ff 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java @@ -11,6 +11,7 @@ import com.azure.storage.blob.models.BlobRange; import com.azure.storage.blob.models.BlobRequestConditions; import com.azure.storage.blob.models.BlobStorageException; +import com.azure.storage.blob.models.DownloadRetryOptions; import com.azure.storage.common.implementation.StorageSeekableByteChannel; import java.io.IOException; @@ -75,7 +76,7 @@ public int read(ByteBuffer dst, long sourceOffset) throws IOException { try (ByteBufferBackedOutputStreamUtil dstStream = new ByteBufferBackedOutputStreamUtil(dst)) { BlobDownloadResponse response = client.downloadStreamWithResponse(dstStream, new BlobRange(sourceOffset, (long) dst.remaining()), - null /*downloadRetryOptions*/, requestConditions, false, null, null); + new DownloadRetryOptions(), requestConditions, false, null, null); resourceLength = CoreUtils.extractSizeFromContentRange(response.getDeserializedHeaders().getContentRange()); return dst.position() - initialPosition; } catch (BlobStorageException e) { @@ -89,6 +90,11 @@ public int read(ByteBuffer dst, long sourceOffset) throws IOException { return sourceOffset < resourceLength ? 0 : -1; } throw LOGGER.logExceptionAsError(e); + } catch (RuntimeException e) { + if (resourceLength > 0 && sourceOffset >= resourceLength && e.getCause() instanceof IOException) { + return -1; + } + throw e; } } From 98f3e840f3c5311b01ac540eb7ed387dfed59c76 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Sun, 31 May 2026 10:29:35 -0700 Subject: [PATCH 09/10] bare minimum changes --- .../storage/blob/stress/BlobScenarioBase.java | 81 ++----------------- ...geSeekableByteChannelBlobReadBehavior.java | 5 -- .../azure/storage/stress/CrcInputStream.java | 57 +++++++------ 3 files changed, 39 insertions(+), 104 deletions(-) diff --git a/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java b/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java index 9ce4a8cbdb94..6bf3dc004108 100644 --- a/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java +++ b/sdk/storage/azure-storage-blob-stress/src/main/java/com/azure/storage/blob/stress/BlobScenarioBase.java @@ -15,10 +15,12 @@ import com.azure.storage.blob.BlobServiceAsyncClient; import com.azure.storage.blob.BlobServiceClient; import com.azure.storage.blob.BlobServiceClientBuilder; +import com.azure.storage.stress.ContentMismatchException; import com.azure.storage.stress.TelemetryHelper; import com.azure.storage.stress.FaultInjectionProbabilities; import com.azure.storage.stress.FaultInjectingHttpPolicy; import com.azure.storage.stress.StorageStressOptions; +import reactor.core.Exceptions; import reactor.core.publisher.Mono; import java.time.Instant; @@ -154,89 +156,20 @@ public void run() { @SuppressWarnings("try") @Override public Mono runAsync() { - // We previously wrapped runInternalAsync with an unconditional `.retryWhen(Retry.max(3))`. - // That mask hid a real liveness bug in the SDK upload pipeline: when an HTTP fault - // (especially the request-side `pq*` indefinite variants from FaultInjectingHttpPolicy) - // causes one parallel rail's Mono to stop making progress, the unconditional retry burns - // 3 attempts of 60s Netty response timeouts on top of an already-stuck pipeline and the - // hang propagates outward. With three parallel rails, a single such stall freezes the - // whole `flatMap(runTestAsync, 1)` for the remainder of the configured --duration, which - // is exactly what the six "Failed" large-payload pods exhibited on 2026-05-26 (see - // sdk/storage/BUG-blob-upload-hang-on-fault.md). - // - // Replace the retry with an outer per-operation timeout. Any single iteration that - // doesn't complete within `OPERATION_TIMEOUT` fails fast as a TimeoutException, which: - // 1. lets the rail recycle and start the next iteration, - // 2. surfaces a real failure on the dashboard instead of a frozen progress counter, - // 3. produces evidence (the TimeoutException) for SDK bug triage, and - // 4. still tolerates the natural per-op latency under fault injection (~6.3% of HTTP - // calls are configured-indefinite; effective avg op latency is a few seconds for - // small payloads and tens of seconds for large multi-request operations, well under - // the 2-minute cap below). - // If a scenario legitimately needs longer than the default per op, override - // `getOperationTimeout()` in the scenario class rather than blanket-raising it here. - // - // Tuning note (2026-05-26): the previous default was 2 minutes, which was too coarse. - // Under fault injection the dominant wedge is the `pq*` (request-side, indefinite) - // variant. Netty's `responseTimeout` already fires after 60s, so any operation still - // unresponsive ~30s after that is a real liveness bug and we want to fail fast and - // recycle the rail. Empirically this 4-6x's small-scenario throughput (small-blob ops - // typically complete in <2s; even fault-injected ops are bounded by the 60s Netty - // response timeout) while still leaving plenty of headroom for multi-block large - // operations via per-scenario overrides. - // - // CRITICAL: the outer `.onErrorResume(e -> Mono.empty())` converts a failed iteration - // into a successful "this iteration is done" signal *after* `instrumentRunAsync` has - // already fired its `doOnError` side-effect (which calls `trackFailure` and increments - // the `failed_runs` metric). Without this, a TimeoutException -- or any other error -- - // propagates out of `runTestAsync()` into `flatMap(runTestAsync, 1)` in - // ApiPerfTestBase.runAllAsync and terminates the entire Flux, cancelling all parallel - // rails and aborting the test loop. The previous `.retryWhen(Retry.max(3))` happened to - // mask this because most ops succeeded within retries, so the propagation path was - // rarely exercised; with the retry gone, every long-tail op would otherwise kill the - // whole job after the first 2-minute timeout. The error has already been logged and - // counted as a failure by the time `onErrorResume` runs, so no information is lost. return telemetryHelper.instrumentRunAsync(ctx -> runInternalAsync(ctx) - .timeout(getOperationTimeout()) + .retryWhen(reactor.util.retry.Retry.max(3) + .filter(e -> !(Exceptions.unwrap(e) + instanceof ContentMismatchException))) .doOnError(e -> { // Log the error for debugging but let legitimate failures propagate LOGGER.atError() .addKeyValue("error", e.getMessage()) .addKeyValue("errorType", e.getClass().getSimpleName()) - .log("Test operation failed"); - })) - .onErrorResume(e -> Mono.empty()); + .log("Test operation failed after retries"); + })); } - /** - * Default per-operation timeout for stress scenarios. Small-payload scenarios complete - * well under 2s in steady state and are bounded by Netty's 60s `responseTimeout` under - * fault injection, so 30s catches genuine liveness wedges without throwing away healthy - * long-tail ops. Multi-block large-payload variants need more time (a single op can be - * many sequential block uploads), so we scale the default up based on `options.getSize()`: - * one extra minute per 16 MiB above 1 MiB, capped at 5 minutes. Scenarios that need an - * even larger envelope can override this method: - * - * 
{@code
-     * @Override
-     * protected Duration getOperationTimeout() { return Duration.ofMinutes(10); }
-     * }
- */ - protected Duration getOperationTimeout() { - long sizeBytes = options.getSize(); - // Baseline 30s, +60s per 16 MiB beyond the first 1 MiB. Capped at 5 minutes so a - // genuinely-wedged rail still recovers promptly even for the largest configured payloads. - long extraSeconds = Math.max(0, (sizeBytes - SMALL_PAYLOAD_THRESHOLD_BYTES) / BYTES_PER_EXTRA_MINUTE) * 60; - long totalSeconds = Math.min(BASE_TIMEOUT_SECONDS + extraSeconds, MAX_TIMEOUT_SECONDS); - return Duration.ofSeconds(totalSeconds); - } - - private static final long SMALL_PAYLOAD_THRESHOLD_BYTES = (long) 1024 * 1024; // 1 MiB - private static final long BYTES_PER_EXTRA_MINUTE = 16L * 1024 * 1024; // 16 MiB - private static final long BASE_TIMEOUT_SECONDS = 30; - private static final long MAX_TIMEOUT_SECONDS = 5 * 60; - protected abstract void runInternal(Context context) throws Exception; protected abstract Mono runInternalAsync(Context context); diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java index 47ef361690ff..5700e1e7dd36 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/StorageSeekableByteChannelBlobReadBehavior.java @@ -90,11 +90,6 @@ public int read(ByteBuffer dst, long sourceOffset) throws IOException { return sourceOffset < resourceLength ? 0 : -1; } throw LOGGER.logExceptionAsError(e); - } catch (RuntimeException e) { - if (resourceLength > 0 && sourceOffset >= resourceLength && e.getCause() instanceof IOException) { - return -1; - } - throw e; } } diff --git a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java index b0f913667ab4..7e48798b30b0 100644 --- a/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java +++ b/sdk/storage/azure-storage-stress/src/main/java/com/azure/storage/stress/CrcInputStream.java @@ -10,6 +10,7 @@ import com.azure.perf.test.core.RepeatingInputStream; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.publisher.Sinks; import java.io.IOException; import java.io.InputStream; @@ -19,6 +20,7 @@ public class CrcInputStream extends InputStream { private final static ClientLogger LOGGER = new ClientLogger(CrcInputStream.class); + private final Sinks.One sink = Sinks.one(); private final InputStream inputStream; private final CRC32 crc = new CRC32(); private final ByteBuffer head = ByteBuffer.allocate(1024); @@ -42,6 +44,7 @@ public CrcInputStream(InputStream source) { public synchronized int read() throws IOException { int b = inputStream.read(); if (b < 0) { + emitContentInfo(); return b; } @@ -57,6 +60,7 @@ public synchronized int read() throws IOException { public synchronized int read(byte buf[], int off, int len) throws IOException { int read = inputStream.read(buf, off, len); if (read < 0) { + emitContentInfo(); return read; } @@ -68,6 +72,33 @@ public synchronized int read(byte buf[], int off, int len) throws IOException { return read; } + // Uses tryEmitValue instead of emitValue(FAIL_FAST) so that resubscriptions + // (SDK retries, verification passes) don't throw on the second EOF. + private void emitContentInfo() { + String baseErrorMessage = "Failed to emit content because "; + Sinks.EmitResult emitResult = sink.tryEmitValue(new ContentInfo(crc.getValue(), length, head)); + switch (emitResult) { + case OK: + case FAIL_TERMINATED: + // No action needed for successful or already-terminated emissions. + break; + case FAIL_CANCELLED: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + + " the sink was previously interrupted by its consumer: " + emitResult)); + case FAIL_OVERFLOW: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the buffer is full: " + emitResult)); + case FAIL_NON_SERIALIZED: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "two threads called emit at " + + "once: " + emitResult)); + case FAIL_ZERO_SUBSCRIBER: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "the sink requires a " + + "subscriber:" + emitResult)); + default: + throw LOGGER.logExceptionAsError(new RuntimeException(baseErrorMessage + "unexpected emit result: " + + emitResult)); + } + } + @Override public synchronized void mark(int readLimit) { if (markSupported) { @@ -93,32 +124,8 @@ public boolean markSupported() { return markSupported; } - /** - * Returns a {@link Mono} that, on subscription, captures a snapshot of the stream's - * current CRC, byte count and head buffer. - * - *

The returned Mono is intentionally lazy: it does not wait for EOF or - * for any sink to be signaled. Callers are therefore responsible for subscribing only - * after the stream has been fully consumed (for example, after the SDK upload - * call has returned for synchronous flows, or via {@code .then(data.getContentInfo())} - * for reactive flows). Subscribing before the stream is done will produce a snapshot of - * whatever has been read so far.

- * - *

This contract avoids the previous design's dependence on the SDK reading past EOF - * (which never happened on known-length uploads and could leave the legacy sink waiting - * indefinitely) and naturally tolerates SDK retries: the snapshot reflects the bytes - * that were actually delivered on the final, successful pass.

- * - * @return a cold Mono that emits a {@link ContentInfo} snapshot on each subscription. - */ public Mono getContentInfo() { - return Mono.fromCallable(() -> { - synchronized (this) { - // duplicate() shares the underlying byte[] but gives the caller an independent - // position/limit so subsequent reads on this stream don't perturb the snapshot. - return new ContentInfo(crc.getValue(), length, head.duplicate()); - } - }); + return sink.asMono(); } @Override From b939f566ff36367b43cdda2ac4c92c586b69e82a Mon Sep 17 00:00:00 2001 From: Isabelle Date: Sun, 31 May 2026 15:21:04 -0700 Subject: [PATCH 10/10] removing perf core change --- .../com/azure/perf/test/core/PerfStressProgram.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java b/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java index 5602b9b94f58..66eae27f7f3e 100644 --- a/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java +++ b/common/perf-test-core/src/main/java/com/azure/perf/test/core/PerfStressProgram.java @@ -348,21 +348,8 @@ public static void runTests(PerfTestBase[] tests, boolean sync, boolean compl .block(); } } catch (Exception e) { - // Previously this catch swallowed the exception, printed a stack trace, and let the - // method fall through to "=== Results ===" / globalCleanupAsync, which caused the - // process to exit 0. That made real test failures (e.g. an SDK NullPointerException - // surfaced by a retry path during a stress run) invisible to CI and dashboards: the - // Kubernetes Job reported Succeeded and the workbook showed a healthy success ratio - // even though the test loop had aborted partway through its configured duration. - // - // Rethrow as an unchecked exception so the surrounding lifecycle still runs the - // `finally` block below (progressStatus.cancel()) and the outer `try { ... } finally` - // in run(...) still drives globalCleanupAsync, but the JVM ultimately exits non-zero - // and the failure is visible. See sdk/storage/BUG-payload-size-gate-npe.md for the - // specific case that motivated this change. System.err.println("Error occurred running tests: " + System.lineSeparator() + e); e.printStackTrace(System.err); - throw (e instanceof RuntimeException) ? (RuntimeException) e : new RuntimeException(e); } finally { progressStatus.cancel(); }