Skip to content

fix(broker): map storage errors for ListOffsets and older Produce and Fetch versions - #273

Merged
PaxMachinaOne merged 3 commits into
KafScale:mainfrom
novatechflow:fix/255-retriable-s3-error
Oct 30, 2026
Merged

PaxMachinaOne merged 3 commits into
KafScale:mainfrom
novatechflow:fix/255-retriable-s3-error

Conversation

@novatechflow

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #272 from QA review. #272 was merged before these changes, so they come as a new PR from the same branch.

Changes

  • ListOffsets: an earliest (-2) lookup that cannot load a partition from S3 returned UNKNOWN_SERVER_ERROR. It now returns KAFKA_STORAGE_ERROR (56) on every version, as Kafka 3.9.1 does in handleListOffsetRequestV1AndAbove and handleListOffsetRequestV0, where KafkaStorageException maps through Errors.forException with no version check.
  • Produce below v4 returns NOT_LEADER_OR_FOLLOWER (6) instead of 56. ProduceRequest.json and ProduceResponse.json say version 4 added KAFKA_STORAGE_ERROR.
  • Fetch v5 and below returns 6 instead of 56, matching maybeDownConvertStorageError in Kafka 3.9.1 KafkaApis (versionId <= 5). FetchResponse.json says version 6 may return KAFKA_STORAGE_ERROR.
  • Produce append errors that do not come from S3 keep UNKNOWN_SERVER_ERROR with no message. pkg/storage now wraps S3 upload and cleanup failures in storage.ErrObjectStorage, and the broker uses the same error for restore failures.
  • Updates docs/protocol.md, docs/architecture.md and the release note.

Tests

  • TestStorageErrorDownConvertsForOldVersions: Produce v0 and v3 get 6, v4 and v9 get 56. Fetch v4 and v5 get 6, v6 and v11 get 56.
  • TestListOffsetsEarliestStorageFailure: v0, v1 and v4 get 56.
  • TestProduceAppendNonStorageErrorStaysUnknown: a canceled append gets -1 with no message.
  • TestPartitionLogFlushErrorPreservesBuffer now checks that a failed upload wraps storage.ErrObjectStorage.

The new broker tests fail against main and pass with the change.

Refs #255

Produce and Fetch returned UNKNOWN_SERVER_ERROR when S3 was unavailable, when an S3 write or read failed, and when a partition could not be restored from S3. librdkafka treats that as permanent and drops the message. They now return KAFKA_STORAGE_ERROR (56), keep REQUEST_TIMED_OUT for degraded S3, and set a clear error message on Produce v8 and later.

Fixes KafScale#255
… Fetch versions

An earliest ListOffsets lookup that cannot load a partition from S3 returned UNKNOWN_SERVER_ERROR. It now returns KAFKA_STORAGE_ERROR, as Kafka does. Produce below v4 and Fetch v5 or below predate KAFKA_STORAGE_ERROR, so they now receive NOT_LEADER_OR_FOLLOWER, matching Kafka's maybeDownConvertStorageError for Fetch. Produce append errors that do not come from S3 keep UNKNOWN_SERVER_ERROR.

Refs KafScale#255

@PaxMachinaOne PaxMachinaOne left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 PaxMachina automated review (worker-code)

The version-aware error mapping and object-storage error classification match Kafka behavior and are adequately tested.

No blocking findings.

@PaxMachinaOne
PaxMachinaOne merged commit f1d0301 into KafScale:main Oct 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants