Skip to content

fix(file-service, v1.2): retry LakeFS health check on startup - #7103

Open
Yicong-Huang wants to merge 2 commits into
apache:release/v1.2from
Yicong-Huang:backport/5647-retry-lakefs-health-check-on-startup-v1.2
Open

fix(file-service, v1.2): retry LakeFS health check on startup#7103
Yicong-Huang wants to merge 2 commits into
apache:release/v1.2from
Yicong-Huang:backport/5647-retry-lakefs-health-check-on-startup-v1.2

Conversation

@Yicong-Huang

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Backport of #5647 to release/v1.2, cherry-picked from 263093a.

Follows the Direct Backport Push convention; opened as a PR (rather than a direct push) as part of a backport-coverage audit for fixes merged to main since early June that were never labeled for backport.

Any related issues, documentation, discussions?

Backport of #5647. Originally linked #5646.

How was this PR tested?

Release-branch CI runs on this PR. The cherry-pick applied cleanly onto release/v1.2; no manual conflict resolution was needed.

Was this PR authored or co-authored using generative AI tooling?

Yes — backport prepared with Claude Code (mechanical cherry-pick; the change itself is #5647 by its original author).

### What changes were proposed in this PR?
- `LakeFSStorageClient.healthCheck()` now retries the LakeFS health
check with **exponential backoff** before failing, so a transient
`Connection reset` during concurrent startup no longer crashes the
file-service.
- Retry policy: 5 attempts with the delay doubling each time, starting
at 200ms (200, 400, 800, 1600ms), capping total wait at ~3s.
- The retry logic is factored into a small, reusable `retryWithBackoff`
helper with an injectable `sleep` function so the backoff can be
unit-tested without real waiting.
- Each failed attempt logs a warning; if LakeFS is still unreachable
after all attempts, startup fails with a clear aggregated error
(preserving the last exception as the cause) so a genuine outage is
still surfaced.
- An `InterruptedException` while waiting restores the thread's
interrupt status and fails fast instead of retrying.
- Added `LakeFSStorageClientSpec` covering immediate success,
retry-then-success with doubling delays, give-up-after-max-attempts
(with cause preserved), and interrupt handling.

### Any related issues, documentation, discussions?
Closes: apache#5646

### How was this PR tested?
- Unit tests: `LakeFSStorageClientSpec` exercises the backoff behavior
(delay sequence, max-attempt failure, interrupt handling) using an
injected `sleep`.
- Reproduce the race: stop LakeFS, start file-service, then start LakeFS
within a few seconds; confirm file-service logs retry warnings and then
starts successfully instead of exiting.
- Regression with LakeFS already up: start file-service, confirm it
binds `:9092` and `curl http://127.0.0.1:9092/api/healthcheck` returns
HTTP 200.
- Compile: run `sbt "FileService/compile"` and expect `[success]`.

### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF

(backported from commit 263093a)
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@codecov-commenter

codecov-commenter commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.12%. Comparing base (a9fa9bc) to head (1ecc242).
⚠️ Report is 2 commits behind head on release/v1.2.

Files with missing lines Patch % Lines
.../amber/core/storage/util/LakeFSStorageClient.scala 72.72% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             release/v1.2    #7103      +/-   ##
==================================================
+ Coverage           52.58%   54.12%   +1.53%     
+ Complexity           2520     1453    -1067     
==================================================
  Files                1078      809     -269     
  Lines               42343    34190    -8153     
  Branches             4561     3454    -1107     
==================================================
- Hits                22267    18504    -3763     
+ Misses              18762    14772    -3990     
+ Partials             1314      914     -400     
Flag Coverage Δ *Carryforward flag
access-control-service 64.35% <ø> (ø)
agent-service 34.36% <ø> (ø) Carriedforward from 29c3469
amber 59.81% <72.72%> (+7.03%) ⬆️
computing-unit-managing-service 1.65% <ø> (ø)
config-service 56.06% <ø> (ø)
file-service 61.72% <ø> (ø)
frontend 47.32% <ø> (+0.03%) ⬆️ Carriedforward from 29c3469
pyamber 90.98% <ø> (-0.03%) ⬇️ Carriedforward from 29c3469
python 90.82% <ø> (ø) Carriedforward from 29c3469
workflow-compiling-service 58.69% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants