Skip to content

[WIP] Backport: Convert escaping Throwable in copyLogSegmentData (0.0.1-SNAPSHOT shape) - #835

Draft
bingkunyangvungle wants to merge 1 commit into
Aiven-Open:mainfrom
bingkunyangvungle:build/0.0.1-with-pr833
Draft

[WIP] Backport: Convert escaping Throwable in copyLogSegmentData (0.0.1-SNAPSHOT shape)#835
bingkunyangvungle wants to merge 1 commit into
Aiven-Open:mainfrom
bingkunyangvungle:build/0.0.1-with-pr833

Conversation

@bingkunyangvungle

Copy link
Copy Markdown

Draft / WIP — not for merge as-is.

This PR carries the same fix as #833 (Convert escaping Throwable in copyLogSegmentData to RemoteStorageException) but applied against the 0.0.1-SNAPSHOT method shape that exists at tag 2024-10-23-1729694047 — the version we currently run in our QA Kafka brokers.

Why two PRs

  • #833 targets current main, where RemoteStorageManager.copyLogSegmentData was refactored to delegate to a KafkaRemoteStorageManager. Cherry-picking #833 onto the older 0.0.1-SNAPSHOT branch produces a merge conflict because the method body is structurally different.
  • This branch is based on the older tag and applies the same semantic fix (outer catch (Throwable)throw new RemoteStorageException(t)) directly inside the un-delegated upload chain.

Why the diff looks like a revert of 284 commits

This PR's base branch is the tag 2024-10-23-1729694047. Compared to current main, it's missing the ~284 commits that have landed since (including PR #797 and assorted dependabot bumps). It is not actually meant to merge into mainmain already has all those commits. The diff is shown as deletions because git compares to main.

What you actually want to review

Just the single new commit on top of the tag:

git --no-pager show eae6370 -- core/src/main/java/io/aiven/kafka/tieredstorage/RemoteStorageManager.java

The semantic change is identical to the catch-Throwable block already proposed in #833.

Intended use

Build artifact for our QA clusters that match 0.0.1-SNAPSHOT exactly (same dependency versions as the 2024-10-23 release). The upstream fix that actually goes into main is #833.

Status

WIP — keeping this open for internal review only. Will close once #833 lands and we cut a release built from a main-based artifact.

Refs #820, #833

…ption

Backport of upstream PR Aiven-Open#833 fix, adapted to the 0.0.1-SNAPSHOT plugin shape
where RemoteStorageManager directly performs uploadSegmentLog/uploadIndexes/
uploadManifest (no KafkaRemoteStorageManager delegate yet).

Why: an Error (e.g. OutOfMemoryError) escaping copyLogSegmentData bypasses
Kafka's RLMTask catch (Exception) and trips
ScheduledThreadPoolExecutor.scheduleWithFixedDelay's silent-suppression rule,
permanently stopping further copy attempts for that partition.

How: outer catch (Throwable) wraps any escaping Throwable as RemoteStorageException,
which the broker's existing catch handles, so the schedule survives.
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.

1 participant