Repository navigation
fix(inkless:storage): bound bulk-delete failure logging [KC-430] - #742
Merged
Merged
Conversation
jeqo
force-pushed
the
jeqo/bound-bulk-delete-logging
branch
from
August 12, 2026 16:11
ffcad22 to
194bab8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Bounds repeated bulk-delete failure logs across cloud storage backends.
Changes:
- Adds aggregated error counts, samples, and causes.
- Applies bounded logging to S3 and Azure.
- Removes GCS key enumeration from exceptions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
DeleteErrorSummaryTest.java |
Tests summary bounds and formatting. |
DeleteErrorSummary.java |
Implements aggregated failure logging. |
S3Storage.java |
Aggregates per-key S3 failures. |
AzureBlobStorage.java |
Aggregates Azure delete exceptions. |
GcsStorage.java |
Reports only failed key count. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeqo
force-pushed
the
jeqo/bound-bulk-delete-logging
branch
from
August 18, 2026 16:24
194bab8 to
c8434f7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
storage/inkless/src/test/java/io/aiven/inkless/storage_backend/common/DeleteErrorSummaryTest.java:32
AGENTS.mdrequires spelling out “for example” instead of usinge.g.in comments. Replace the abbreviation in this new Javadoc.
* A delete pass can fail for every key it submitted (e.g. a bucket policy denial) and repeats every
jeqo
force-pushed
the
jeqo/bound-bulk-delete-logging
branch
from
August 18, 2026 16:45
c8434f7 to
e1dd358
Compare
A delete pass that fails for every key it submitted logs one line per key, with a stack trace each on Azure, and repeats that on every cleanup cycle, since undeleted keys stay marked for deletion. GCS interpolates the whole key set into the exception message, which is the bug #733 fixed for S3. Count the failures of a pass by error code and log them once, at INFO when every code is a throttle and at WARN otherwise, so the output is bounded by the distinct codes rather than by the worklist. Report an exception that carries no service code in full instead, since the stack trace is then the only diagnosis: a whole-request `SdkException` on S3, any non-`BlobStorageException` on Azure. Both stop the pass, because neither is attributable to a single key and the rest retry on the next cycle. GCS logs only the key count, since it can't report per-key results. The counts describe one pass and never the worklist: `FileCleaner` reports how many candidates it deleted and derives `FileCleanerFilesFailedRate` from the same numbers. Both throttle sets keep only the codes the provider calls throttling, since the set picks the log level and a transient server error doesn't self-heal like backpressure. S3 drops `ServiceUnavailable`, which AWS classifies as a retryable 503. Azure keeps `ServerBusy` alone, sourced from `BlobErrorCode`. [KC-430] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeqo
force-pushed
the
jeqo/bound-bulk-delete-logging
branch
from
August 18, 2026 21:42
e1dd358 to
b7ec806
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (4)
storage/inkless/src/main/java/io/aiven/inkless/storage_backend/azure/AzureBlobStorage.java:231
- This new early exit contradicts the method comment at lines 212–215, which says that a failure on one key must not abandon the remaining keys. Update that comment to distinguish coded service failures, which continue, from code-less or unexpected failures, which stop the pass.
LOGGER.warn("Deleting {} failed unexpectedly, stopping the pass", key, Exceptions.unwrap(e));
break;
storage/inkless/src/main/java/io/aiven/inkless/storage_backend/azure/AzureBlobStorage.java:241
- No Azure failure-path test exercises this new aggregation and level selection; the shared backend tests cover only successful bulk deletion. Add tests that verify one aggregate
INFOrecord forServerBusy, one aggregateWARNrecord for a non-throttle code, and the early-stop behavior for an unexpected exception.
if (!failuresByCode.isEmpty()) {
// Throttling is backpressure the next FileCleaner cycle retries; any other code needs an
// operator, so it must not sit at INFO while the worklist stops draining.
if (THROTTLE_ERROR_CODES.containsAll(failuresByCode.keySet())) {
LOGGER.info("Delete failures by error code {}", failuresByCode);
} else {
LOGGER.warn("Delete failures by error code {}", failuresByCode);
storage/inkless/src/main/java/io/aiven/inkless/storage_backend/azure/AzureBlobStorage.java:225
BlobStorageException#getErrorCode()can returnnullwhen Azure or an intermediary returns no service error code.String.valueOfturns that into the literal"null", so this path logs only{null=n}and drops the stack trace that is needed to diagnose a code-less response. Detect a missing code, log the exception in full, and stop the pass; aggregate only real service codes.
This issue also appears in the following locations of the same file:
- line 230
- line 235
failuresByCode.merge(String.valueOf(e.getErrorCode()), 1, Integer::sum);
storage/inkless/src/main/java/io/aiven/inkless/storage_backend/s3/S3Storage.java:215
- The existing
S3StorageDeleteTestexercises mixed delete errors but asserts only returned keys and request count. It doesn't verify this PR's main contract: one bounded aggregate log atINFOwhen all codes are throttles and atWARNotherwise. Add log-capture cases for all-throttle and mixed-code responses so level or per-key logging regressions fail tests.
if (THROTTLE_ERROR_CODES.containsAll(failuresByCode.keySet())) {
LOGGER.info("Delete failures by error code {}", failuresByCode);
} else {
LOGGER.warn("Delete failures by error code {}", failuresByCode);
jeqo
marked this pull request as ready for review
August 18, 2026 22:03
tvainika
approved these changes
Aug 19, 2026
jeqo
added a commit
that referenced
this pull request
Aug 19, 2026
A delete pass that fails for every key it submitted logs one line per key, with a stack trace each on Azure, and repeats that on every cleanup cycle, since undeleted keys stay marked for deletion. GCS interpolates the whole key set into the exception message, which is the bug #733 fixed for S3. Count the failures of a pass by error code and log them once, at INFO when every code is a throttle and at WARN otherwise, so the output is bounded by the distinct codes rather than by the worklist. Report an exception that carries no service code in full instead, since the stack trace is then the only diagnosis: a whole-request `SdkException` on S3, any non-`BlobStorageException` on Azure. Both stop the pass, because neither is attributable to a single key and the rest retry on the next cycle. GCS logs only the key count, since it can't report per-key results. The counts describe one pass and never the worklist: `FileCleaner` reports how many candidates it deleted and derives `FileCleanerFilesFailedRate` from the same numbers. Both throttle sets keep only the codes the provider calls throttling, since the set picks the log level and a transient server error doesn't self-heal like backpressure. S3 drops `ServiceUnavailable`, which AWS classifies as a retryable 503. Azure keeps `ServerBusy` alone, sourced from `BlobErrorCode`. [KC-430] Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
jeqo
added a commit
that referenced
this pull request
Aug 19, 2026
A delete pass that fails for every key it submitted logs one line per key, with a stack trace each on Azure, and repeats that on every cleanup cycle, since undeleted keys stay marked for deletion. GCS interpolates the whole key set into the exception message, which is the bug #733 fixed for S3. Count the failures of a pass by error code and log them once, at INFO when every code is a throttle and at WARN otherwise, so the output is bounded by the distinct codes rather than by the worklist. Report an exception that carries no service code in full instead, since the stack trace is then the only diagnosis: a whole-request `SdkException` on S3, any non-`BlobStorageException` on Azure. Both stop the pass, because neither is attributable to a single key and the rest retry on the next cycle. GCS logs only the key count, since it can't report per-key results. The counts describe one pass and never the worklist: `FileCleaner` reports how many candidates it deleted and derives `FileCleanerFilesFailedRate` from the same numbers. Both throttle sets keep only the codes the provider calls throttling, since the set picks the log level and a transient server error doesn't self-heal like backpressure. S3 drops `ServiceUnavailable`, which AWS classifies as a retryable 503. Azure keeps `ServerBusy` alone, sourced from `BlobErrorCode`. [KC-430] Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A delete pass that fails for every key it submitted logs one line per key, with a stack trace each on Azure, and repeats that on every cleanup cycle, since undeleted keys stay marked for deletion. GCS interpolates the whole key set into the exception message, which is the bug #733 fixed for S3.
Count the failures of a pass by error code and log them once, at INFO when every code is a throttle and at WARN otherwise, so the output is bounded by the distinct codes rather than by the worklist. Report an exception that carries no service code in full instead, since the stack trace is then the only diagnosis: a whole-request
SdkExceptionon S3, any non-BlobStorageExceptionon Azure. Both stop the pass, because neither is attributable to a single key and the rest retry on the next cycle. GCS logs only the key count, since it can't report per-key results.The counts describe one pass and never the worklist:
FileCleanerreports how many candidates it deleted and derivesFileCleanerFilesFailedRatefrom the same numbers.Both throttle sets keep only the codes the provider calls throttling, since the set picks the log level and a transient server error doesn't self-heal like backpressure. S3 drops
ServiceUnavailable, which AWS classifies as a retryable 503. Azure keepsServerBusyalone, sourced fromBlobErrorCode.KC-430